feathersjs-ecosystem / feathers-hooks-common

Useful hooks for use with FeathersJS services.
https://hooks-common.feathersjs.com
MIT License
193 stars 89 forks source link

isProvider() not working as expected #678

Closed uplke closed 2 years ago

uplke commented 2 years ago

Consider a provider check in a hook: iff(isProvider('external'), doSomethingForExternal())

Expected behavior

doSomethingForExternal() is executed only if context.params.provider === 'external'

Actual behavior

doSomethingForExternal() is executed if context.params.provider === 'rest'

Replacing isProvider() works as expected: iff(context => context.params.provider === 'external', doSomethingForExternal())

Module versions v6.1.0 Possibly an issue with the comparison in index.js - line 784: providers.some((provider) => provider === hookProvider || provider === "server" && !hookProvider || provider === "external" && !!hookProvider)

fratzinger commented 2 years ago

It's not a bug, it's a feature ™️

How does it come, that you use context.params.provider as 'external'? I don't know any feathers library, that does this. isProvider expects your input as: 'socketio' | 'rest' | 'primus' | undefined. That are the common values for context.params.provider. If you set it yourself, please set it to socketio or rest to be consistent with other libraries.

uplke commented 2 years ago

Thx for the quick reply and the good work - I was missing the valid values for context.params.provider that isProvider extends-) For complete reference: https://docs.feathersjs.com/api/services.html#params context.params.provider can be 'socketio' | 'rest' | 'primus' | undefined

https://hooks-common.feathersjs.com/utilities.html#isprovider isProvider extends this and accepts 'socketio' | 'rest' | 'primus' | 'external' | 'server'