search

feathersjs-ecosystem / feathers-hooks-common

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

chore: update tests for replaceItems with actOnDispatch #752

Closed jd1378 closed 3 months ago

jd1378 commented 3 months ago

Summary

Tests for #751

fratzinger commented 3 months ago

Wow. Thanks for the fast delivery! 💚 Looks good. The only see I wonder about is 'NOTE THIS TEST NOTE THIS TEST' in one of the tests? Is this a leftover or intentional?

jd1378 commented 3 months ago

well I just copied the test block above it and modified it for dispatch, but It looks like an important test so it might be intentional (?). but I did not add that part for sure

fratzinger commented 3 months ago

lol. merged it then. I want to work on a few things in this repo anyways. I don't like the implicit magic that happens around replaceItems. When to use context.data, when to use context.result depending on context.type? It does not work for around hooks at all? What do you want to achieve with replaceItems in a before hook if there already is context.result because you want to skip the adapter call.

Too many questions and to much magic for my taste. I want to introduce a more explicit replaceData and replaceResult and maybe replaceDispatch or get that covered by replaceResult(..., { useDispatch: true | false | 'ifExistent' }) or something. Just thinking out loud.

That said I'll eventually rework this stuff anyways.

jd1378 commented 3 months ago

hmm, I found the bug for replaceItems when I was using feathers-casl. it was using replaceItems but things were not getting replaced properly* (feathers-pinia had a bug where it couldn't handle arrays, it's fixed now). so It took me some time to pin point it to the dispatch issue. and then I had to patch feathers-casl to replace both result and dispatch.
I have done A LOT of modifications to feathers-casl to get it working the way I want it to, and it's still not what I expect. I know it's not the place to discuss it, just wanted to mention where this little utility was having impact on my project. (btw feathers-casl's most interesting feature for me was the getChannelsWithReadAbility function, which worked very good)

fratzinger commented 3 months ago

Wrote you on discord. I would love to chat about this stuff. Not now but in the near future.