Skip to content

Commit

Permalink
# This is a combination of 2 commits.
Browse files Browse the repository at this point in the history
# This is the 1st commit message:

Distinguish `Factory` and `InternalFactory`

Like with our distinction between `Owner` and `InternalOwner`, the
notion of a `Factory` as we use it internally is somewhat richer than
what we provide as a public interface. Thus, add an `InternalFactory`
type which extends the public `Factory` interface, and update the
internals which previously referred to `Factory` to refer to the new
type instead.

In a future cleanup pass, we can likely remove many of these uses of
`InternalFactory` in favor of using *only* the public API for `Factory`
from instead, and that will help smooth the path to removing many of
these unnecessary extra details.

# The commit message #2 will be skipped:

# InternalFactory needs Factory :sigh:
  • Loading branch information
chriskrycho committed Nov 17, 2022
1 parent 50e6aac commit f41b3c6
Show file tree
Hide file tree
Showing 12 changed files with 83 additions and 48 deletions.
15 changes: 9 additions & 6 deletions packages/@ember/-internals/container/lib/container.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {
Factory,
InternalFactory,
FactoryClass,
InternalOwner,
RegisterOptions,
Expand Down Expand Up @@ -145,7 +145,10 @@ export default class Container {
@param {RegisterOptions} [options]
@return {any}
*/
lookup(fullName: string, options?: RegisterOptions): Factory<object> | object | undefined {
lookup(
fullName: string,
options?: RegisterOptions
): InternalFactory<object> | object | undefined {
if (this.isDestroyed) {
throw new Error(`Cannot call \`.lookup\` after the owner has been destroyed`);
}
Expand Down Expand Up @@ -268,8 +271,8 @@ function isInstantiatable(container: Container, fullName: string) {
function lookup(
container: Container,
fullName: string,
): Factory<object> | object | undefined {
options: RegisterOptions = {}
): InternalFactory<object> | object | undefined {
let normalizedName = fullName;

if (
Expand Down Expand Up @@ -370,8 +373,8 @@ function instantiateFactory(
container: Container,
normalizedName: string,
fullName: string,
): Factory<object> | object | undefined {
options: RegisterOptions
): InternalFactory<object> | object | undefined {
let factoryManager = factoryFor(container, normalizedName, fullName);

if (factoryManager === undefined) {
Expand Down Expand Up @@ -450,7 +453,7 @@ export interface LazyInjection {
}

declare interface DebugFactory<T extends object, C extends FactoryClass | object = FactoryClass>
extends Factory<T, C> {
extends InternalFactory<T, C> {
_onLookup?: (fullName: string) => void;
_initFactory?: (factoryManager: FactoryManager<T, C>) => void;
_lazyInjections(): { [key: string]: LazyInjection };
Expand All @@ -477,7 +480,7 @@ export class FactoryManager<T extends object, C extends FactoryClass | object =

constructor(
container: Container,
factory: Factory<T, C>,
factory: InternalFactory<T, C>,
fullName: string,
normalizedName: string
) {
Expand Down
18 changes: 9 additions & 9 deletions packages/@ember/-internals/container/lib/registry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Factory, Resolver, RegisterOptions } from '@ember/-internals/owner';
import type { InternalFactory, Resolver, RegisterOptions } from '@ember/-internals/owner';
import { dictionary, intern } from '@ember/-internals/utils';
import { assert, deprecate } from '@ember/debug';
import type { set } from '@ember/object';
Expand Down Expand Up @@ -58,10 +58,10 @@ export default class Registry implements IRegistry {
readonly _failSet: Set<string>;
resolver: Resolver | null;
readonly fallback: IRegistry | null;
readonly registrations: Record<string, Factory<object> | object>;
readonly _normalizeCache: Record<string, string>;
readonly _resolveCache: Record<string, Factory<object> | object>;
readonly registrations: Record<string, InternalFactory<object> | object>;
readonly _options: Record<string, RegisterOptions>;
readonly _resolveCache: Record<string, InternalFactory<object> | object>;
readonly _typeOptions: Record<string, RegisterOptions>;

set?: typeof set;
Expand Down Expand Up @@ -168,12 +168,12 @@ export default class Registry implements IRegistry {
): void;
register<T extends object, C extends FactoryClass | object>(
fullName: string,
factory: Factory<T, C>,
factory: InternalFactory<T, C>,
options?: RegisterOptions
): void;
register(
fullName: string,
factory: object | Factory<object>,
factory: object | InternalFactory<object>,
options: RegisterOptions = {}
): void {
assert('fullName must be a proper full name', this.isValidFullName(fullName));
Expand Down Expand Up @@ -252,7 +252,7 @@ export default class Registry implements IRegistry {
@param {String} fullName
@return {Function} fullName's factory
*/
resolve(fullName: string): Factory<object> | object | undefined {
resolve(fullName: string): InternalFactory<object> | object | undefined {
let factory = resolve(this, this.normalize(fullName));
if (factory === undefined && this.fallback !== null) {
factory = this.fallback.resolve(fullName);
Expand Down Expand Up @@ -324,7 +324,7 @@ export default class Registry implements IRegistry {
@param {string} fullName
@return {function} toString function
*/
makeToString(factory: Factory<object>, fullName: string): string {
makeToString(factory: InternalFactory<object>, fullName: string): string {
if (this.resolver !== null && this.resolver.makeToString) {
return this.resolver.makeToString(factory, fullName);
} else if (this.fallback !== null) {
Expand Down Expand Up @@ -545,7 +545,7 @@ if (DEBUG) {
function resolve(
registry: Registry,
_normalizedName: string
): Factory<object> | object | undefined {
): InternalFactory<object> | object | undefined {
let normalizedName = _normalizedName;

let cached = registry._resolveCache[normalizedName];
Expand All @@ -556,7 +556,7 @@ function resolve(
return;
}

let resolved: Factory<object> | object | undefined;
let resolved: InternalFactory<object> | object | undefined;

if (registry.resolver) {
resolved = registry.resolver.resolve(normalizedName);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getOwner, setOwner } from '@ember/-internals/owner';
import type { Factory, default as Owner } from '@ember/-internals/owner';
import type { InternalFactory, default as Owner } from '@ember/-internals/owner';
import { enumerableSymbol, guidFor } from '@ember/-internals/utils';
import { addChildView, setElementView, setViewElement } from '@ember/-internals/views';
import { assert, debugFreeze } from '@ember/debug';
Expand Down Expand Up @@ -104,7 +104,7 @@ const EMPTY_POSITIONAL_ARGS: Reference[] = [];

debugFreeze(EMPTY_POSITIONAL_ARGS);

type ComponentFactory = Factory<
type ComponentFactory = InternalFactory<
Component,
{
create(props?: any): Component;
Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/glimmer/lib/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import type { FactoryManager } from '@ember/-internals/container/lib/container';
import type { Factory, InternalOwner } from '@ember/-internals/owner';
import type { InternalFactory, InternalOwner } from '@ember/-internals/owner';
import { setOwner } from '@ember/-internals/owner';
import { FrameworkObject } from '@ember/object/-internals';
import { getDebugName } from '@ember/-internals/utils';
Expand All @@ -21,11 +21,11 @@ export type HelperFunction<T, P extends unknown[], N extends Dict<unknown>> = (
named: N
) => T;

export type SimpleHelperFactory<T, P extends unknown[], N extends Dict<unknown>> = Factory<
export type SimpleHelperFactory<T, P extends unknown[], N extends Dict<unknown>> = InternalFactory<
SimpleHelper<T, P, N>,
HelperFactory<SimpleHelper<T, P, N>>
>;
export type ClassHelperFactory = Factory<HelperInstance, HelperFactory<HelperInstance>>;
export type ClassHelperFactory = InternalFactory<HelperInstance, HelperFactory<HelperInstance>>;

export interface HelperFactory<T> {
isHelperFactory: true;
Expand Down
11 changes: 7 additions & 4 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { privatize as P } from '@ember/-internals/container';
import { ENV } from '@ember/-internals/environment';
import type { Factory, InternalOwner, RegisterOptions } from '@ember/-internals/owner';
import type { InternalFactory, InternalOwner, RegisterOptions } from '@ember/-internals/owner';
import { isFactory } from '@ember/-internals/owner';
import { EMBER_UNIQUE_ID_HELPER } from '@ember/canary-features';
import { assert } from '@ember/debug';
Expand Down Expand Up @@ -55,7 +55,10 @@ function instrumentationPayload(name: string) {
}

let fullName = `component:${name}`;
function componentFor(name: string, owner: InternalOwner): Option<Factory<object> | object> {
function componentFor(
name: string,
owner: InternalOwner
): Option<InternalFactory<object> | object> {
return owner.factoryFor(fullName) || null;
}

Expand All @@ -71,11 +74,11 @@ function layoutFor(

type LookupResult =
| {
component: Factory<object>;
component: InternalFactory<object>;
layout: TemplateFactory;
}
| {
component: Factory<object>;
component: InternalFactory<object>;
layout: null;
}
| {
Expand Down
36 changes: 31 additions & 5 deletions packages/@ember/-internals/owner/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ export interface RegisterOptions {
singleton?: boolean | undefined;
}

/**
* Registered factories are instantiated by having create called on them.
* Additionally they are singletons by default, so each time they are looked up
* they return the same instance.
*
* However, that behavior can be modified with the `instantiate` and `singleton`
* options to the {@linkcode Owner.register} method.
*/
export interface Factory<T extends object> {
// NOTE: this does not check against the types of the target object in any
// way, unfortunately. However, we actually *cannot* constrain it further than
// this without going down a *very* deep rabbit hole (see the historic types
// for `.create()` on DefinitelyTyped if you're curious), because we need (for
// historical reasons) to support classes which implement this contract to be
// able to provide a *narrower* interface than "exactly the public fields on
// the class" while still falling back to the "exactly the public fields on
// the class" for the general case. :sigh:
/**
* A function that will create an instance of the class with any
* dependencies injected.
*
* @param initialValues Any values to set on an instance of the class
*/
create(initialValues?: object): T;
}

/**
* A `Resolver` is the mechanism responsible for looking up code in your
* application and converting its naming conventions into the actual classes,
Expand All @@ -63,16 +89,16 @@ export interface FactoryClass {
positionalParams?: string | string[] | undefined | null;
}

export interface Factory<T extends object, C extends FactoryClass | object = FactoryClass> {
export interface InternalFactory<T extends object, C extends FactoryClass | object = FactoryClass>
extends Factory<T> {
class?: C;
name?: string;
fullName?: string;
fullName?: FullName;
normalizedName?: string;
create(props?: { [prop: string]: any }): T;
}

export function isFactory(obj: unknown): obj is Factory<object> {
return obj != null && typeof (obj as Factory<object>).create === 'function';
export function isFactory(obj: unknown): obj is InternalFactory<object> {
return obj != null && typeof (obj as InternalFactory<object>).create === 'function';
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/routing/lib/dsl.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Factory } from '@ember/-internals/owner';
import type { InternalFactory } from '@ember/-internals/owner';
import { assert } from '@ember/debug';
import type { Option } from '@glimmer/interfaces';
import type { MatchCallback } from 'route-recognizer';
Expand Down Expand Up @@ -44,7 +44,7 @@ export interface DSLImplOptions {
enableLoadingSubstates: boolean;
engineInfo?: EngineInfo;
addRouteForEngine(name: string, routeOptions: EngineRouteInfo): void;
resolveRouteMap(name: string): Factory<any, any>;
resolveRouteMap(name: string): InternalFactory<any, any>;
}

export default class DSLImpl implements DSL {
Expand Down
9 changes: 6 additions & 3 deletions packages/@ember/routing/lib/generate_controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { get } from '@ember/-internals/metal';
import type { Factory, default as Owner } from '@ember/-internals/owner';
import type { InternalFactory, default as Owner } from '@ember/-internals/owner';
import Controller from '@ember/controller';
import { assert, info } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
Expand All @@ -16,7 +16,10 @@ import { DEBUG } from '@glimmer/env';
@private
*/

export function generateControllerFactory(owner: Owner, controllerName: string): Factory<{}> {
export function generateControllerFactory(
owner: Owner,
controllerName: string
): InternalFactory<{}> {
let factoryManager = owner.factoryFor('controller:basic');
assert(
'[BUG] unexpectedly missing a factoryManager for `controller:basic`',
Expand All @@ -39,7 +42,7 @@ export function generateControllerFactory(owner: Owner, controllerName: string):

owner.register(fullName, Factory);

return owner.factoryFor(fullName) as Factory<{}>;
return owner.factoryFor(fullName) as InternalFactory<{}>;
}

/**
Expand Down
12 changes: 6 additions & 6 deletions packages/internal-test-helpers/lib/test-cases/rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ModuleBasedResolver } from '../test-resolver';
import AbstractTestCase from './abstract';
import buildOwner from '../build-owner';
import { runAppend, runDestroy, runTask } from '../run';
import type { Factory } from '@ember/-internals/owner';
import type { InternalFactory } from '@ember/-internals/owner';
import type { BootOptions, EngineInstanceOptions } from '@ember/engine/instance';
import type EngineInstance from '@ember/engine/instance';
import type { HelperFunction } from '@ember/-internals/glimmer/lib/helper';
Expand Down Expand Up @@ -75,7 +75,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase {
return new ModuleBasedResolver();
}

add(specifier: string, factory: Factory<object> | object) {
add(specifier: string, factory: InternalFactory<object> | object) {
this.resolver.add(specifier, factory);
}

Expand Down Expand Up @@ -173,7 +173,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase {
}
}

registerCustomHelper(name: string, definition: Factory<object>) {
registerCustomHelper(name: string, definition: InternalFactory<object>) {
this.owner.register(`helper:${name}`, definition);
}

Expand All @@ -194,13 +194,13 @@ export default abstract class RenderingTestCase extends AbstractTestCase {
}
}

registerModifier(name: string, ModifierClass: Factory<object>) {
registerModifier(name: string, ModifierClass: InternalFactory<object>) {
let { owner } = this;

owner.register(`modifier:${name}`, ModifierClass);
}

registerComponentManager(name: string, manager: Factory<object>) {
registerComponentManager(name: string, manager: InternalFactory<object>) {
let owner = this.owner;
owner.register(`component-manager:${name}`, manager);
}
Expand All @@ -219,7 +219,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase {
}
}

registerService(name: string, klass: Factory<object>) {
registerService(name: string, klass: InternalFactory<object>) {
this.owner.register(`service:${name}`, klass);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import buildOwner from '../build-owner';
import { runAppend, runDestroy } from '../run';
import type { BootOptions, EngineInstanceOptions } from '@ember/engine/instance';
import type EngineInstance from '@ember/engine/instance';
import type { Factory } from '@ember/-internals/owner';
import type { InternalFactory } from '@ember/-internals/owner';

export default class RouterNonApplicationTestCase extends AbstractTestCase {
owner: EngineInstance;
Expand Down Expand Up @@ -59,7 +59,7 @@ export default class RouterNonApplicationTestCase extends AbstractTestCase {
return new ModuleBasedResolver();
}

add(specifier: string, factory: Factory<object> | object) {
add(specifier: string, factory: InternalFactory<object> | object) {
this.resolver.add(specifier, factory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import AbstractApplicationTestCase from './abstract-application';
import type Resolver from '../test-resolver';
import { ModuleBasedResolver } from '../test-resolver';
import Component from '@ember/component';
import type { Factory } from '@ember/-internals/owner';
import type { InternalFactory } from '@ember/-internals/owner';

export default abstract class TestResolverApplicationTestCase extends AbstractApplicationTestCase {
abstract resolver?: Resolver;
Expand All @@ -13,7 +13,7 @@ export default abstract class TestResolverApplicationTestCase extends AbstractAp
});
}

add(specifier: string, factory: Factory<object> | object) {
add(specifier: string, factory: InternalFactory<object> | object) {
this.resolver!.add(specifier, factory);
}

Expand Down
Loading

0 comments on commit f41b3c6

Please sign in to comment.