Skip to content

Commit

Permalink
Merge pull request #19967 from wagenet/refactor-owner
Browse files Browse the repository at this point in the history
Refactor owner
  • Loading branch information
wagenet authored Feb 11, 2022
2 parents bd6bad0 + bd02b0f commit 8455848
Show file tree
Hide file tree
Showing 25 changed files with 181 additions and 103 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Owner } from '@ember/-internals/owner';
import { generateControllerFactory } from '@ember/-internals/routing';
import { assert } from '@ember/debug';
import EngineInstance from '@ember/engine/instance';
import { associateDestroyableChild } from '@glimmer/destroyable';
import {
Expand Down Expand Up @@ -74,6 +75,7 @@ class MountManager
// we should resolve the engine app template in the helper
// it also should use the owner that looked up the mount helper.

assert('Expected owner to be an EngineInstance', owner instanceof EngineInstance);
let engine = owner.buildChildEngineInstance(name);

engine.boot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,14 @@ class OutletComponentManager
let currentOwner = valueForRef(currentStateRef)!.render!.owner;

if (parentOwner && parentOwner !== currentOwner) {
let engine = currentOwner as EngineInstance;
assert(
'Expected currentOwner to be an EngineInstance',
currentOwner instanceof EngineInstance
);

assert('invalid engine: missing mountPoint', typeof currentOwner.mountPoint === 'string');
assert('invalid engine: missing routable', currentOwner.routable === true);
let mountPoint = currentOwner.mountPoint;

let mountPoint = engine.mountPoint!;

state.engine = engine;
state.engine = currentOwner;
state.engineBucket = { mountPoint };
}
}
Expand Down
7 changes: 4 additions & 3 deletions packages/@ember/-internals/glimmer/lib/components/link-to.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Owner } from '@ember/-internals/owner';
import { Route, RouterState, RoutingService } from '@ember/-internals/routing';
import { isSimpleClick } from '@ember/-internals/views';
import { assert, debugFreeze, inspect, warn } from '@ember/debug';
Expand Down Expand Up @@ -490,11 +489,13 @@ class LinkTo extends InternalComponent {
}

private get isEngine(): boolean {
return getEngineParent(this.owner as EngineInstance) !== undefined;
let owner = this.owner;
return owner instanceof EngineInstance && getEngineParent(owner) !== undefined;
}

private get engineMountPoint(): string | undefined {
return (this.owner as Owner | EngineInstance).mountPoint;
let owner = this.owner;
return owner instanceof EngineInstance ? owner.mountPoint : undefined;
}

