Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(core): Run after all hooks first, and then after method hooks #3004

Merged
merged 1 commit into from
Jan 28, 2023

Conversation

KidkArolis
Copy link
Contributor

Summary

Fix #3002 - bring back the "after all" and "after method" hook ordering from V4.

The implementation is not as neat as it was before (having just 1 set of collected hooks was a nice idea). Let me know if you have a better idea.

@netlify
Copy link

netlify bot commented Jan 23, 2023

‼️ Deploy request for feathers-dove rejected.

Name Link
🔨 Latest commit f771955

@daffl daffl changed the title fix(feathers): Run after all hooks first, and then after method hooks fix(core): Run after all hooks first, and then after method hooks Jan 28, 2023
@daffl
Copy link
Member

daffl commented Jan 28, 2023

Thank you for looking into this. I can't believe we didn't have a test covering that. I was trying a few things to see if there was a different fix but I think this will have to be it for now. The fun and not-so-fun fact here is that a lot of that code is for backwards compatibility for a very specific case which is being able to call service.hooks multiple times.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After all and after method specific hook order changed in V5
2 participants