Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Order of multiple hooks: mismatch of documentation and behavior #38

Closed
Mairu opened this issue Jun 1, 2020 · 2 comments · Fixed by #37
Closed

Order of multiple hooks: mismatch of documentation and behavior #38

Mairu opened this issue Jun 1, 2020 · 2 comments · Fixed by #37

Comments

@Mairu
Copy link

Mairu commented Jun 1, 2020

Steps to reproduce

Running the script from the documentation: https://github.com/feathersjs/hooks#middleware

Expected behavior

The output of the script should be (documented):

hook1 before
hook2 before
hook3 before
Hello David
hook3 after
hook2 after
hook1 after

Actual behavior

The output of the script is:

hook2 before
hook1 before
Hello David!
hook1 after
hook2 after
hook3 after

Thoughts

As hooks are decorators, it is clear that the last wrapped hook is run first.
So the question is, if the documentation should be adjusted, or if the order of the array should be reversed for wrapping. So that the first entry is wrapped as last one.

Brought up in Slack: https://feathersjs.slack.com/archives/C0HJE6A65/p1591009725097800?thread_ts=1590992656.097000&cid=C0HJE6A65

@daffl
Copy link
Member

daffl commented Jun 1, 2020

I believe this should be fixed via #37. There was a middleware.reverse() in there (f597ad1#diff-dfef6fe5d351adc488c7440e64968cd7R53) that definitely did not belong there.

@daffl daffl closed this as completed in #37 Jun 1, 2020
@daffl
Copy link
Member

daffl commented Jun 1, 2020

Published as v0.5.0

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

Successfully merging a pull request may close this issue.

2 participants