-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
RFC: Asset Loader Service #158
Conversation
Where are we at with this? @dgeb / @MiguelMadero / @nathanhammond / @trentmwillis - Have y'all reviewed? |
We have a working implementation in ember-asset-loader that has been used successfully for both lazy loading Engines and generic use cases (e.g., loading a charting lib). Primary open question to me is where does this Service live in the future? For many use cases, having a standalone, core-maintained addon seems sufficient. But, for the lazy Engines use case, it needs to integrate with some of the internals of Ember's Router, which makes it seem like this could potentially be part of Ember itself. At the very least, if it continues as a separate addon, we likely need a solid API for hooking into the Router. |
Have reviewed the RFC and many of the PRs that went into the implementation. This is set by my estimation. |
988be15
to
2ca9b52
Compare
@rwjblue clarified some wording. I think this is ready to move to FCP. |
We discussed this during todays Ember core team meeting today, and we are 👍 on this moving forward. Given that there is little conversation here, and the core team is in favor I an moving this forward into the final comment period. |
|
||
`loadAsset` will return an `AssetPromise` that will resolve when the asset is successfully loaded or reject when the asset fails to load. | ||
|
||
Subsequent calls to `loadAsset` with the same arguments will return the same `AssetPromise` object to ensure a single source of truth for determining asset state as well as to prevent duplicate requests being made for the same asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we clarify here that this is true unless/until retryLoad
is called, and link down to the Provide A Way To Attempt Recovery
section?
Discussed this a bit with @tomdale and we decided that we should close this. Not as "wontfix" but more as "already done" in addon space without needing more detailed core framework involvement. This is absolutely public API from ember-engines point of view, and it will be supported as such. @trentmwillis - Thank you very much for your hard work on this, and I'm very sorry that we let this sit for sooo long. |
Pre-reading/context: Asset Manifest RFC
Rendered.