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.

Related changes:
- Update node for CI to version 10 to satisfy the
requirements for ember-engines.
- Change CI install command to fix errors when yarn tries to
install link dependencies.
  • Loading branch information
Jackson Dean authored and jackson-dean committed Sep 20, 2019
1 parent fc5a602 commit ab03a39
Show file tree
Hide file tree
Showing 15 changed files with 676 additions and 15 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/tmp/

# dependencies
/node_modules/
**/node_modules/

# misc
/.env*
Expand Down
5 changes: 4 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ language: node_js
node_js:
# we recommend testing addons with the same minimum supported node version as Ember CLI
# so that your addon works for all apps
- "6"
- "10"

sudo: false
dist: trusty
Expand Down Expand Up @@ -57,5 +57,8 @@ before_install:
- npm install -g npm@4
- npm --version

install:
- yarn install --no-lockfile --non-interactive

script:
- node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO
16 changes: 9 additions & 7 deletions addon/services/prefetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ if (gte('3.6.0')) {
let changeSet = createPrefetchChangeSet(privateRouter, transition);
if (changeSet.shouldCall) {
for (let i = 0; i < changeSet.for.length; i++) {
let { route, fullParams } = changeSet.for[i];
if (seenRoutes.has(route)) continue;

route._prefetched = new RSVP.Promise(r => {
return r(route.prefetch(fullParams, transition))
// normalize route to always be a promise since lazy loaded
// routes are promises, while preloaded routes are not. We want
// to treat them both the same.
RSVP.resolve(changeSet.for[i]).then(({ route, fullParams }) => {
RSVP.resolve(route).then(resolvedRoute => {
if (seenRoutes.has(resolvedRoute) || transition.isAborted) return;
route._prefetched = RSVP.resolve(resolvedRoute.prefetch(fullParams, transition));
seenRoutes.set(resolvedRoute, true);
});
seenRoutes.set(route, true);
if (transition.isAborted) return;
});
}
}
}
Expand Down
1 change: 1 addition & 0 deletions config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = function() {
getChannelURL('canary')
]).then((urls) => {
return {
useYarn: true,
scenarios: [
{
name: 'ember-lts-2.18',
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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 @@ -38,6 +39,7 @@
"ember-try": "^1.0.0",
"eslint-plugin-ember": "^5.2.0",
"eslint-plugin-node": "^7.0.1",
"lazy-engine": "link:./tests/dummy/lib/lazy-engine",
"loader.js": "^4.7.0",
"qunit-dom": "^0.8.0"
},
Expand Down
13 changes: 13 additions & 0 deletions tests/acceptance/lazy-engine-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
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');

assert.equal(currentURL(), '/lazy-engine');
});
});
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() {
// Define your engine's route map here
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{outlet}}
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": "*"
}
}
Loading

0 comments on commit ab03a39

Please sign in to comment.