From 4c99fa1416445d2baeb5698df220afd6a55b2f28 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Thu, 29 Jun 2023 08:26:10 -0600 Subject: [PATCH] [BUGFIX stable] Fix types for Resolver contract We need `Resolver` implementors (e.g. `ember-resolver`) to be able to provide a class which can satisfy this contract, which means we need to support either a static `.create()` method or a constructor (with a strong preference for the constructor). This is likely to be a common issue with this kind of assignability of classic classes, because it ultimately comes down to the definition of `Factory` and `CoreObject`. To resolve this, add two overloads for `CoreObject.prototype.create`: - One which just produces an instance type with no args at all. - One which produces an instance type by merging with passed-in args. These two overloads also improve the type safety of `.create()` more generally, allowing us to handle a couple cases which the types did not previously account for (and indeed had given up handling). This also adds a smoke test for something shaped like the `app.ts` file generated for every Ember app, so that we can be sure that the code we emit when generating an app actually works. For now, this simply copies over the types from `ember-resolver`, `ember-load-initializers`, and a basic `environment`. --- .../-internals/container/lib/registry.ts | 9 ++++-- packages/@ember/array/proxy.ts | 4 +-- .../@ember/array/type-tests/proxy.test.ts | 2 +- packages/@ember/object/core.ts | 11 +++++-- .../internal-test-helpers/lib/build-owner.ts | 3 +- .../@ember/object-test/create-negative.ts | 29 ++++--------------- type-tests/ember/create-negative.ts | 29 ++++--------------- type-tests/smoke/basic-app-alike/app.ts | 12 ++++++++ .../ember-load-initializers-alike.d.ts | 10 +++++++ .../basic-app-alike/ember-resolver-alike.d.ts | 4 +++ .../basic-app-alike/environment-alike.d.ts | 14 +++++++++ 11 files changed, 71 insertions(+), 56 deletions(-) create mode 100644 type-tests/smoke/basic-app-alike/app.ts create mode 100644 type-tests/smoke/basic-app-alike/ember-load-initializers-alike.d.ts create mode 100644 type-tests/smoke/basic-app-alike/ember-resolver-alike.d.ts create mode 100644 type-tests/smoke/basic-app-alike/environment-alike.d.ts diff --git a/packages/@ember/-internals/container/lib/registry.ts b/packages/@ember/-internals/container/lib/registry.ts index 08892bb678e..3c8a0d92ee6 100644 --- a/packages/@ember/-internals/container/lib/registry.ts +++ b/packages/@ember/-internals/container/lib/registry.ts @@ -1,4 +1,5 @@ import type { + Factory, FactoryClass, FullName, InternalFactory, @@ -18,9 +19,11 @@ export interface Injection { specifier: FullName; } -export interface ResolverClass { - create(...args: unknown[]): Resolver; -} +export interface ResolverClass + extends Factory, + Partial<{ + new (...args: any): Resolver; + }> {} export interface RegistryOptions { fallback?: Registry; diff --git a/packages/@ember/array/proxy.ts b/packages/@ember/array/proxy.ts index a153a9d193d..ff72f1990ac 100644 --- a/packages/@ember/array/proxy.ts +++ b/packages/@ember/array/proxy.ts @@ -125,7 +125,7 @@ interface ArrayProxy extends MutableArray { @type EmberArray @public */ - content: EmberArray | NativeArray | null; + content: T[] | EmberArray | NativeArray | null; /** The array that the proxy pretends to be. In the default `ArrayProxy` implementation, this and `content` are the same. Subclasses of `ArrayProxy` @@ -214,7 +214,7 @@ class ArrayProxy extends EmberObject implements PropertyDidChange { this._removeArrangedContentArrayObserver(); } - declare content: EmberArray | NativeArray | null; + declare content: T[] | EmberArray | NativeArray | null; declare arrangedContent: EmberArray | null; diff --git a/packages/@ember/array/type-tests/proxy.test.ts b/packages/@ember/array/type-tests/proxy.test.ts index c7f74be1cd6..e10b7d22582 100644 --- a/packages/@ember/array/type-tests/proxy.test.ts +++ b/packages/@ember/array/type-tests/proxy.test.ts @@ -17,5 +17,5 @@ let proxy = ArrayProxy.create({ content }) as ArrayProxy; expectTypeOf(proxy).toMatchTypeOf>(); expectTypeOf(proxy).toMatchTypeOf>(); -expectTypeOf(proxy.content).toEqualTypeOf | NativeArray | null>(); +expectTypeOf(proxy.content).toEqualTypeOf | NativeArray | null>(); expectTypeOf(proxy.arrangedContent).toEqualTypeOf | null>(); diff --git a/packages/@ember/object/core.ts b/packages/@ember/object/core.ts index ed18d97d530..c61c16c1494 100644 --- a/packages/@ember/object/core.ts +++ b/packages/@ember/object/core.ts @@ -763,14 +763,21 @@ class CoreObject { @param [arguments]* @public */ + static create(this: C): InstanceType; static create< C extends typeof CoreObject, I extends InstanceType, K extends keyof I, - Args extends Array>> + Args extends Array> + >(this: C, ...args: Args): InstanceType & MergeArray; + static create< + C extends typeof CoreObject, + I extends InstanceType, + K extends keyof I, + Args extends Array> >(this: C, ...args: Args): InstanceType & MergeArray { let props = args[0]; - let instance; + let instance: InstanceType; if (props !== undefined) { instance = new this(getOwner(props)) as InstanceType; diff --git a/packages/internal-test-helpers/lib/build-owner.ts b/packages/internal-test-helpers/lib/build-owner.ts index 5172f6f91ef..c385f32dae8 100644 --- a/packages/internal-test-helpers/lib/build-owner.ts +++ b/packages/internal-test-helpers/lib/build-owner.ts @@ -4,9 +4,8 @@ import Engine from '@ember/engine'; import { registerDestructor } from '@ember/destroyable'; import type Resolver from './test-resolver'; import type { EngineInstanceOptions } from '@ember/engine/instance'; -import type { ResolverClass } from '@ember/-internals/container'; -class ResolverWrapper implements ResolverClass { +class ResolverWrapper { resolver: Resolver; constructor(resolver: Resolver) { diff --git a/type-tests/@ember/object-test/create-negative.ts b/type-tests/@ember/object-test/create-negative.ts index feeb9366f30..86557265951 100644 --- a/type-tests/@ember/object-test/create-negative.ts +++ b/type-tests/@ember/object-test/create-negative.ts @@ -1,25 +1,8 @@ -import { expectTypeOf } from 'expect-type'; import { Person } from './create'; -// Let's see some *real* type-unsafety! These demonstrate that we just -// absolutely lie about the runtime behavior for `.create()`. 😬 The result of -// calling these with `firstName: 99` is to *change the type* of the resulting -// object from `Person` to `Person` with `firstName: number`. While we might -// *like* to prevent that call and make sure that `firstName` matches the type -// we cannot reasonably do so. - -expectTypeOf(Person.create({ firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); -expectTypeOf(Person.create({}, { firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); -expectTypeOf(Person.create({}, {}, { firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); +// @ts-expect-error +Person.create({ firstName: 99 }); +// @ts-expect-error +Person.create({}, { firstName: 99 }); +// @ts-expect-error +Person.create({}, {}, { firstName: 99 }); diff --git a/type-tests/ember/create-negative.ts b/type-tests/ember/create-negative.ts index feeb9366f30..86557265951 100644 --- a/type-tests/ember/create-negative.ts +++ b/type-tests/ember/create-negative.ts @@ -1,25 +1,8 @@ -import { expectTypeOf } from 'expect-type'; import { Person } from './create'; -// Let's see some *real* type-unsafety! These demonstrate that we just -// absolutely lie about the runtime behavior for `.create()`. 😬 The result of -// calling these with `firstName: 99` is to *change the type* of the resulting -// object from `Person` to `Person` with `firstName: number`. While we might -// *like* to prevent that call and make sure that `firstName` matches the type -// we cannot reasonably do so. - -expectTypeOf(Person.create({ firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); -expectTypeOf(Person.create({}, { firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); -expectTypeOf(Person.create({}, {}, { firstName: 99 })).toEqualTypeOf< - Person & { - firstName: number; - } ->(); +// @ts-expect-error +Person.create({ firstName: 99 }); +// @ts-expect-error +Person.create({}, { firstName: 99 }); +// @ts-expect-error +Person.create({}, {}, { firstName: 99 }); diff --git a/type-tests/smoke/basic-app-alike/app.ts b/type-tests/smoke/basic-app-alike/app.ts new file mode 100644 index 00000000000..d4345007360 --- /dev/null +++ b/type-tests/smoke/basic-app-alike/app.ts @@ -0,0 +1,12 @@ +import Application from '@ember/application'; +import Resolver from './ember-resolver-alike'; +import loadInitializers from './ember-load-initializers-alike'; +import config from './environment-alike'; + +export default class App extends Application { + modulePrefix = config.modulePrefix; + podModulePrefix = config.podModulePrefix; + Resolver = Resolver; +} + +loadInitializers(App, config.modulePrefix); diff --git a/type-tests/smoke/basic-app-alike/ember-load-initializers-alike.d.ts b/type-tests/smoke/basic-app-alike/ember-load-initializers-alike.d.ts new file mode 100644 index 00000000000..e8455e95c6a --- /dev/null +++ b/type-tests/smoke/basic-app-alike/ember-load-initializers-alike.d.ts @@ -0,0 +1,10 @@ +declare global { + var requirejs: { + _eak_seen: Object; + }; +} +import Engine from '@ember/engine'; +/** + * Configure your application as it boots + */ +export default function loadInitializers(app: typeof Engine, prefix: string): void; diff --git a/type-tests/smoke/basic-app-alike/ember-resolver-alike.d.ts b/type-tests/smoke/basic-app-alike/ember-resolver-alike.d.ts new file mode 100644 index 00000000000..c56a56b00c7 --- /dev/null +++ b/type-tests/smoke/basic-app-alike/ember-resolver-alike.d.ts @@ -0,0 +1,4 @@ +import { Resolver as ResolverContract } from '@ember/owner'; +import EmberObject from '@ember/object'; +export default class Resolver extends EmberObject {} +export default interface Resolver extends Required {} diff --git a/type-tests/smoke/basic-app-alike/environment-alike.d.ts b/type-tests/smoke/basic-app-alike/environment-alike.d.ts new file mode 100644 index 00000000000..a8f92b9feb9 --- /dev/null +++ b/type-tests/smoke/basic-app-alike/environment-alike.d.ts @@ -0,0 +1,14 @@ +/** + * Type declarations for + * import config from 'my-app/config/environment' + */ +declare const config: { + environment: string; + modulePrefix: string; + podModulePrefix: string; + locationType: 'history' | 'hash' | 'none' | 'auto'; + rootURL: string; + APP: Record; +}; + +export default config;