From 10e4b0384ce28e4bb97c7f726b1d617a26473e21 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Tue, 15 Nov 2022 18:08:42 -0700 Subject: [PATCH] # This is a combination of 2 commits. # 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: --- .../-internals/container/lib/container.ts | 15 ++++---- .../-internals/container/lib/registry.ts | 18 +++++----- .../glimmer/lib/component-managers/curly.ts | 4 +-- .../@ember/-internals/glimmer/lib/helper.ts | 6 ++-- .../@ember/-internals/glimmer/lib/resolver.ts | 11 +++--- packages/@ember/-internals/owner/index.ts | 36 ++++++++++++++++--- packages/@ember/routing/lib/dsl.ts | 4 +-- .../@ember/routing/lib/generate_controller.ts | 9 +++-- .../lib/test-cases/rendering.ts | 12 +++---- .../lib/test-cases/router-non-application.ts | 4 +-- .../test-cases/test-resolver-application.ts | 4 +-- .../lib/test-resolver.ts | 8 ++--- 12 files changed, 83 insertions(+), 48 deletions(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 438859bc52e..0e8977259c1 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -1,5 +1,5 @@ import type { - Factory, + InternalFactory, FactoryClass, InternalOwner, RegisterOptions, @@ -145,7 +145,10 @@ export default class Container { @param {RegisterOptions} [options] @return {any} */ - lookup(fullName: string, options?: RegisterOptions): Factory | object | undefined { + lookup( + fullName: string, + options?: RegisterOptions + ): InternalFactory | object | undefined { if (this.isDestroyed) { throw new Error(`Cannot call \`.lookup\` after the owner has been destroyed`); } @@ -268,8 +271,8 @@ function isInstantiatable(container: Container, fullName: string) { function lookup( container: Container, fullName: string, -): Factory | object | undefined { options: RegisterOptions = {} +): InternalFactory | object | undefined { let normalizedName = fullName; if ( @@ -370,8 +373,8 @@ function instantiateFactory( container: Container, normalizedName: string, fullName: string, -): Factory | object | undefined { options: RegisterOptions +): InternalFactory | object | undefined { let factoryManager = factoryFor(container, normalizedName, fullName); if (factoryManager === undefined) { @@ -450,7 +453,7 @@ export interface LazyInjection { } declare interface DebugFactory - extends Factory { + extends InternalFactory { _onLookup?: (fullName: string) => void; _initFactory?: (factoryManager: FactoryManager) => void; _lazyInjections(): { [key: string]: LazyInjection }; @@ -477,7 +480,7 @@ export class FactoryManager, + factory: InternalFactory, fullName: string, normalizedName: string ) { diff --git a/packages/@ember/-internals/container/lib/registry.ts b/packages/@ember/-internals/container/lib/registry.ts index db07d333cda..e8ae27c06eb 100644 --- a/packages/@ember/-internals/container/lib/registry.ts +++ b/packages/@ember/-internals/container/lib/registry.ts @@ -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'; @@ -58,10 +58,10 @@ export default class Registry implements IRegistry { readonly _failSet: Set; resolver: Resolver | null; readonly fallback: IRegistry | null; - readonly registrations: Record | object>; readonly _normalizeCache: Record; - readonly _resolveCache: Record | object>; + readonly registrations: Record | object>; readonly _options: Record; + readonly _resolveCache: Record | object>; readonly _typeOptions: Record; set?: typeof set; @@ -168,12 +168,12 @@ export default class Registry implements IRegistry { ): void; register( fullName: string, - factory: Factory, + factory: InternalFactory, options?: RegisterOptions ): void; register( fullName: string, - factory: object | Factory, + factory: object | InternalFactory, options: RegisterOptions = {} ): void { assert('fullName must be a proper full name', this.isValidFullName(fullName)); @@ -252,7 +252,7 @@ export default class Registry implements IRegistry { @param {String} fullName @return {Function} fullName's factory */ - resolve(fullName: string): Factory | object | undefined { + resolve(fullName: string): InternalFactory | object | undefined { let factory = resolve(this, this.normalize(fullName)); if (factory === undefined && this.fallback !== null) { factory = this.fallback.resolve(fullName); @@ -324,7 +324,7 @@ export default class Registry implements IRegistry { @param {string} fullName @return {function} toString function */ - makeToString(factory: Factory, fullName: string): string { + makeToString(factory: InternalFactory, fullName: string): string { if (this.resolver !== null && this.resolver.makeToString) { return this.resolver.makeToString(factory, fullName); } else if (this.fallback !== null) { @@ -545,7 +545,7 @@ if (DEBUG) { function resolve( registry: Registry, _normalizedName: string -): Factory | object | undefined { +): InternalFactory | object | undefined { let normalizedName = _normalizedName; let cached = registry._resolveCache[normalizedName]; @@ -556,7 +556,7 @@ function resolve( return; } - let resolved: Factory | object | undefined; + let resolved: InternalFactory | object | undefined; if (registry.resolver) { resolved = registry.resolver.resolve(normalizedName); diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index c9ef1717268..dd7fdeac0a1 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -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'; @@ -104,7 +104,7 @@ const EMPTY_POSITIONAL_ARGS: Reference[] = []; debugFreeze(EMPTY_POSITIONAL_ARGS); -type ComponentFactory = Factory< +type ComponentFactory = InternalFactory< Component, { create(props?: any): Component; diff --git a/packages/@ember/-internals/glimmer/lib/helper.ts b/packages/@ember/-internals/glimmer/lib/helper.ts index 82254766ac2..e65a3792b43 100644 --- a/packages/@ember/-internals/glimmer/lib/helper.ts +++ b/packages/@ember/-internals/glimmer/lib/helper.ts @@ -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'; @@ -21,11 +21,11 @@ export type HelperFunction> = ( named: N ) => T; -export type SimpleHelperFactory> = Factory< +export type SimpleHelperFactory> = InternalFactory< SimpleHelper, HelperFactory> >; -export type ClassHelperFactory = Factory>; +export type ClassHelperFactory = InternalFactory>; export interface HelperFactory { isHelperFactory: true; diff --git a/packages/@ember/-internals/glimmer/lib/resolver.ts b/packages/@ember/-internals/glimmer/lib/resolver.ts index 55e2ede3768..ec9c6427fc6 100644 --- a/packages/@ember/-internals/glimmer/lib/resolver.ts +++ b/packages/@ember/-internals/glimmer/lib/resolver.ts @@ -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'; @@ -55,7 +55,10 @@ function instrumentationPayload(name: string) { } let fullName = `component:${name}`; -function componentFor(name: string, owner: InternalOwner): Option | object> { +function componentFor( + name: string, + owner: InternalOwner +): Option | object> { return owner.factoryFor(fullName) || null; } @@ -71,11 +74,11 @@ function layoutFor( type LookupResult = | { - component: Factory; + component: InternalFactory; layout: TemplateFactory; } | { - component: Factory; + component: InternalFactory; layout: null; } | { diff --git a/packages/@ember/-internals/owner/index.ts b/packages/@ember/-internals/owner/index.ts index e77f498c4ec..862ee46c7bb 100644 --- a/packages/@ember/-internals/owner/index.ts +++ b/packages/@ember/-internals/owner/index.ts @@ -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 { + // 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, @@ -63,16 +89,16 @@ export interface FactoryClass { positionalParams?: string | string[] | undefined | null; } -export interface Factory { +export interface InternalFactory + extends Factory { class?: C; name?: string; - fullName?: string; + fullName?: FullName; normalizedName?: string; - create(props?: { [prop: string]: any }): T; } -export function isFactory(obj: unknown): obj is Factory { - return obj != null && typeof (obj as Factory).create === 'function'; +export function isFactory(obj: unknown): obj is InternalFactory { + return obj != null && typeof (obj as InternalFactory).create === 'function'; } /** diff --git a/packages/@ember/routing/lib/dsl.ts b/packages/@ember/routing/lib/dsl.ts index bad99044a3d..f401e6cf811 100644 --- a/packages/@ember/routing/lib/dsl.ts +++ b/packages/@ember/routing/lib/dsl.ts @@ -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'; @@ -44,7 +44,7 @@ export interface DSLImplOptions { enableLoadingSubstates: boolean; engineInfo?: EngineInfo; addRouteForEngine(name: string, routeOptions: EngineRouteInfo): void; - resolveRouteMap(name: string): Factory; + resolveRouteMap(name: string): InternalFactory; } export default class DSLImpl implements DSL { diff --git a/packages/@ember/routing/lib/generate_controller.ts b/packages/@ember/routing/lib/generate_controller.ts index 81f919e9fef..b1877254490 100644 --- a/packages/@ember/routing/lib/generate_controller.ts +++ b/packages/@ember/routing/lib/generate_controller.ts @@ -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'; @@ -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`', @@ -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<{}>; } /** diff --git a/packages/internal-test-helpers/lib/test-cases/rendering.ts b/packages/internal-test-helpers/lib/test-cases/rendering.ts index 8ae48049978..b73a952824d 100644 --- a/packages/internal-test-helpers/lib/test-cases/rendering.ts +++ b/packages/internal-test-helpers/lib/test-cases/rendering.ts @@ -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'; @@ -75,7 +75,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase { return new ModuleBasedResolver(); } - add(specifier: string, factory: Factory | object) { + add(specifier: string, factory: InternalFactory | object) { this.resolver.add(specifier, factory); } @@ -173,7 +173,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase { } } - registerCustomHelper(name: string, definition: Factory) { + registerCustomHelper(name: string, definition: InternalFactory) { this.owner.register(`helper:${name}`, definition); } @@ -194,13 +194,13 @@ export default abstract class RenderingTestCase extends AbstractTestCase { } } - registerModifier(name: string, ModifierClass: Factory) { + registerModifier(name: string, ModifierClass: InternalFactory) { let { owner } = this; owner.register(`modifier:${name}`, ModifierClass); } - registerComponentManager(name: string, manager: Factory) { + registerComponentManager(name: string, manager: InternalFactory) { let owner = this.owner; owner.register(`component-manager:${name}`, manager); } @@ -219,7 +219,7 @@ export default abstract class RenderingTestCase extends AbstractTestCase { } } - registerService(name: string, klass: Factory) { + registerService(name: string, klass: InternalFactory) { this.owner.register(`service:${name}`, klass); } diff --git a/packages/internal-test-helpers/lib/test-cases/router-non-application.ts b/packages/internal-test-helpers/lib/test-cases/router-non-application.ts index c8eaff9372d..03803cc22f1 100644 --- a/packages/internal-test-helpers/lib/test-cases/router-non-application.ts +++ b/packages/internal-test-helpers/lib/test-cases/router-non-application.ts @@ -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; @@ -59,7 +59,7 @@ export default class RouterNonApplicationTestCase extends AbstractTestCase { return new ModuleBasedResolver(); } - add(specifier: string, factory: Factory | object) { + add(specifier: string, factory: InternalFactory | object) { this.resolver.add(specifier, factory); } diff --git a/packages/internal-test-helpers/lib/test-cases/test-resolver-application.ts b/packages/internal-test-helpers/lib/test-cases/test-resolver-application.ts index 2489826d32c..e292fecc4ef 100644 --- a/packages/internal-test-helpers/lib/test-cases/test-resolver-application.ts +++ b/packages/internal-test-helpers/lib/test-cases/test-resolver-application.ts @@ -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; @@ -13,7 +13,7 @@ export default abstract class TestResolverApplicationTestCase extends AbstractAp }); } - add(specifier: string, factory: Factory | object) { + add(specifier: string, factory: InternalFactory | object) { this.resolver!.add(specifier, factory); } diff --git a/packages/internal-test-helpers/lib/test-resolver.ts b/packages/internal-test-helpers/lib/test-resolver.ts index 1f4c16cca4c..2372832089c 100644 --- a/packages/internal-test-helpers/lib/test-resolver.ts +++ b/packages/internal-test-helpers/lib/test-resolver.ts @@ -1,6 +1,6 @@ import { compile } from 'ember-template-compiler'; -import type { Factory, Resolver as IResolver } from '@ember/-internals/owner'; +import type { InternalFactory, Resolver as IResolver } from '@ember/-internals/owner'; const DELIMITER = '%'; @@ -14,17 +14,17 @@ function serializeKey(specifier: string, source?: unknown, namespace?: unknown) } class Resolver implements IResolver { - _registered: Record | object>; + _registered: Record | object>; constructor() { this._registered = {}; } - resolve(specifier: string): Factory | object | undefined { + resolve(specifier: string): InternalFactory | object | undefined { return this._registered[specifier] || this._registered[serializeKey(specifier)]; } add( lookup: string | { specifier: string; source: unknown; namespace: unknown }, - factory: Factory | object + factory: InternalFactory | object ) { let key; switch (typeof lookup) {