feathersjs-ecosystem / feathers-sync

Synchronize service events between Feathers application instances
MIT License
221 stars 41 forks source link

Needs update for Feathers v5 #166

Closed barankyle closed 3 years ago

barankyle commented 3 years ago

We recently upgraded to Feathers version 5 and discovered that a couple of things in core.js are no longer valid. I forked this repo and added some fixes to make it work, but I wasn't sure if at least one of them was the ideal way to do it since I still am wrapping my head around the differences in v5: https://github.com/XRFoundation/feathers-sync/blob/release/lib/core.js

One, _serviceEvents appears to no longer be a field on the service object, so service.serviceEvents.include errors out from trying to get a property of undefined. From going through the Feathers code it seemed that importing and using the getServiceOptions method on the service object was the new way to get serviceEvents.

Two, @feathersjs/commons no longer has hooks or _, so the conditional stripping of app and service from the context won't work. I copied isHookObject into core.js and stripped out app and service via object destructuring, but I'm not sure if v5 has some newer way to check if something is a hook or a replacement for their implementation of _.

mrfrase3 commented 3 years ago

I just spent 1.5 days debugging this :sob: as it annoyingly runs after the global hooks, so the error wasn't getting caught, and all I got from the client was "Cannot read property 'includes' of undefined", which is not very helpful.

I'd suggest that once fixed, updating this package should be mentioned in the migration guide.

Anyway, here is a patch file for your change as well, for use with patch-package feathers-sync+2.3.0.zip

daffl commented 3 years ago

https://github.com/feathersjs-ecosystem/feathers-sync/pull/167 should allow compatibility for both, v4 and v5. Maybe someone else can give it a quick test before it is published.

mrfrase3 commented 3 years ago

Hey, been running this in v5 on our dev instances since you updated it, haven't had any issues so far

daffl commented 3 years ago

Great, thank you for letting me know. Published as v2.4.0