fetch addon config lazily #166
Conversation
lennyburdette
force-pushed
the
lenny/lazy-config
branch
from
00ef1fc
to
733e02d
Compare
index.js
Outdated
| @@ -18,19 +18,22 @@ module.exports = { | |||
| included() { | |||
| this._super.included.apply(this, arguments); | |||
|
|
|||
| const host = this._findHost(); | |||
| this.addonConfig = host.options['apollo'] || {}; | |||
| this.host = this._findHost(); | |||
There was a problem hiding this comment.
Is it necessary to store this reference to the host, rather than just calling _findHost() in the addonConfig method?
There was a problem hiding this comment.
I don't have strong feelings on this. _findHost() does a little bit of work, iterating up the addon tree (while (parent = this.parent)). But this code path is called infrequently so the cost of the work won't add up to much.
What do you think?
There was a problem hiding this comment.
That's true! I don't have a strong argument either way, your point about going up the tree is really good.
There was a problem hiding this comment.
Thanks for submitting this! I do think
@alexlafroscia's point about not needing to store the host would make the code slightly easier to follow, and shouldn't have any meaningful performance impact.
Could you amend your commit to make that one change? Otherwise this is good to go so I'll merge as soon as that's done. Thanks 🙏
It's a somewhat common practice for one addon to augment another addon's config. One example is ember-decorators, which adds babel plugins to ember-cli-babel's config. For my use case: I'm using ember-apollo-client in an application with several engines. I'm creating the client in the host application and injecting it into the individual engines (partly because of ember-graphql#152, but I think this structure generally make sense). To make testing each engine easier, I'm wrapping ember-apollo-client with an addon that preconfigures the client. I then install my wrapper addon into the "dummy" app for each engine. Before this change, ember-apollo-client caches its config (in `included()`) before my wrapper addon has a chance to mutate it.
lennyburdette
force-pushed
the
lenny/lazy-config
branch
from
733e02d
to
bdd0ac5
Compare
|
@bgentry Done! 👍 |
|
Thank you @bgentry and @alexlafroscia ! |
It's a somewhat common practice for one addon to augment another addon's config. One example is ember-decorators, which adds babel plugins to ember-cli-babel's config.
For my use case: I'm using ember-apollo-client in an application with several engines. I'm creating the client in the host application and injecting it into the individual engines (partly because of #152, but I think this structure generally make sense). To make testing each engine easier, I'm wrapping ember-apollo-client with an addon that preconfigures the client. I then install my wrapper addon into the "dummy" app for each engine.
Before this change, ember-apollo-client caches its config (in
included()) before my wrapper addon has a chance to mutate it.