Skip to content

Commit

Permalink
Make sure lazy engines with async routes are handled
Browse files Browse the repository at this point in the history
Lazy engine routes are initially promises which resolve to
route instances once the modules are loaded. In these cases
prefetch current throws an error 'TypeError: route.prefetch is not a function'
since it tries to call prefetch directly on the promise. It
seems this used to be handled properly but was lost in a more
recent refactor. This is just adding the functionality back.
It's important to note that we do not preload the lazy engine
assets in the test environment here because preloaded engine
routes are not promises but the actual route instance.

- Adds ember-engines to dev dependencies for testing.
- Create a lazy engine in the dummy app.
- Make sure we can visit this engine without preloading its assets.
  • Loading branch information
Jackson Dean authored and jackson-dean committed Oct 1, 2019
1 parent cd654e9 commit 1d3e64d
Show file tree
Hide file tree
Showing 19 changed files with 2,729 additions and 1,289 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

# dependencies
/node_modules/
tests/dummy/lib/**/node_modules

# misc
/coverage/
Expand Down
8 changes: 7 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ module.exports = {
'config/**/*.js',
'tests/dummy/config/**/*.js',
],
excludedFiles: ['addon/**', 'addon-test-support/**', 'app/**', 'tests/dummy/app/**'],
excludedFiles: [
'addon/**',
'addon-test-support/**',
'app/**',
'tests/dummy/app/**',
'tests/dummy/lib/**',
],
parserOptions: {
sourceType: 'script',
ecmaVersion: 2015,
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# dependencies
/node_modules/
tests/dummy/lib/**/node_modules

# misc
/.env*
Expand Down
7 changes: 7 additions & 0 deletions addon/-private/utils/is-thenable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function isThenable(obj) {
if ((typeof obj === 'object' && obj !== null) || typeof obj === 'function') {
return typeof obj.then === 'function';
}

return false;
}
22 changes: 19 additions & 3 deletions addon/services/prefetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { schedule } from '@ember/runloop';
import { createPrefetchChangeSet } from '../-private/diff-route-info';
import RSVP from 'rsvp';
import { gte } from 'ember-compatibility-helpers';
import { isThenable } from '../-private/utils/is-thenable';

let PrefetchService;

Expand All @@ -28,9 +29,24 @@ if (gte('3.6.0')) {
let { route, fullParams } = changeSet.for[i];
if (seenRoutes.has(route)) continue;

route._prefetched = new RSVP.Promise(r => {
return r(route.prefetch(fullParams, transition));
});
if (isThenable(route)) {
// Ensure we use the expected fullParams when the promise then
// handler is executed for lazy loaded routes.
RSVP.Promise.resolve(fullParams).then(capturedFullParams => {
route.then(resolvedRoute => {
if (this.isDestroying || transition.isAborted) {
return;
}

resolvedRoute._prefetched = RSVP.Promise.resolve(
resolvedRoute.prefetch(capturedFullParams, transition)
);
});
});
} else {
route._prefetched = RSVP.Promise.resolve(route.prefetch(fullParams, transition));
}

seenRoutes.set(route, true);
if (transition.isAborted) return;
}
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"ember-cli-template-lint": "^1.0.0-beta.1",
"ember-cli-uglify": "^2.1.0",
"ember-disable-prototype-extensions": "^1.1.3",
"ember-engines": "^0.8.2",
"ember-export-application-global": "2.0.0",
"ember-load-initializers": "^1.1.0",
"ember-maybe-import-regenerator": "^0.1.6",
Expand All @@ -40,6 +41,7 @@
"eslint-plugin-ember": "^7.1.0",
"eslint-plugin-node": "^10.0.0",
"eslint-plugin-prettier": "^3.1.1",
"lazy-engine": "link:./tests/dummy/lib/lazy-engine",
"loader.js": "^4.7.0",
"prettier": "^1.18.2",
"qunit-dom": "^0.8.0"
Expand Down
14 changes: 14 additions & 0 deletions tests/acceptance/lazy-engine-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { module, test } from 'qunit';
import { visit, currentURL } from '@ember/test-helpers';
import { setupApplicationTest } from 'ember-qunit';

module('Acceptance | lazy engine', function(hooks) {
setupApplicationTest(hooks);

test('it can load an async route from a lazy loaded engine', async function(assert) {
await visit('/lazy-engine/my-route');

assert.dom('#test-my-route-model').hasText('test prefetch hook');
assert.equal(currentURL(), '/lazy-engine/my-route');
});
});
1 change: 1 addition & 0 deletions tests/dummy/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Router.map(function() {
this.route('abort-transition-to-child', function() {
this.route('child');
});
this.mount('lazy-engine');
});

export default Router;
14 changes: 14 additions & 0 deletions tests/dummy/lib/lazy-engine/addon/engine.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import Engine from 'ember-engines/engine';
import loadInitializers from 'ember-load-initializers';
import Resolver from './resolver';
import config from './config/environment';

const { modulePrefix } = config;
const Eng = Engine.extend({
modulePrefix,
Resolver,
});

loadInitializers(Eng, modulePrefix);

export default Eng;
3 changes: 3 additions & 0 deletions tests/dummy/lib/lazy-engine/addon/resolver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import Resolver from 'ember-resolver';

export default Resolver;
5 changes: 5 additions & 0 deletions tests/dummy/lib/lazy-engine/addon/routes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import buildRoutes from 'ember-engines/routes';

export default buildRoutes(function() {
this.route('my-route');
});
8 changes: 8 additions & 0 deletions tests/dummy/lib/lazy-engine/addon/routes/my-route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Route from '@ember/routing/route';
import RSVP from 'rsvp';

export default Route.extend({
prefetch() {
return RSVP.Promise.resolve({ test: 'test prefetch hook' });
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{outlet}}
1 change: 1 addition & 0 deletions tests/dummy/lib/lazy-engine/addon/templates/my-route.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div id="test-my-route-model">{{model.test}}</div>
11 changes: 11 additions & 0 deletions tests/dummy/lib/lazy-engine/ember-config/environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-env node */
'use strict';

module.exports = function(environment) {
let ENV = {
modulePrefix: 'lazy-engine',
environment,
};

return ENV;
};
17 changes: 17 additions & 0 deletions tests/dummy/lib/lazy-engine/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* eslint-env node */
'use strict';

// eslint-disable-next-line node/no-extraneous-require
const EngineAddon = require('ember-engines/lib/engine-addon');

module.exports = EngineAddon.extend({
name: 'lazy-engine',

lazyLoading: Object.freeze({
enabled: true,
}),

isDevelopingAddon() {
return true;
},
});
15 changes: 15 additions & 0 deletions tests/dummy/lib/lazy-engine/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "lazy-engine",
"version": "0.0.0",
"keywords": [
"ember-addon",
"ember-engine"
],
"ember-addon": {
"engineConfigPath": "ember-config"
},
"dependencies": {
"ember-cli-babel": "*",
"ember-cli-htmlbars": "*"
}
}
12 changes: 12 additions & 0 deletions tests/unit/utils/is-thenable-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { isThenable } from 'ember-prefetch/-private/utils/is-thenable';
import { module, test } from 'qunit';

module('Unit | Utility | is-thenable', function() {
test('basic sanity check', function(assert) {
const thenable = { then: () => {} };
assert.equal(isThenable(thenable), true, 'returns true for thenable');
assert.equal(isThenable({}), false, 'returns false for non thenable');
assert.equal(isThenable(), false, 'returns false for undefined');
assert.equal(isThenable(null), false, 'returns false for null');
});
});
Loading

0 comments on commit 1d3e64d

Please sign in to comment.