feathers-objection icon indicating copy to clipboard operation
feathers-objection copied to clipboard

Getting Ready for Feathers v5 Dove

Open marshallswain opened this issue 3 years ago • 17 comments

The following changes are needed to fully prepare feathers-objection for Feathers v5 Dove:

  • [ ] Port to TypeScript
  • [ ] Support the updated @feathersjs/adapter-tests suite. (including params.adapter)
  • [ ] Publish compiled JS version
  • [ ] Automate Postgres Tests
    • [ ] Split Postgres-specific tests into their own file.
    • [ ] Setup Postgres container in Github Actions
    • [ ] Setup test-local environment for testing against local Postgres DB.
    • [ ] Setup ci environment for Github Actions.

marshallswain avatar Feb 12 '22 21:02 marshallswain

Is this plugin still planned to be made fully compatible with Feathers v5 dove (and integrated back into feathers cli as an option when generating services)?

ramsestom avatar Feb 15 '23 18:02 ramsestom

@ramsestom It might already work with Feather v5. I'm not aware of any breaking-changes and it seems that the list from @marshallswain is mostly about TypeScript and tests. I'm not using FeathersJS in my current projects, but any PRs are welcome.

dekelev avatar Feb 24 '23 14:02 dekelev

While upgrading dependencies, I've tried running the tests with Feathers v5 and it didn't work. Looks like the AdapterService class that it is extending doesn't exist anymore. As much as I'd like to help, I'm not into TypeScript development these days and also haven't used FeathersJS for the past 2 years, so unless I get a PR regarding this upgrade, this library will remain unsupported in FeathersJS v5 (same for other plugins that I contributed in the past - feathers-cassandra, feathers-service-tests-cassandra, feathers-http-disributed, feathers-opentracing, feathers-kong, feathers-stripe-webhooks, feathers-hubspot-webhooks).

dekelev avatar Mar 04 '23 10:03 dekelev

@dekelev I think that it got moved to @feathersjs/adapter-commons and renamed to AdapterBase. Does this look like the right class? https://github.com/feathersjs/feathers/blob/dove/packages/adapter-commons/src/service.ts#L22-L29

marshallswain avatar Mar 08 '23 16:03 marshallswain

Thanks @marshallswain , I tried that now and got this error on invalid service object that was trying to initialize for the tests. Any idea why it's failing?

image image

dekelev avatar Mar 08 '23 17:03 dekelev

I had to remove the underscore from the service method names. It works now, but some tests are failing on code related to the FeathersError class. This import is not defined anymore:

import errors from '@feathersjs/errors';

It works when importing each class in separate.

dekelev avatar Mar 08 '23 17:03 dekelev

Any idea where's the filterQuery base service method? It's not defined in the old place (this.filterQuery).

dekelev avatar Mar 08 '23 17:03 dekelev

Try importing it from @feathersjs/adapter-commons, as well. https://github.com/feathersjs/feathers/blob/dove/packages/adapter-commons/src/query.ts#L137

marshallswain avatar Mar 08 '23 17:03 marshallswain

import errors from '@feathersjs/errors';

I think it's now

import * as errors from '@feathersjs/errors' 

if you want them all.

marshallswain avatar Mar 08 '23 17:03 marshallswain

Got it, thanks

dekelev avatar Mar 08 '23 17:03 dekelev

image

Is the whitelist option deprecated? Was it replaced by something else?

dekelev avatar Mar 08 '23 18:03 dekelev

@dekelev, Two solutions replaced it. If you're using @feathersjs/schema or @feathersjs/typebox, the query validators replace the need for whitelist. If you're not using the new validators, you can find the replacement for the whitelist here: https://feathersjs.com/guides/migrating.html#custom-filters-operators

marshallswain avatar Mar 16 '23 03:03 marshallswain

Thanks! I'll check it out

dekelev avatar Mar 16 '23 09:03 dekelev

@dekelev Can you push your branch? I’ll see if I can help 🙂

nickdex avatar Mar 25 '23 02:03 nickdex

Sure, I'll push it soon. It's still a mess and I haven't got time to work on it since last week.

dekelev avatar Mar 25 '23 07:03 dekelev

I pushed the feathers-dove branch.

dekelev avatar Mar 25 '23 08:03 dekelev