Skip to content

Commit

Permalink
Refactor router: remove owner presence checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@sly7-7
sly7-7 committed Mar 12, 2021
1 parent b945e5c commit 2bb231a
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 113 deletions.
12 changes: 3 additions & 9 deletions packages/@ember/-internals/routing/lib/system/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,8 @@ class EmberRouter extends EmberObject {
super(...arguments);

this._resetQueuedQueryParameterChanges();
if (owner) {
this.namespace = owner.lookup('application:main');
this._bucketCache = owner.lookup(P`-bucket-cache:main`);
}
this.namespace = owner.lookup('application:main');
this._bucketCache = owner.lookup(P`-bucket-cache:main`);
}

_initRouterJs() {
Expand Down Expand Up @@ -393,10 +391,6 @@ class EmberRouter extends EmberObject {

_hasModuleBasedResolver() {
let owner = getOwner(this);
if (!owner) {
return false;
}

let resolver = get(owner, 'application.__registry__.resolver.moduleBasedResolver');
return Boolean(resolver);
}
Expand Down Expand Up @@ -693,7 +687,7 @@ class EmberRouter extends EmberObject {
let rootURL = this.rootURL;
let owner = getOwner(this);

if ('string' === typeof location && owner) {
if ('string' === typeof location) {
let resolvedLocation = owner.lookup<IEmberLocation>(`location:${location}`);

if (resolvedLocation !== undefined) {
Expand Down
131 changes: 53 additions & 78 deletions packages/@ember/-internals/routing/tests/system/dsl_test.js
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import { setOwner } from '@ember/-internals/owner';
import EmberRouter from '../../lib/system/router';
import { buildOwner, moduleFor, AbstractTestCase } from 'internal-test-helpers';

let Router;

moduleFor(
'Ember Router DSL',
class extends AbstractTestCase {
constructor() {
super();
Router = EmberRouter.extend();
this.Router = class extends EmberRouter {};

this.routerInstance = new this.Router(
buildOwner({
ownerOptions: { routable: true },
})
);
}

teardown() {
Router = null;
this.Router = null;
this.routerInstance = null;
}

['@test should fail when using a reserved route name'](assert) {
Expand All @@ -23,41 +27,37 @@ moduleFor(

reservedNames.forEach((reservedName) => {
expectAssertion(() => {
Router = EmberRouter.extend();
let Router = class extends this.Router {};

Router.map(function () {
this.route(reservedName);
});

let router = Router.create();
router._initRouterJs();
new Router(buildOwner())._initRouterJs();
}, "'" + reservedName + "' cannot be used as a route name.");
});
}

['@test [GH#16642] better error when using a colon in a route name']() {
expectAssertion(() => {
Router = EmberRouter.extend();

Router.map(function () {
this.Router.map(function () {
this.route('resource/:id');
});

let router = Router.create();
router._initRouterJs();
this.routerInstance._initRouterJs();
}, "'resource/:id' is not a valid route name. It cannot contain a ':'. You may want to use the 'path' option instead.");
}

['@test should retain resource namespace if nested with routes'](assert) {
Router = Router.map(function () {
this.Router.map(function () {
this.route('bleep', function () {
this.route('bloop', function () {
this.route('blork');
});
});
});

let router = Router.create();
let router = this.routerInstance;
router._initRouterJs();

assert.ok(
Expand All @@ -75,16 +75,17 @@ moduleFor(
}

['@test should add loading and error routes if _isRouterMapResult is true'](assert) {
Router.map(function () {
this.Router.map(function () {
this.route('blork');
});

let router = Router.create({
this.routerInstance.reopen({
_hasModuleBasedResolver() {
return true;
},
});

let router = this.routerInstance;
router._initRouterJs();

assert.ok(router._routerMicrolib.recognizer.names['blork'], 'main route was created');
Expand All @@ -96,11 +97,11 @@ moduleFor(
}

['@test should not add loading and error routes if _isRouterMapResult is false'](assert) {
Router.map(function () {
this.Router.map(function () {
this.route('blork');
});

let router = Router.create();
let router = this.routerInstance;
router._initRouterJs(false);

assert.ok(router._routerMicrolib.recognizer.names['blork'], 'main route was created');
Expand All @@ -117,19 +118,20 @@ moduleFor(
['@test should reset namespace of loading and error routes for routes with resetNamespace'](
assert
) {
Router.map(function () {
this.Router.map(function () {
this.route('blork', function () {
this.route('blorp');
this.route('bleep', { resetNamespace: true });
});
});

let router = Router.create({
this.routerInstance.reopen({
_hasModuleBasedResolver() {
return true;
},
});

let router = this.routerInstance;
router._initRouterJs();

assert.ok(router._routerMicrolib.recognizer.names['blork.blorp'], 'nested route was created');
Expand Down Expand Up @@ -167,13 +169,13 @@ moduleFor(
}

['@test should throw an error when defining a route serializer outside an engine'](assert) {
Router.map(function () {
this.Router.map(function () {
assert.throws(() => {
this.route('posts', { serialize: function () {} });
}, /Defining a route serializer on route 'posts' outside an Engine is not allowed/);
});

Router.create()._initRouterJs();
this.routerInstance._initRouterJs();
}
}
);
Expand All @@ -183,30 +185,31 @@ moduleFor(
class extends AbstractTestCase {
constructor() {
super();
Router = EmberRouter.extend();
this.Router = class extends EmberRouter {};
this.routerInstance = new this.Router(
buildOwner({
ownerOptions: { routable: true },
})
);
}

teardown() {
Router = null;
this.Router = null;
this.routerInstance = null;
}

['@test should allow mounting of engines'](assert) {
assert.expect(3);

Router = Router.map(function () {
this.Router.map(function () {
this.route('bleep', function () {
this.route('bloop', function () {
this.mount('chat');
});
});
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create();
setOwner(router, engineInstance);
let router = this.routerInstance;
router._initRouterJs();

assert.ok(
Expand All @@ -226,20 +229,15 @@ moduleFor(
['@test should allow mounting of engines at a custom path'](assert) {
assert.expect(1);

Router = Router.map(function () {
this.Router.map(function () {
this.route('bleep', function () {
this.route('bloop', function () {
this.mount('chat', { path: 'custom-chat' });
});
});
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create();
setOwner(router, engineInstance);
let router = this.routerInstance;
router._initRouterJs();

assert.deepEqual(
Expand All @@ -254,20 +252,15 @@ moduleFor(
['@test should allow aliasing of engine names with `as`'](assert) {
assert.expect(1);

Router = Router.map(function () {
this.Router.map(function () {
this.route('bleep', function () {
this.route('bloop', function () {
this.mount('chat', { as: 'blork' });
});
});
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create();
setOwner(router, engineInstance);
let router = this.routerInstance;
router._initRouterJs();

assert.deepEqual(
Expand All @@ -280,22 +273,17 @@ moduleFor(
}

['@test should add loading and error routes to a mount if _isRouterMapResult is true'](assert) {
Router.map(function () {
this.Router.map(function () {
this.mount('chat');
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create({
this.routerInstance.reopen({
_hasModuleBasedResolver() {
return true;
},
});
setOwner(router, engineInstance);
router._initRouterJs();

this.routerInstance._initRouterJs();
let router = this.routerInstance;
assert.ok(router._routerMicrolib.recognizer.names['chat'], 'main route was created');
assert.ok(router._routerMicrolib.recognizer.names['chat_loading'], 'loading route was added');
assert.ok(router._routerMicrolib.recognizer.names['chat_error'], 'error route was added');
Expand All @@ -304,21 +292,17 @@ moduleFor(
['@test should add loading and error routes to a mount alias if _isRouterMapResult is true'](
assert
) {
Router.map(function () {
this.Router.map(function () {
this.mount('chat', { as: 'shoutbox' });
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create({
this.routerInstance.reopen({
_hasModuleBasedResolver() {
return true;
},
});
setOwner(router, engineInstance);
router._initRouterJs();
this.routerInstance._initRouterJs();
let router = this.routerInstance;

assert.ok(router._routerMicrolib.recognizer.names['shoutbox'], 'main route was created');
assert.ok(
Expand All @@ -331,16 +315,11 @@ moduleFor(
['@test should not add loading and error routes to a mount if _isRouterMapResult is false'](
assert
) {
Router.map(function () {
this.Router.map(function () {
this.mount('chat');
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create();
setOwner(router, engineInstance);
let router = this.routerInstance;
router._initRouterJs(false);

assert.ok(router._routerMicrolib.recognizer.names['chat'], 'main route was created');
Expand All @@ -357,24 +336,20 @@ moduleFor(
['@test should reset namespace of loading and error routes for mounts with resetNamespace'](
assert
) {
Router.map(function () {
this.Router.map(function () {
this.route('news', function () {
this.mount('chat');
this.mount('blog', { resetNamespace: true });
});
});

let engineInstance = buildOwner({
ownerOptions: { routable: true },
});

let router = Router.create({
this.routerInstance.reopen({
_hasModuleBasedResolver() {
return true;
},
});
setOwner(router, engineInstance);
router._initRouterJs();
this.routerInstance._initRouterJs();
let router = this.routerInstance;

assert.ok(router._routerMicrolib.recognizer.names['news.chat'], 'nested route was created');
assert.ok(
Expand Down
Loading

0 comments on commit 2bb231a

Please sign in to comment.