Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1397,9 +1397,15 @@ function findRouteStateName(route: Route, state: string) {
return routeHasBeenDefined(owner, router, stateName, stateNameFull) ? stateNameFull : '';
}

function isPromise(p: any): boolean {
return p !== null && typeof p === 'object' && typeof p.then === 'function';
}

/**
Determines whether or not a route has been defined by checking that the route
is in the Router's map and the owner has a registration for that route.
Determines whether or not a route has been defined by checking
- that the route is in the Router's map and
- the owner has a registration for that route and
- it has been fully resolved (think of aync assets loading)

@private
@param {Owner} owner
Expand All @@ -1410,9 +1416,15 @@ function findRouteStateName(route: Route, state: string) {
*/
function routeHasBeenDefined(owner: Owner, router: any, localName: string, fullName: string) {
let routerHasRoute = router.hasRoute(fullName);
let ownerHasRoute =
owner.hasRegistration(`template:${localName}`) || owner.hasRegistration(`route:${localName}`);
return routerHasRoute && ownerHasRoute;

if (routerHasRoute && !isPromise(router.getRoute(fullName))) {
let ownerHasRoute =
owner.hasRegistration(`template:${localName}`) || owner.hasRegistration(`route:${localName}`);

return ownerHasRoute;
}

return false;
}

export function triggerEvent(
Expand Down
91 changes: 34 additions & 57 deletions packages/ember/tests/routing/model_loading_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@
import { Route } from '@ember/-internals/routing';
import Controller from '@ember/controller';
import { Object as EmberObject, A as emberA } from '@ember/-internals/runtime';
import { moduleFor, ApplicationTestCase, getTextOf } from 'internal-test-helpers';
import {
moduleFor,
ApplicationTestCase,
getTextOf,
runLoopSettled,
lazyLoadingRouterOptions,
} from 'internal-test-helpers';
import { run } from '@ember/runloop';
import { computed, set } from '@ember/-internals/metal';
import { isDestroying } from '@glimmer/destroyable';
import RSVP from 'rsvp';

let originalConsoleError;

Expand Down Expand Up @@ -502,8 +506,6 @@ class LoadingTests extends ApplicationTestCase {
})
);

this.add('route:loading', Route.extend({}));
this.add('route:home', Route.extend({}));
this.add(
'route:special',
Route.extend({
Expand All @@ -518,7 +520,6 @@ class LoadingTests extends ApplicationTestCase {

this.addTemplate('root.index', '<h3>Home</h3>');
this.addTemplate('special', '<p>{{@model.id}}</p>');
this.addTemplate('loading', '<p>LOADING!</p>');

return this.visit('/').then(() => {
rootElement = document.getElementById('qunit-fixture');
Expand Down Expand Up @@ -755,7 +756,7 @@ class LoadingTests extends ApplicationTestCase {
});
}

['@test Parent route context change'](assert) {
async ['@test Parent route context change'](assert) {
let editCount = 0;
let editedPostIds = emberA();

Expand All @@ -777,8 +778,8 @@ class LoadingTests extends ApplicationTestCase {
'route:posts',
Route.extend({
actions: {
showPost(context) {
expectDeprecation(() => {
async showPost(context) {
await expectDeprecationAsync(() => {
this.transitionTo('post', context);
}, /Calling transitionTo on a route is deprecated/);
},
Expand All @@ -798,8 +799,8 @@ class LoadingTests extends ApplicationTestCase {
},

actions: {
editPost() {
expectDeprecation(() => {
async editPost() {
await expectDeprecationAsync(() => {
this.transitionTo('post.edit');
}, /Calling transitionTo on a route is deprecated/);
},
Expand All @@ -815,22 +816,26 @@ class LoadingTests extends ApplicationTestCase {
editedPostIds.push(postId);
return null;
},

setup() {
this._super(...arguments);
editCount++;
},
})
);

return this.visit('/posts/1').then(() => {
assert.ok(true, '/posts/1 has been handled');
let router = this.applicationInstance.lookup('router:main');
run(() => router.send('editPost'));
run(() => router.send('showPost', { id: '2' }));
run(() => router.send('editPost'));
assert.equal(editCount, 2, 'set up the edit route twice without failure');
assert.deepEqual(editedPostIds, ['1', '2'], 'modelFor posts.post returns the right context');
});
await this.visit('/posts/1');
assert.ok(true, '/posts/1 has been handled');
let router = this.applicationInstance.lookup('router:main');
run(() => router.send('editPost'));
await runLoopSettled();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rwjblue I have doubt about that artificial synchronisation

await runLoopSettled();
run(() => router.send('showPost', { id: '2' }));
await runLoopSettled();
run(() => router.send('editPost'));
await runLoopSettled();
assert.equal(editCount, 2, 'set up the edit route twice without failure');
assert.deepEqual(editedPostIds, ['1', '2'], 'modelFor posts.post returns the right context');
}

['@test ApplicationRoute with model does not proxy the currentPath'](assert) {
Expand Down Expand Up @@ -941,6 +946,14 @@ class LoadingTests extends ApplicationTestCase {
assert.equal(childcount, 2);
});
}

['@test What about loading states'](assert) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a reminder to me, to remove this test

assert.expect(1);
this.add('route:loading', Route.extend({}));
return this.visit('/').then(() => {
assert.ok(true);
});
}
}

moduleFor('Route - model loading', LoadingTests);
Expand All @@ -949,43 +962,7 @@ moduleFor(
'Route - model loading (simulated within lazy engine)',
class extends LoadingTests {
get routerOptions() {
return {
location: 'none',
setupRouter() {
this._super(...arguments);
let getRoute = this._routerMicrolib.getRoute;
this._enginePromises = Object.create(null);
this._resolvedEngines = Object.create(null);

let routes = new Map();
let routePromises = new Map();
this._routerMicrolib.getRoute = (name) => {
if (routes.has(name)) {
return routes.get(name);
}

if (routePromises.has(name)) {
return routePromises.get(name);
}

let promise = new RSVP.Promise((resolve) => {
setTimeout(() => {
if (isDestroying(this)) {
return;
}

let route = getRoute(name);

routes.set(name, route);
resolve(route);
}, 10);
});
routePromises.set(name, promise);

return promise;
};
},
};
return lazyLoadingRouterOptions;
}
}
);
Loading