From c54e79795994b8f337e2dbe2bf5ccfe8a7eb0364 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 23 May 2022 11:02:32 -0600 Subject: [PATCH 01/14] RFC: public types for Owner, Transition, RouteInfo --- text/0821-public-types.md | 358 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 358 insertions(+) create mode 100644 text/0821-public-types.md diff --git a/text/0821-public-types.md b/text/0821-public-types.md new file mode 100644 index 0000000000..f5e46fc033 --- /dev/null +++ b/text/0821-public-types.md @@ -0,0 +1,358 @@ +--- +Stage: Accepted +Start Date: 2022-05-23 +Release Date: Unreleased +Release Versions: + ember-source: vX.Y.Z + ember-data: vX.Y.Z +Relevant Team(s): + - Ember.js + - TypeScript + - Learning +RFC PR: https://github.com/emberjs/rfcs/pull/821 +--- + +# Public API for Type-Only Imports + + +## Summary + +Introduce public import locations for type-only imports which have previously had no imports, and fully specify their public APIs for end users: + +- `Owner`, with `TypeOptions` and `FactoryManager` +- `Transition` +- `RouteInfo` and `RouteInfoWithAttributes` + + +## Motivation + +Prior to supporting TypeScript, Ember has defined certain types as part of its API, but *without* public imports, since the types were not designed to be imported, subclassed, etc. by users. For the community-maintained type definitions, the Typed Ember team chose to match that policy so as to avoid committing the main project to public API. With the introduction of TypeScript as a first-class language (see esp. RFCs [0724: Official TypeScript Support][0724] and [0800: TypeScript Adoption Plan][0800]) those types now need public imports so that users can reference them; per the [Semantic Versioning for TypeScript Types spec][spec], they also need to define *how* they are public: can they be sub-classed, re-implemented, etc.? + +[0274]: https://rfcs.emberjs.com/id/0724-road-to-typescript +[0800]: https://rfcs.emberjs.com/id/0800-ts-adoption-plan +[spec]: https://www.semver-ts.org + +Additionally, the lack of a public import or contract for the `Owner` interface has been a long-standing problem for *all* users, but especially TypeScript users, and given the APIs provided for e.g. the Glimmer Component class where `owner` is the first argument, the pervasive use of `getOwner` in low-level library code, etc., it is important for TypeScript users to be able to use an `Owner` safely, and for JavaScript users to be able to get autocomplete etc. from the types we ship. + + +## Detailed design + +**Note:** For the terms "user-constructible" and "user-subclassable" see the [Semantic Versioning for TypeScript Types spec][spec]. + +### `Owner` + +`Owner` is a user-constructible interface, with an intentionally-minimal subset of the : + +```ts +export default interface Owner { + lookup(name: string): unknown; + + register( + fullName: string, + factory: FactoryManager, + options?: TypeOptions + ): void; + + hasRegistration(fullName: string): boolean; + + resolveRegistration( + fullName: string + ): FactoryManager | object | undefined; +} +``` + +It is the default import from a new module, `@ember/owner`. With it come two other new types: `TypeOptions` and `FactoryManager`. + +#### `TypeOptions` + +`TypeOptions` is a user-constructible interface: + +```ts +export interface TypeOptions { + instantiate?: boolean | undefined; + singleton?: boolean | undefined; +} +``` + +Although users will not usually need to use it directly, instead simply passing it as a POJO to `Owner#register`, it is available as a named export from `@ember/owner`: + +```ts +import { type TypeOptions } from '@ember/owner'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/owner').TypeOptions} typeOptions + */ +function useTypeOptions(typeOptions) { + // ... +} +``` + + +#### `FactoryManager` + +`FactoryManager` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this user-constructible interface: + +[ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=factoryFor + +```ts +export interface FactoryManager { + class: Class; + create( + initialValues?: { + [K in keyof InstanceOf]?: InstanceOf[K]; + } + ): InstanceOf; +} +``` + +
The InstanceOf and ClassProps type + +The `InstanceOf` type here is a utility type which is *not* exported, because it is only necessary to guarantee that `create` accepts and returns the appropriate values: the fields to set on the class instance, and the instance after construction respectively. It is provided here only for completeness. + +```ts +type InstanceOf = T extends new (...args: any) => infer R ? R : never; +``` + +
+ +`FactoryManager` is now available as a named import from `@ember/owner`: + +```ts +import { type FactoryManager } from '@ember/owner'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/owner').FactoryManager} factoryManager + */ +function useFactoryManager(factoryManager) { + // ... +} +``` + +Note that the `Class` type parameter must be defined using `typeof SomeClass`, *not* `SomeClass`: + +```ts +import { type FactoryManager } from '@ember/owner'; + +class Person { + constructor(public name: string, public age: number) {} +} + +class PersonManager implements FactoryManager { + class = Person; + create({ name = "", age = 0 } = {}) { + return new this.class(name, age); + } +} +``` + +(This is not the actual usual internal implementation in Ember, but shows that it can be implemented safely with these types.) + +[^factory-name]: In Ember’s types today, this type is known as `Factory`, and the `class` is named `FactoryClass`. However, Ember’s public API docs already refer to this type as `FactoryManager`, *not* `Factory`. I have chosen `FactoryManager` to match in this RFC + + +### `Transition` + +`Transition` is a non-user-constructible, non-user-subclassable class. It is identical to the *existing* public API, with two new features: + +- the specification of the generic type `T` representing the resolution state of the `Promise` associated with the route (and used with `RouteInfoWithAttributes`; see below) +- a public import location + +Since it is neither constructible nor implementable, it should be supplied as type-only export. For example: + +```ts +class _Transition + implements Pick, 'then' | 'catch' | 'finally'> { + + data: Record; + readonly from: RouteInfoWithAttributes | null; + readonly promise: Promise; + readonly to: RouteInfo | RouteInfoWithAttributes; + abort(): Transition; + followRedirects(): Promise; + method(method?: string): Transition; + retry(): Transition; + trigger(ignoreFailure: boolean, name: string): void; + send(ignoreFailure: boolean, name: string): void; + + // These names come from the Promise API, which Transition implements. + then( + onfulfilled?: (value: T) => TResult1 | PromiseLike, + onrejected?: (reason: unknown) => TResult2 | PromiseLike, + label?: string, + ): Promise; + catch( + onRejected?: (reason: unknown) => TResult | PromiseLike, + label?: string, + ): Promise; + finally(onFinally?: () => void, label?: string): Promise; +} + +export default interface Transition extends _Transition {} +``` + +It is the default import from `@ember/routing/transition`: + +```ts +import type Transition from '@ember/routing/transition'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/routing/transition').default} theTransition + */ +function takesTransition(theTransition) { + // ... +} +``` + + +### `RouteInfo` + +`RouteInfo` is a non-user-constructible interface. It is identical to the *existing* public API, with the addition of a public import. + +```ts +export default interface RouteInfo { + readonly child: RouteInfo | null; + readonly localName: string; + readonly name: string; + readonly paramNames: string[]; + readonly params: Record; + readonly parent: RouteInfo | null; + readonly queryParams: Record; + readonly metadata: unknown; + find( + callback: (item: RouteInfo, index: number, array: RouteInfo[]) => boolean, + target?: unknown + ): RouteInfo | undefined; +} +``` + +It is the default import from `@ember/routing/route-info`: + +```ts +import type RouteInfo from '@ember/routing/route-info'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/routing/route-info').default} routeInfo + */ +function takesRouteInfo(routeInfo) { + // ... +} +``` + + +#### `RouteInfoWithAttributes` + +`RouteInfoWithAttributes` is a non-user-constructible interface, which extends `RouteInfo` by adding the `attributes` property. The attributes property represents the resolved return value from the route's model hook: + +```ts +export interface RouteInfoWithAttributes extends RouteInfo { + attributes: T; +} +``` + +It is a named export from `@ember/routing/route-info`: + +```ts +import { type RouteInfoWithAttributes } from '@ember/routing/route-info'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/routing/route-info').RouteInfoWithAttributes} routeInfo + */ +function takesRouteInfoWithAttributes(routeInfoWithAttributes) { + // ... +} +``` + + +## How we teach this + +These concepts all already exist, but need updates to and in some cases wholly new pages in Ember's API docs. + + +### `Owner` + +- We need to introduce API documentation in a new, dedicate module for `Owner`, `@ember/owner`. The docs on `Owner` itself should become the home for much of the documentation currently shared across the `EngineInstance` and `ApplicationInstance` classes. +- We need to document the relationship between `EngineInstance` and `ApplicationInstance` as implementations of `Owner`. + + +### `Transition`, `RouteInfo`, and `RouteInfoWithAttributes` + +These types need two updates to the existing API documentation: + +- Specify the modules they can be imported from (as noted above). +- Specify that these types are *not* meant to be implemented by end users. For example: + + > While the `Transition` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `Transition` is by using Ember's routing APIs, like [the `transitionTo` method](https://api.emberjs.com/ember/4.3/classes/RouterService/methods/transitionTo?anchor=transitionTo) on [the router service](https://api.emberjs.com/ember/4.3/classes/RouterService). + + And: + + > While the `RouteInfo` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `RouteInfo` is from an of [the `Transition` class](https://api.emberjs.com/ember/4.3/classes/Transition). + + +### Blog post + +Besides the API docs described above and the usual discussion of new features in an Ember release blog post, we will include an extended discussion in that blog post, a blog post timed to come out around the same time as that release, or a blog post corresponding to when we add them to DefinitelyTyped, as makes the most sense. That post will situate these as part of the work done on the "road to TypeScript": + +- emphasizing the benefits to both JS and TS users + +- providing a straightforward explanation of the "non-user-constructible" constraint and refer users to the Semantic Versioning for TypeScript Types spec for more details + +- explaining some choices about the public `Owner` API: + - that it includes the subset of the `Owner` API we want to maintain over time, *not* the full set available on the `EngineInstance` type, much of which we expect to deprecate over the course of the 4.x release cycle + - that it does not include type safe registry look-ups, since we are waiting to see what TypeScript does with the Stage 3 decorators spec before deciding how to work with registries going forward[^revise] + +[^revise]: Depending on implementation timelines, it is possible we will revise this based on what TypeScript ships in the meantime. + +More general work to clarify the use of TypeScript with these APIs will be addressed as part of the forthcoming RFCs on integrating TypeScript into Ember's docs. + + +## Drawbacks + +For once: none! These are already all "intimate API" *at minimum*—`Owner` is effectively public API—and the only difference between the _status quo_ and the proposed outcome is that users can know these imports won't break. Moreover, the design here is forward-compatible with any iterations on these types we can currently foresee, including expanding the type of `Owner#lookup` to support registry, changes to the `Owner` API per [RFC #0585: Improved Ember Registry APIs][0585], or future directions for the Ember routing layer. + +[0585]: https://rfcs.emberjs.com/id/0585-improved-ember-registry-apis + + +## Alternatives + +- We could leave these in their private API locations. +- We could not publish these types at all, and have users continue to use utility types to name them (`ReturnType` etc.). + + +## Unresolved questions + +- Should `Owner` be exported from `@glimmer/owner` instead? Presently, that acts as only a pass-through, with the notion of an owner being delegated to implementors of the various Glimmer "manager" APIs.. + +- Should `Owner` have any other of its existing APIs, and in particular should it be identical to the APIs exposed via `EngineInstance`, including these additional methods? + - `inject` + - `ownerInjection` + - `registerOptions` + - `registerOptionsForType` + - `registeredOption` + - `registeredOptions` + - `registeredOptionsForType` + - `unregister` + + (Since `Owner` itself has never been public API, and we hope to *deprecate and remove* all of these methods in the 4.x → 5.0 era, it seems best to leave them off of `Owner` in the newly-public API, only publishing the interface we want to support long-term. + + +- Should `Owner` be directly user-constructible, or should users be restricted to subclassing one of the existing concrete instances (`Engine` or its subclass `Application`)? From ff376d8b9c47c4f05f150dd4db080208af3a824c Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Tue, 24 May 2022 07:36:39 -0600 Subject: [PATCH 02/14] Fix one typo and and one wordo Co-authored-by: MrChocolatine <47531779+MrChocolatine@users.noreply.github.com> --- text/0821-public-types.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index f5e46fc033..cb45290469 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -290,7 +290,7 @@ These concepts all already exist, but need updates to and in some cases wholly n ### `Owner` -- We need to introduce API documentation in a new, dedicate module for `Owner`, `@ember/owner`. The docs on `Owner` itself should become the home for much of the documentation currently shared across the `EngineInstance` and `ApplicationInstance` classes. +- We need to introduce API documentation in a new, dedicated module for `Owner`, `@ember/owner`. The docs on `Owner` itself should become the home for much of the documentation currently shared across the `EngineInstance` and `ApplicationInstance` classes. - We need to document the relationship between `EngineInstance` and `ApplicationInstance` as implementations of `Owner`. @@ -305,7 +305,7 @@ These types need two updates to the existing API documentation: And: - > While the `RouteInfo` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `RouteInfo` is from an of [the `Transition` class](https://api.emberjs.com/ember/4.3/classes/Transition). + > While the `RouteInfo` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `RouteInfo` is using the `from` or `to` properties on an instance of [the `Transition` class](https://api.emberjs.com/ember/4.3/classes/Transition) ### Blog post From c71d685c765f91ea0a52f056e62e28c3a36bc3e7 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Tue, 24 May 2022 17:23:51 -0600 Subject: [PATCH 03/14] Use `Factory`, not `FactoryManager` --- text/0821-public-types.md | 43 +++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index cb45290469..69be02adbe 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -19,7 +19,7 @@ RFC PR: https://github.com/emberjs/rfcs/pull/821 Introduce public import locations for type-only imports which have previously had no imports, and fully specify their public APIs for end users: -- `Owner`, with `TypeOptions` and `FactoryManager` +- `Owner`, with `TypeOptions` and `Factory` - `Transition` - `RouteInfo` and `RouteInfoWithAttributes` @@ -49,7 +49,7 @@ export default interface Owner { register( fullName: string, - factory: FactoryManager, + factory: Factory, options?: TypeOptions ): void; @@ -57,18 +57,18 @@ export default interface Owner { resolveRegistration( fullName: string - ): FactoryManager | object | undefined; + ): Factory | object | undefined; } ``` -It is the default import from a new module, `@ember/owner`. With it come two other new types: `TypeOptions` and `FactoryManager`. +It is the default import from a new module, `@ember/owner`. With it come two other new types: `TypeOptions` and `Factory`. -#### `TypeOptions` +#### `RegisterOptions` -`TypeOptions` is a user-constructible interface: +`RegisterOptions` is a user-constructible interface: ```ts -export interface TypeOptions { +export interface RegisterOptions { instantiate?: boolean | undefined; singleton?: boolean | undefined; } @@ -77,29 +77,29 @@ export interface TypeOptions { Although users will not usually need to use it directly, instead simply passing it as a POJO to `Owner#register`, it is available as a named export from `@ember/owner`: ```ts -import { type TypeOptions } from '@ember/owner'; +import { type RegisterOptions } from '@ember/owner'; ``` JS users can refer to it in JSDoc comments using `import()` syntax: ```js /** - * @param {import('@ember/owner').TypeOptions} typeOptions + * @param {import('@ember/owner').RegisterOptions} registerOptions */ -function useTypeOptions(typeOptions) { +function useRegisterOptions(registerOptions) { // ... } ``` -#### `FactoryManager` +#### `Factory` -`FactoryManager` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this user-constructible interface: +`Factory` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this user-constructible interface: [ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=factoryFor ```ts -export interface FactoryManager { +export interface Factory { class: Class; create( initialValues?: { @@ -119,19 +119,19 @@ type InstanceOf = T extends new (...args: any) => infer R ? R : never; -`FactoryManager` is now available as a named import from `@ember/owner`: +`Factory` is now available as a named import from `@ember/owner`: ```ts -import { type FactoryManager } from '@ember/owner'; +import { type Factory } from '@ember/owner'; ``` JS users can refer to it in JSDoc comments using `import()` syntax: ```js /** - * @param {import('@ember/owner').FactoryManager} factoryManager + * @param {import('@ember/owner').Factory} Factory */ -function useFactoryManager(factoryManager) { +function useFactory(Factory) { // ... } ``` @@ -139,13 +139,13 @@ function useFactoryManager(factoryManager) { Note that the `Class` type parameter must be defined using `typeof SomeClass`, *not* `SomeClass`: ```ts -import { type FactoryManager } from '@ember/owner'; +import { type Factory } from '@ember/owner'; class Person { constructor(public name: string, public age: number) {} } -class PersonManager implements FactoryManager { +class PersonManager implements Factory { class = Person; create({ name = "", age = 0 } = {}) { return new this.class(name, age); @@ -155,7 +155,7 @@ class PersonManager implements FactoryManager { (This is not the actual usual internal implementation in Ember, but shows that it can be implemented safely with these types.) -[^factory-name]: In Ember’s types today, this type is known as `Factory`, and the `class` is named `FactoryClass`. However, Ember’s public API docs already refer to this type as `FactoryManager`, *not* `Factory`. I have chosen `FactoryManager` to match in this RFC +[^factory-name]: In Ember’s internal types today, this type is known as `Factory`, and the `class` is named `FactoryClass`. Ember’s public API docs refer to this type as a "factory manager", *not* `Factory`. However, that term is not really appropriate, so I have chosen to stick with `Factory` here; see discussion under [How We Teach This: Owner](#owner-1) for details on updating the docs. ### `Transition` @@ -291,8 +291,11 @@ These concepts all already exist, but need updates to and in some cases wholly n ### `Owner` - We need to introduce API documentation in a new, dedicated module for `Owner`, `@ember/owner`. The docs on `Owner` itself should become the home for much of the documentation currently shared across the `EngineInstance` and `ApplicationInstance` classes. + - We need to document the relationship between `EngineInstance` and `ApplicationInstance` as implementations of `Owner`. +- We must update the existing docs for `factoryFor` to match the updated types: we return a *factory* rather than a *factory manager*. A "manager" is an existing concept in Ember (component managers, helper managers, modifier managers, etc.), and the factory returned by `factoryFor` is *not* one of those and not similar to them. + ### `Transition`, `RouteInfo`, and `RouteInfoWithAttributes` From a7cc41f7accee0c08a4cbbd1904dd2673819c559 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Tue, 24 May 2022 17:29:21 -0600 Subject: [PATCH 04/14] Move `getOwner` and `setOwner` --- text/0821-public-types.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 69be02adbe..4023932994 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -215,6 +215,19 @@ function takesTransition(theTransition) { } ``` +#### `getOwner` and `setOwner` + +Both of the existing `getOwner` and `setOwner` functions now make *much* more sense as named exports from `@ember/owner`. They will also now take `owner` as the type of their argument explicitly: + +```ts +export function getOwner(object): Owner | undefined; +export function setOwner(object: unknown, owner: Owner): void; +``` + +The existing exports from `@ember/application` will become re-exports of these functions. The existing exports will also be deprecated, with the deprecation becoming "Available" no earlier than the first minor *after* an LTS containing the updated import location. For example, if `getOwner` and `setOwner` are available to import from `@ember/owner` in Ember v4.7, the deprecation for the imports from `@ember/application` would not be deprecated until at least Ember v4.9, after Ember v4.8 LTS.[^timeline] + +[^timeline]: This is in line with our normal approach to deprecation: it allows the addon ecosystem to absorb the change via LTS support releases, so that consuming apps are not flooded with deprecations without recourse. + ### `RouteInfo` From 4011bcf74486e031ff12a9a80fecca1bc8eede50 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Tue, 24 May 2022 17:32:46 -0600 Subject: [PATCH 05/14] spacing --- text/0821-public-types.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 4023932994..34c5a261b7 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -315,6 +315,7 @@ These concepts all already exist, but need updates to and in some cases wholly n These types need two updates to the existing API documentation: - Specify the modules they can be imported from (as noted above). + - Specify that these types are *not* meant to be implemented by end users. For example: > While the `Transition` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `Transition` is by using Ember's routing APIs, like [the `transitionTo` method](https://api.emberjs.com/ember/4.3/classes/RouterService/methods/transitionTo?anchor=transitionTo) on [the router service](https://api.emberjs.com/ember/4.3/classes/RouterService). From ca1328d25e10fdbd5c40c17aa9faf0d22ed2f962 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 13 Jun 2022 07:52:21 -0600 Subject: [PATCH 06/14] Clarify how to get `RouteInfo`s legally --- text/0821-public-types.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 34c5a261b7..18421de537 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -322,7 +322,11 @@ These types need two updates to the existing API documentation: And: - > While the `RouteInfo` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `RouteInfo` is using the `from` or `to` properties on an instance of [the `Transition` class](https://api.emberjs.com/ember/4.3/classes/Transition) + > While the `RouteInfo` interface can be imported to name the type for use in your own code, it is not user-implementable. The only supported way to get a `RouteInfo` is using a public API which provides it, including but not limited to: + > + > - the `from` or `to` properties on an instance of [the `Transition` class](https://api.emberjs.com/ember/4.3/classes/Transition) + > - the `currentRoute` property on [the `RouterService` class](https://api.emberjs.com/ember/4.4/classes/RouterService) + > - the `child` or `parent` properties on another `RouteInfo` instance, or as returned from the `find()` method on a `RouteInfo` ### Blog post From 1f97c1a9ac536f905e9c719b47a85f3caca8af59 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 13 Jun 2022 07:52:28 -0600 Subject: [PATCH 07/14] Fix link to RFC 0724 --- text/0821-public-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 18421de537..3040e9ec4d 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -28,7 +28,7 @@ Introduce public import locations for type-only imports which have previously ha Prior to supporting TypeScript, Ember has defined certain types as part of its API, but *without* public imports, since the types were not designed to be imported, subclassed, etc. by users. For the community-maintained type definitions, the Typed Ember team chose to match that policy so as to avoid committing the main project to public API. With the introduction of TypeScript as a first-class language (see esp. RFCs [0724: Official TypeScript Support][0724] and [0800: TypeScript Adoption Plan][0800]) those types now need public imports so that users can reference them; per the [Semantic Versioning for TypeScript Types spec][spec], they also need to define *how* they are public: can they be sub-classed, re-implemented, etc.? -[0274]: https://rfcs.emberjs.com/id/0724-road-to-typescript +[0724]: https://rfcs.emberjs.com/id/0724-road-to-typescript [0800]: https://rfcs.emberjs.com/id/0800-ts-adoption-plan [spec]: https://www.semver-ts.org From 21ccda4a3ac470fcb67c4c6fdfc77675c3599c08 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 13 Jun 2022 07:54:06 -0600 Subject: [PATCH 08/14] Fix a few missed `TypeOptions` -> `RegisterOptions` --- text/0821-public-types.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 3040e9ec4d..126bcc74be 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -19,7 +19,7 @@ RFC PR: https://github.com/emberjs/rfcs/pull/821 Introduce public import locations for type-only imports which have previously had no imports, and fully specify their public APIs for end users: -- `Owner`, with `TypeOptions` and `Factory` +- `Owner`, with `RegisterOptions` and `Factory` - `Transition` - `RouteInfo` and `RouteInfoWithAttributes` @@ -50,7 +50,7 @@ export default interface Owner { register( fullName: string, factory: Factory, - options?: TypeOptions + options?: RegisterOptions ): void; hasRegistration(fullName: string): boolean; @@ -61,7 +61,7 @@ export default interface Owner { } ``` -It is the default import from a new module, `@ember/owner`. With it come two other new types: `TypeOptions` and `Factory`. +It is the default import from a new module, `@ember/owner`. With it come two other new types: `RegisterOptions` and `Factory`. #### `RegisterOptions` From 1d616436b5a30dc31ec4ca7df58370448cee5a50 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 13 Jun 2022 08:02:22 -0600 Subject: [PATCH 09/14] Fix typo: `Engine#factoryFor` -> `Engine#lookup` --- text/0821-public-types.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 126bcc74be..52d00ea829 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -94,9 +94,9 @@ function useRegisterOptions(registerOptions) { #### `Factory` -`Factory` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this user-constructible interface: +`Factory` is an existing concept available to users via [the `Engine#lookup` API][ff]. The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this user-constructible interface: -[ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=factoryFor +[ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=lookup ```ts export interface Factory { From 5b9bc9c6ac6a30ff25b64daea64aa1b619638dff Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 13 Jun 2022 08:02:41 -0600 Subject: [PATCH 10/14] Define the `FactoryFor` type (for bike shedding!) --- text/0821-public-types.md | 45 +++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 52d00ea829..bca55d0026 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -41,7 +41,7 @@ Additionally, the lack of a public import or contract for the `Owner` interface ### `Owner` -`Owner` is a user-constructible interface, with an intentionally-minimal subset of the : +`Owner` is a user-constructible interface, with an intentionally-minimal subset of the existing `Owner` API, aimed at what we *want* to support for `Owner` in the future: ```ts export default interface Owner { @@ -55,9 +55,7 @@ export default interface Owner { hasRegistration(fullName: string): boolean; - resolveRegistration( - fullName: string - ): Factory | object | undefined; + factoryFor(fullName: string): FactoryFor | undefined; } ``` @@ -155,7 +153,42 @@ class PersonManager implements Factory { (This is not the actual usual internal implementation in Ember, but shows that it can be implemented safely with these types.) -[^factory-name]: In Ember’s internal types today, this type is known as `Factory`, and the `class` is named `FactoryClass`. Ember’s public API docs refer to this type as a "factory manager", *not* `Factory`. However, that term is not really appropriate, so I have chosen to stick with `Factory` here; see discussion under [How We Teach This: Owner](#owner-1) for details on updating the docs. + +#### `FactoryFor` + +`FactoryFor` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-for-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this **non-user-constructible** interface: + +[ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=lookup + +```ts +export class FactoryFor { + readonly class: Factory; + create( + initialValues?: { + [K in keyof InstanceOf]?: InstanceOf[K]; + } + ): InstanceOf; +} +``` + +`FactoryFor` is now available as a named import from `@ember/owner`: + +```ts +import { type FactoryFor } from '@ember/owner'; +``` + +JS users can refer to it in JSDoc comments using `import()` syntax: + +```js +/** + * @param {import('@ember/owner').FactoryFor} factoryForObj + */ +function useFactoryForObj(factoryForObj) { + // ... +} +``` + +[^factory-for-name]: In Ember’s internal types today, this type is known as `FactoryManager`, and the `class` is the `Factory`, which may in turn have a `FactoryClass` as *its* `class` property. Ember’s public API docs refer to this type as a "factory manager", *not* `Factory`. However, that term is not really appropriate, so I have chosen to use `FactoryFor` here; see discussion under [How We Teach This: Owner](#owner-1) for details on updating the docs. ### `Transition` @@ -307,7 +340,7 @@ These concepts all already exist, but need updates to and in some cases wholly n - We need to document the relationship between `EngineInstance` and `ApplicationInstance` as implementations of `Owner`. -- We must update the existing docs for `factoryFor` to match the updated types: we return a *factory* rather than a *factory manager*. A "manager" is an existing concept in Ember (component managers, helper managers, modifier managers, etc.), and the factory returned by `factoryFor` is *not* one of those and not similar to them. +- We must update the existing docs for `factoryFor` to match the updated types: we return a `FactoryFor` rather than a *factory manager*. A "manager" is an existing concept in Ember (component managers, helper managers, modifier managers, etc.), and the factory returned by `factoryFor` is *not* one of those and not similar to them. ### `Transition`, `RouteInfo`, and `RouteInfoWithAttributes` From 88f13cdeac4e4963731fbe50f9bd9446b1d03a5a Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 20 Jul 2022 09:19:32 -0600 Subject: [PATCH 11/14] RFC 821: revert to using `FactoryManager` --- text/0821-public-types.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index bca55d0026..949770d454 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -55,7 +55,7 @@ export default interface Owner { hasRegistration(fullName: string): boolean; - factoryFor(fullName: string): FactoryFor | undefined; + factoryFor(fullName: string): FactoryManager | undefined; } ``` @@ -154,14 +154,14 @@ class PersonManager implements Factory { (This is not the actual usual internal implementation in Ember, but shows that it can be implemented safely with these types.) -#### `FactoryFor` +#### `FactoryManager` -`FactoryFor` is an existing concept available to users via [the `Engine#factoryFor` API][ff].[^factory-for-name] The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this **non-user-constructible** interface: +`FactoryManager` is an existing concept available to users via [the `Engine#factoryFor` API][ff]. The public API to date has included only two fields, `class` and `create`, and we maintain that in this RFC. The result is this **non-user-constructible** interface: [ff]: https://api.emberjs.com/ember/4.3/classes/EngineInstance/methods/factoryFor?anchor=lookup ```ts -export class FactoryFor { +export class FactoryManager { readonly class: Factory; create( initialValues?: { @@ -171,25 +171,23 @@ export class FactoryFor { } ``` -`FactoryFor` is now available as a named import from `@ember/owner`: +`FactoryManager` is now available as a named import from `@ember/owner`: ```ts -import { type FactoryFor } from '@ember/owner'; +import { type FactoryManager } from '@ember/owner'; ``` JS users can refer to it in JSDoc comments using `import()` syntax: ```js /** - * @param {import('@ember/owner').FactoryFor} factoryForObj + * @param {import('@ember/owner').FactoryManager} factoryManager */ -function useFactoryForObj(factoryForObj) { +function useFactoryManager(factoryManager) { // ... } ``` -[^factory-for-name]: In Ember’s internal types today, this type is known as `FactoryManager`, and the `class` is the `Factory`, which may in turn have a `FactoryClass` as *its* `class` property. Ember’s public API docs refer to this type as a "factory manager", *not* `Factory`. However, that term is not really appropriate, so I have chosen to use `FactoryFor` here; see discussion under [How We Teach This: Owner](#owner-1) for details on updating the docs. - ### `Transition` @@ -340,7 +338,7 @@ These concepts all already exist, but need updates to and in some cases wholly n - We need to document the relationship between `EngineInstance` and `ApplicationInstance` as implementations of `Owner`. -- We must update the existing docs for `factoryFor` to match the updated types: we return a `FactoryFor` rather than a *factory manager*. A "manager" is an existing concept in Ember (component managers, helper managers, modifier managers, etc.), and the factory returned by `factoryFor` is *not* one of those and not similar to them. +- We must update the existing docs for `factoryFor` to clarify that it is the only public API for getting a `FactoryManager`. ### `Transition`, `RouteInfo`, and `RouteInfoWithAttributes` From 42fc48aa24411a36d1465443da93128adcff1ba0 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 20 Jul 2022 09:24:24 -0600 Subject: [PATCH 12/14] RFC 821: `Owner` is non-user-constructible --- text/0821-public-types.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 949770d454..4336694eea 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -41,7 +41,7 @@ Additionally, the lack of a public import or contract for the `Owner` interface ### `Owner` -`Owner` is a user-constructible interface, with an intentionally-minimal subset of the existing `Owner` API, aimed at what we *want* to support for `Owner` in the future: +`Owner` is a **non-user-constructible** interface, with an intentionally-minimal subset of the existing `Owner` API, aimed at what we *want* to support for `Owner` in the future: ```ts export default interface Owner { @@ -59,7 +59,12 @@ export default interface Owner { } ``` -It is the default import from a new module, `@ember/owner`. With it come two other new types: `RegisterOptions` and `Factory`. +`Owner` is the default import from a new module, `@ember/owner`. With it come two other new types: `RegisterOptions` and `Factory`. + +`Owner` is non-user-constructible because constructing it correctly also requires the ability to provide a factory manager.[^existing-owner-usage] + +[^existing-owner-usage]: Existing usage of the Owner interface this way (e.g. setting custom owners for tests) mostly falls under the "intimate API" rules, and will likely be deprecated after a future introduction of createOwner() hook so that that there is a public API way to get the required type. + #### `RegisterOptions` From a1b21725b5cc49103d67321693d6bc8e3434f45a Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 20 Jul 2022 09:25:53 -0600 Subject: [PATCH 13/14] RFC 821: fix front-matter for linting --- text/0821-public-types.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 4336694eea..94da8c0b16 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -5,10 +5,7 @@ Release Date: Unreleased Release Versions: ember-source: vX.Y.Z ember-data: vX.Y.Z -Relevant Team(s): - - Ember.js - - TypeScript - - Learning +Relevant Team(s): "Ember.js, TypeScript, Learning" RFC PR: https://github.com/emberjs/rfcs/pull/821 --- From e0e6df970d3f27d9105647a7275f4685df1cd76c Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Fri, 29 Jul 2022 12:13:04 -0600 Subject: [PATCH 14/14] Remove hasRegistration --- text/0821-public-types.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/text/0821-public-types.md b/text/0821-public-types.md index 94da8c0b16..503b543021 100644 --- a/text/0821-public-types.md +++ b/text/0821-public-types.md @@ -50,8 +50,6 @@ export default interface Owner { options?: RegisterOptions ): void; - hasRegistration(fullName: string): boolean; - factoryFor(fullName: string): FactoryManager | undefined; } ```