Skip to content

Commit

Permalink
[BUGFIX stable] Fix types for Resolver contract
Browse files Browse the repository at this point in the history
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`.

(cherry picked from commit 4c99fa1)
  • Loading branch information
chriskrycho committed Jun 29, 2023
1 parent fdc1283 commit 9939768
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 56 deletions.
9 changes: 6 additions & 3 deletions packages/@ember/-internals/container/lib/registry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {
Factory,
FactoryClass,
FullName,
InternalFactory,
Expand All @@ -18,9 +19,11 @@ export interface Injection {
specifier: FullName;
}

export interface ResolverClass {
create(...args: unknown[]): Resolver;
}
export interface ResolverClass
extends Factory<Resolver>,
Partial<{
new (...args: any): Resolver;
}> {}

export interface RegistryOptions {
fallback?: Registry;
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/array/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ interface ArrayProxy<T> extends MutableArray<T> {
@type EmberArray
@public
*/
content: EmberArray<T> | NativeArray<T> | null;
content: T[] | EmberArray<T> | NativeArray<T> | null;
/**
The array that the proxy pretends to be. In the default `ArrayProxy`
implementation, this and `content` are the same. Subclasses of `ArrayProxy`
Expand Down Expand Up @@ -214,7 +214,7 @@ class ArrayProxy<T> extends EmberObject implements PropertyDidChange {
this._removeArrangedContentArrayObserver();
}

declare content: EmberArray<T> | NativeArray<T> | null;
declare content: T[] | EmberArray<T> | NativeArray<T> | null;

declare arrangedContent: EmberArray<T> | null;

Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/array/type-tests/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ let proxy = ArrayProxy.create({ content }) as ArrayProxy<Foo>;
expectTypeOf(proxy).toMatchTypeOf<EmberArray<Foo>>();
expectTypeOf(proxy).toMatchTypeOf<MutableArray<Foo>>();

expectTypeOf(proxy.content).toEqualTypeOf<EmberArray<Foo> | NativeArray<Foo> | null>();
expectTypeOf(proxy.content).toEqualTypeOf<Foo[] | EmberArray<Foo> | NativeArray<Foo> | null>();
expectTypeOf(proxy.arrangedContent).toEqualTypeOf<EmberArray<Foo> | null>();
11 changes: 9 additions & 2 deletions packages/@ember/object/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -763,14 +763,21 @@ class CoreObject {
@param [arguments]*
@public
*/
static create<C extends typeof CoreObject>(this: C): InstanceType<C>;
static create<
C extends typeof CoreObject,
I extends InstanceType<C>,
K extends keyof I,
Args extends Array<Partial<Record<K, I[K]>>>
Args extends Array<Partial<{ [Key in K]: I[Key] }>>
>(this: C, ...args: Args): InstanceType<C> & MergeArray<Args>;
static create<
C extends typeof CoreObject,
I extends InstanceType<C>,
K extends keyof I,
Args extends Array<Partial<{ [Key in K]: I[Key] }>>
>(this: C, ...args: Args): InstanceType<C> & MergeArray<Args> {
let props = args[0];
let instance;
let instance: InstanceType<C>;

if (props !== undefined) {
instance = new this(getOwner(props)) as InstanceType<C>;
Expand Down
3 changes: 1 addition & 2 deletions packages/internal-test-helpers/lib/build-owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
29 changes: 6 additions & 23 deletions type-tests/@ember/object-test/create-negative.ts
Original file line number Diff line number Diff line change
@@ -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 });
29 changes: 6 additions & 23 deletions type-tests/ember/create-negative.ts
Original file line number Diff line number Diff line change
@@ -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 });
12 changes: 12 additions & 0 deletions type-tests/smoke/basic-app-alike/app.ts
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 4 additions & 0 deletions type-tests/smoke/basic-app-alike/ember-resolver-alike.d.ts
Original file line number Diff line number Diff line change
@@ -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<ResolverContract> {}
14 changes: 14 additions & 0 deletions type-tests/smoke/basic-app-alike/environment-alike.d.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;
};

export default config;

0 comments on commit 9939768

Please sign in to comment.