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

Decorations not propagated to sibling plugins #2727

Closed
Marsup opened this issue Aug 19, 2015 · 8 comments
Closed

Decorations not propagated to sibling plugins #2727

Marsup opened this issue Aug 19, 2015 · 8 comments
Assignees
Labels
Milestone

Comments

@Marsup
Copy link
Contributor

@Marsup Marsup commented Aug 19, 2015

Credits go to @jedireza for finding this one.

Registration order has an impact on decorations access, regardless of the use of server.dependency.
For exemple calling server.views with [Vision, Lout] will work, [Lout, Vision] won't : TypeError: Object [object Object] has no method 'views'.

Decorations are only copied on the initialization of the plugins, so while the server.root is always aware of those decorations, this line will not be propagate the decorators to others.

It seems I have 2 choices here :

  • always use server.root.*
  • fix it in hapi, by copying again (maybe here) all the decorations from the root

Willing to make a PR if that's the 2nd case.

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 19, 2015

Why is this not an issue of lout needing to wait for vision to load? You are seeing it now because vision is not built in anymore but this isn't new. Before it was still the same problem. You need to wait for after event when vision is loaded.

I am not saying that's the only issue here but that it's not just a matter of supporting random order.

@Marsup
Copy link
Contributor Author

@Marsup Marsup commented Aug 19, 2015

I am. New Plugin instances are provided to each register, they don't share the same this (though they do share the same root).

@Marsup
Copy link
Contributor Author

@Marsup Marsup commented Aug 19, 2015

To be even more explicit, I think we should have a similar loop as this one before calling the dependency callback.

@Marsup
Copy link
Contributor Author

@Marsup Marsup commented Aug 29, 2015

@hueniverse do you want a PR doing what I suggested in OP ?

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 30, 2015

Nah. I need to do a deeper fix.

@hueniverse hueniverse added this to the 9.0.4 milestone Sep 2, 2015
@hueniverse hueniverse closed this in 3a9028b Sep 2, 2015
@Marsup
Copy link
Contributor Author

@Marsup Marsup commented Sep 2, 2015

Wow, that was simpler than I expected :)

@devinivy
Copy link
Member

@devinivy devinivy commented Sep 2, 2015

That commit is sort of delightful.

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Sep 2, 2015

There are some dark corner of this code base :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants