Skip to content

Commit

Permalink
fix(engine-loader): avoid stale asset-loader
Browse files Browse the repository at this point in the history
For some reason the injection of the `asset-loader` becomes "stale" in
production builds. It seems like, the service is looked up first, then
re-registered by a third-party and the injection is not updated.

This causes the manifest to be empty and the loading to always fail.

I have no idea what the root cause is, but this trivial change is safe
and makes sure that we always look up the "feshest" version of the
service.
  • Loading branch information
buschtoens committed Jun 26, 2019
1 parent 2852190 commit d30931e
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions addon/services/engine-loader.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import Service from '@ember/service';
import { get } from '@ember/object';
import { getOwner } from '@ember/application';
import { inject as service } from '@ember/service';
import require from 'require';

export default Service.extend({
assetLoader: service(),

/**
* Checks the owner to see if it has a registration for an Engine. This is a
* proxy to tell if an Engine's assets are loaded or not.
Expand Down Expand Up @@ -44,7 +40,7 @@ export default Service.extend({
async load(name) {
if (this.isLoaded(name)) return;

const assetLoader = get(this, 'assetLoader');
const assetLoader = getOwner(this).lookup('service:asset-loader');
await assetLoader.loadBundle(name);
this.register(name);
}
Expand Down

2 comments on commit d30931e

@jbailey4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buschtoens Thanks for this great addon! I believe this revision may have fixed an issue my team has been trying to solve, which is lazy loading a route-less engine within a lazy route engine...

It wasn't working before this version, and now it seems to be fixed. If I recall, the original issue was definitely something around the const assetLoader = get(this, 'assetLoader') not being available in our scenario. Seems like always looking up the "freshest" version fixed that.

@buschtoens
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbailey4 That's great to hear! Thank you. ❤️

Please sign in to comment.