private classFor(state: 'active' | 'loading' | 'disabled'): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import {

import { ENV } from '@ember/-internals/environment';
import { Component, setComponentManager } from '@ember/-internals/glimmer';
import { EngineInstanceOptions, Owner } from '@ember/-internals/owner';
import { Owner } from '@ember/-internals/owner';
import { Route } from '@ember/-internals/routing';
import Controller from '@ember/controller';
import { assert, captureRenderTree } from '@ember/debug';
import Engine from '@ember/engine';
import EngineInstance from '@ember/engine/instance';
import EngineInstance, { EngineInstanceOptions } from '@ember/engine/instance';
import { CapturedRenderNode } from '@glimmer/interfaces';
import { componentCapabilities, setComponentTemplate } from '@glimmer/manager';
import { templateOnlyComponent } from '@glimmer/runtime';
Expand Down
34 changes: 19 additions & 15 deletions packages/@ember/-internals/owner/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { getOwner as glimmerGetOwner, setOwner as glimmerSetOwner } from '@glimmer/owner';
import { TypeOptions } from '../container/lib/registry';

/**
@module @ember/application
*/

export { TypeOptions };

export interface FactoryClass {
positionalParams?: string | string[] | undefined | null;
}
Expand All @@ -16,25 +19,26 @@ export interface Factory<T, C extends FactoryClass | object = FactoryClass> {
create(props?: { [prop: string]: any }): T;
}

export interface EngineInstanceOptions {
mountPoint: string;
routable: boolean;
}

import EngineInstance from '@ember/engine/instance';
import { TypeOptions } from '../container/lib/registry';
// A combination of the public methods on ContainerProxyMixin and RegistryProxyMixin
export interface Owner {
// From ContainerProxy
ownerInjection(): void;
lookup(fullName: string, options?: TypeOptions): unknown;
factoryFor(fullName: string): Factory<unknown> | undefined;
register(fullName: string, factory: Factory<unknown>, options?: TypeOptions): void;
hasRegistration(name: string): boolean;

/** @internal */
mountPoint?: string;
/** @internal */
routable?: boolean;
/** @internal */
buildChildEngineInstance(name: string, options?: EngineInstanceOptions): EngineInstance;
// From RegistryProxy
resolveRegistration<T, C>(fullName: string): Factory<T, C> | undefined;
register(fullName: string, factory: Factory<unknown>, options?: TypeOptions): void;
unregister(fullName: string): void;
hasRegistration(fullName: string): boolean;
registeredOption<K extends keyof TypeOptions>(
fullName: string,
optionName: K
): TypeOptions[K] | undefined;
registerOptions(fullName: string, options: TypeOptions): void;
registeredOptions(fullName: string): TypeOptions | undefined;
registerOptionsForType(type: string, options: TypeOptions): void;
registeredOptionsForType(type: string): TypeOptions | undefined;
}

/**
Expand Down
7 changes: 4 additions & 3 deletions packages/@ember/-internals/routing/lib/system/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
import { isProxy, lookupDescriptor, symbol } from '@ember/-internals/utils';
import Controller from '@ember/controller';
import { assert, info, isTesting } from '@ember/debug';
import EngineInstance from '@ember/engine/instance';
import { dependentKeyCompat } from '@ember/object/compat';
import { once } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
Expand Down Expand Up @@ -548,7 +549,7 @@ class Route<T = unknown>
_setRouteName(name: string) {
this.routeName = name;
let owner = getOwner(this);
assert('Route is unexpectedly missing an owner', owner);
assert('Expected route to have EngineInstance as owner', owner instanceof EngineInstance);
this.fullRouteName = getEngineRouteName(owner, name)!;
}

Expand Down Expand Up @@ -1684,7 +1685,7 @@ class Route<T = unknown>
modelFor(_name: string): unknown | undefined {
let name;
let owner = getOwner(this);
assert('Route is unexpectedly missing an owner', owner);
assert('Expected router owner to be an EngineInstance', owner instanceof EngineInstance);
let transition =
this._router && this._router._routerMicrolib
? this._router._routerMicrolib.activeTransition
Expand Down Expand Up @@ -2338,7 +2339,7 @@ function addQueryParamsObservers(controller: any, propNames: string[]) {
});
}

function getEngineRouteName(engine: Owner, routeName: string) {
function getEngineRouteName(engine: EngineInstance, routeName: string) {
if (engine.routable) {
let prefix = engine.mountPoint;

Expand Down
8 changes: 2 additions & 6 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import Router, {
TransitionState,
} from 'router_js';
import { EngineRouteInfo } from './engines';
import EngineInstance from '@ember/engine/instance';

function defaultDidTransition<R extends Route>(
this: EmberRouter<R>,
Expand Down Expand Up @@ -112,11 +113,6 @@ interface OutletState<T extends RenderOutletState = RenderOutletState> {
wasUsed?: boolean;
}

interface EngineInstance extends Owner {
boot(): void;
destroy(): void;
}

export interface QueryParam {
prop: string;
urlKey: string;
Expand Down Expand Up @@ -1408,7 +1404,7 @@ class EmberRouter<R extends Route = Route> extends EmberObject.extend(Evented) i

if (!engineInstance) {
let owner = getOwner(this);
assert('Router is unexpectedly missing an owner', owner);
assert('Expected router to have EngineInstance as owner', owner instanceof EngineInstance);

assert(
`You attempted to mount the engine '${name}' in your router map, but the engine can not be found.`,
Expand Down
3 changes: 2 additions & 1 deletion packages/@ember/-internals/routing/lib/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { get } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
import { assert, deprecate } from '@ember/debug';
import EngineInstance from '@ember/engine/instance';
import EmberError from '@ember/error';
import Router, { STATE_SYMBOL, InternalRouteInfo, ModelFor } from 'router_js';
import Route from './system/route';
Expand Down Expand Up @@ -247,7 +248,7 @@ export function prefixRouteNameArg<T extends NamedRouteArgs<Route> | UnnamedRout
): T {
let routeName: string;
let owner = getOwner(route);
assert('Route is unexpectedly missing an owner', owner);
assert('Expected route to have EngineInstance as owner', owner instanceof EngineInstance);

let prefix = owner.mountPoint;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { setOwner } from '@ember/-internals/owner';
import Controller from '@ember/controller';
import { buildOwner, moduleFor, AbstractTestCase } from 'internal-test-helpers';
import { buildOwner, moduleFor, runDestroy, AbstractTestCase } from 'internal-test-helpers';

moduleFor(
'@ember/-internals/routing/ext/controller',
Expand Down Expand Up @@ -51,6 +51,8 @@ moduleFor(
'passes query param only transitions through'
);
}, /Calling transitionToRoute on a controller is deprecated/);

runDestroy(engineInstance);
}

["@test replaceRoute considers an engine's mountPoint"](assert) {
Expand Down Expand Up @@ -96,6 +98,8 @@ moduleFor(
'passes query param only transitions through'
);
}, /Calling replaceRoute on a controller is deprecated/);

runDestroy(engineInstance);
}
}
);
29 changes: 17 additions & 12 deletions packages/@ember/-internals/routing/tests/system/dsl_test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import EmberRouter from '../../lib/system/router';
import { buildOwner, moduleFor, AbstractTestCase } from 'internal-test-helpers';
import { buildOwner, moduleFor, runDestroy, AbstractTestCase } from 'internal-test-helpers';

moduleFor(
'Ember Router DSL',
Expand All @@ -8,19 +8,20 @@ moduleFor(
super();
this.Router = class extends EmberRouter {};

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

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

['@test should fail when using a reserved route name'](assert) {
let owners = [];
let reservedNames = ['basic', 'application'];

assert.expect(reservedNames.length);
Expand All @@ -33,9 +34,13 @@ moduleFor(
this.route(reservedName);
});

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

owners.forEach((o) => runDestroy(o));
}

['@test [GH#16642] better error when using a colon in a route name']() {
Expand Down Expand Up @@ -186,14 +191,14 @@ moduleFor(
constructor() {
super();
this.Router = class extends EmberRouter {};
this.routerInstance = new this.Router(
buildOwner({
ownerOptions: { routable: true },
})
);
this.owner = buildOwner({
ownerOptions: { routable: true },
});
this.routerInstance = new this.Router(this.owner);
}

teardown() {
runDestroy(this.owner);
this.Router = null;
this.routerInstance = null;
}
Expand Down
12 changes: 12 additions & 0 deletions packages/@ember/-internals/routing/tests/system/route_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ moduleFor(
let authService = owner.lookup('service:auth');

assert.equal(authService, appRoute.get('authService'), 'service.auth is injected');

runDestroy(owner);
}
}
);
Expand Down Expand Up @@ -454,6 +456,8 @@ moduleFor(
{ c: 'd' },
'params match for `posts` route in engine'
);

runDestroy(engineInstance);
}

["@test modelFor considers an engine's mountPoint"](assert) {
Expand Down Expand Up @@ -503,6 +507,8 @@ moduleFor(

assert.strictEqual(route.modelFor('application'), applicationModel);
assert.strictEqual(route.modelFor('posts'), postsModel);

runDestroy(engineInstance);
}

["@test transitionTo considers an engine's mountPoint"](assert) {
Expand Down Expand Up @@ -548,6 +554,8 @@ moduleFor(
'passes query param only transitions through'
);
}, /Calling transitionTo on a route is deprecated/);

runDestroy(engineInstance);
}

["@test intermediateTransitionTo considers an engine's mountPoint"](assert) {
Expand Down Expand Up @@ -582,6 +590,8 @@ moduleFor(
let queryParams = {};
route.intermediateTransitionTo(queryParams);
assert.strictEqual(lastRoute, queryParams, 'passes query param only transitions through');

runDestroy(engineInstance);
}

["@test replaceWith considers an engine's mountPoint"](assert) {
Expand Down Expand Up @@ -627,6 +637,8 @@ moduleFor(
'passes query param only transitions through'
);
}, /Calling replaceWith on a route is deprecated/);

runDestroy(engineInstance);
}
}
);
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { Container } from '@ember/-internals/container';
import { TypeOptions } from '@ember/-internals/container/lib/registry';
import { Factory } from '@ember/-internals/owner';
import Mixin from '../../types/mixin';

interface ContainerProxy {
ownerInjection: Container['ownerInjection'];
lookup: Container['lookup'];
factoryFor: Container['factoryFor'];
/** @internal */
__container__: Container;

ownerInjection(): void;
lookup(fullName: string, options?: TypeOptions): unknown;
factoryFor(fullName: string): Factory<unknown> | undefined;
}
declare const ContainerProxy: Mixin;

Expand Down
Loading

0 comments on commit 8455848

Please sign in to comment.