feathersjs-ecosystem / feathers-swagger

Add documentation to your FeatherJS services and feed them to Swagger UI.
MIT License
226 stars 63 forks source link

Bug: Not compatible with newest version of DOVE #225

Closed MarcGodard closed 3 years ago

MarcGodard commented 3 years ago

Starting at DOVE V5 pre-6 this add on isn't working anymore:

/Users/marcgodard/Documents/Github/template/node_modules/feathers-swagger/lib/index.js:85
      app.providers.push((path, service) => specGenerator.addService(service, path));

Let me know if I should cross post this issue. Worked fine on pre-5.

Mairu commented 3 years ago

I have not looked into dove yet, but I already thought that a new version will be necessary, especially if something schema like will be introduced, that could directly be used. I though that a new major version for dove without support for lower versions would be the best. Perhaps @daffl can shed some light when dove will be feature complete, at least in the regions where feathers-swagger would be affected.

daffl commented 3 years ago

Yes, I was going to create an issue for that version once everything is put together (especially the schema stuff). I think there is a really nice way on putting a lot of it together automatically.

For v5 I removed the app.providers because it really ended up doing the same as app.mixins. Now I'm actually debating of adding an alias of app.mixins to app.providers to not break so many things.

daffl commented 3 years ago

Maybe not, another reason why it got removed was that the signature was different. However, I think this error can be fixed already in a non-breaking way by changing

app.providers.push((path, service) => specGenerator.addService(service, path));

To

app.mixins.push((service, path) => specGenerator.addService(service, path));
Mairu commented 3 years ago

Well app.mixins should already be used for newer feather versions, but the pre versions seems to have the version number not set like previous versions.

https://github.com/feathersjs-ecosystem/feathers-swagger/blob/6dff498d35d0e8d1c2dc7187efa78fa94ecfbc7c/lib/index.js#L82-L86

daffl commented 3 years ago

Ah, that might be it then, the publish was busted. I believe this should work again with .pre.9 then.