Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Introduce preview types #20180

Merged
merged 39 commits into from Sep 6, 2022
Merged

[FEATURE] Introduce preview types #20180

merged 39 commits into from Sep 6, 2022

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Sep 1, 2022

Summary

This PR introduces public types to Ember.js for the first time, as part of the effort to implement RFC 0800: TypeScript Adoption Plan. 🚀

This PR does not finish the work of publishing types in Ember itself, however. Instead, it intentionally provides a means by which we can get early feedback, in line with Ember's normal path to stabilizing features. Users can opt into the types here, but no one will be interacting with these types automatically. This approach also lays the foundation for future work.

Since this is a significant and since the details of how we are doing this are notable, the rest of this comment explains the details of what these types do and do not support, the mechanics behind this approach, why we are using those mechanics, and the rest of the path to publishing stable types for Ember, including how this approach supports it.

Much of the content below can and will be repurposed for a blog post announcing these features (appropriately restructured and in some places reworded).

(Part 1 of #20174.)

Demo

Screen.Recording.2022-08-31.at.21.05.08.mov

Implementation details

Introduce types in a new types/preview directory, with a corresponding type-tests/preview directory. The types directory is intended to be the long-term home for all types published from Ember, whether ambient types (as in this case) or published from source in the future. The type-tests directory may or may not be a permanent addition, depending on how we choose to test the public API for the types published from Ember's own source: many type tests already exist within the repository, colocated with the packages they are testing.

The types introduced here are copied from DefinitelyTyped, but with the following significant changes:

  • removal of support for Ember Classic features (see below)
  • a number of significant bug fixes, which we will not be porting back to DefinitelyTyped
  • the use of expect-type type assertion library instead of DTSLint to validate the types
  • updates to how the types are structured to support consumption without TypeScript's special support for @types packages

The structure of the types/preview directory mirrors the structure of the packages directory, with a root tsconfig.json in types/preview that simply extends from the base tsconfig.json maintained by Typed Ember, so that its types reflect the strictness and best practices required by the relevant Ember RFCs as well as the Semantic Versioning for TypeScript Types spec.

Each file within the types/preview directory defines the module it corresponds to in the packages directory using declare module syntax. A root file at types/preview/index.d.ts explicitly imports each of those files. This makes those module definitions visible for anyone who imports that path. A typesVersions key in package.json maps ember-source/preview to types/preview, so users will be able to import the types without being exposed to the internal layout. See below under Mechanics for more details of how this will work for end users.

To make sure all of this continues to work over time, the PR also introduces new scripts under lint:tsc in package.json. Currently those scripts check the main code base and the preview types; when we introduce stable types, we can run a check against those as well if we so choose.

Ember Classic support

RFC 0800 specified the basic shape of what these types do and do not support. Quoting the RFC:

Per the edition support policy, we will provide minimal support for Ember Classic features:

  • Ember's classic class system: we will provide minimal definitions for the .extend(), .reopen(), .reopenClass(), and .create() methods, which make no attempt to use them to actually update the types of the items they modify.…

  • Ember’s get and set helpers: we will not provide types to make get and set type-safe beyond simple property lookups on objects—i.e. no support for nested path lookups.…

  • Classic computed property handling: we will not provide “safe” types for the classic form of computed properties.

See the RFC for additional details; this PR implements the RFC exactly.

Additionally, this work exposed a number of errors in the existing types, especially around Array prototype extensions (see 1dc0e55 for details). As a result, these types do not support Array prototype extensions, and it is unlikely that future work will be able to add that support. (The support provided via the types on DefinitelyTyped only worked because the types were defined incorrectly, resulting in a variety of kinds of unsafety.)

Type registries

These types, as a fairly direct extraction from DefinitelyTyped, currently maintain the service and controller type registries. Given the lack of support for classic computed properties, which are the only way to take advantage of those at present, it is extremely likely these will be removed before stabilizing the types.

Legacy routing type locations

In line with RFC 0821: Public API for Type-Only Imports, this PR also removes support for importing the types for Transition, RouteInfo, and RouteInfoWithMetadata from the private locations that DefinitelyTyped presently supports for backwards compatibility. Users will need to migrate to using the correct import paths when switching to use these imports.

Introducing a Resolver type

The types on DefinitelyTyped supply a definition of Resolver from ember-resolver, which is where most Ember users get their resolver. However, ember-resolver and other resolvers work because they implement Ember’s contract for what a resolver is. A future RFC will introduce a public type import for this. (It was missed during the work on RFC 0821 because the type presently does not come from Ember!)

For now, the type exists at @ember/-internals/resolver, and is introduced to be type-compatible with the type for ember-resolver on DefinitelyTyped. (See ember-cli/ember-resolver#795 for an issue tracking publishing types from ember-resolver, which is likely gated on a public type import from Ember, but until we ship stable types, can be managed via careful types work on DefinitelyTyped.)

Mechanics

Users will opt into these types by using a type-only import. During the preview period, that import will be from ember-source/types/preview:

import 'ember-source/preview'

This is a “side-effect” type imports. The contents of types/preview/index.d.ts is a list of more “side-effect” type imports: one for every module which is part of Ember's type definitions. Those further modules wrap each module definition in a declare module '<name>' {} so that importing it makes the module visible in the type system. This makes it so that users can use the types by simply writing the single import show above.

This approach will scale forward to all sorts of future work in TS in Ember, including publishing at ember-source/types/stable, or for future editions e.g. at ember-source/types/polaris. Moreover, this allows these to happen simultaneously. That is, as we publish types from source, those can go directly into the stable directory and users can simply have both imports present. Assuming careful use of package.json configuration to specify the types so that ember-source resolves correctly, users will be able to write this:

import 'ember-source';
import 'ember-source/preview';

Similarly, because this relies on module declarations, module merging works and this will allow us to at some point publish stable types supporting only new editions, with the default type import being at stable but users being able to opt into types which support the old edition:

import 'ember-source';
import 'ember-source/classic';

Or, for a preview of Polaris:

import 'ember-source';
import 'ember-source/polaris-preview';

And so on.

While this is a nice capability, it is also (and more importantly) necessary. ember-source represents the Ember CLI/build entry point for consuming Ember, not the packages consumed by Ember app or addon authors. Users therefore need some way to make the module declarations visible to them, and for those module declarations to work with declaration merging to support tools like Glint. Additionally, the Typed Ember team considers it critical to avoid putting references to direct node_modules paths in the tsconfig.json. This approach solves all of those problems neatly.

Critically, it is also forward-compatible with a world where packages like @ember/routing are normal packages shipping normal ESM on disk and provided by: users could depend on those packages directly, or could continue to depend on a “metapackage” like ember-source. In the former case, the types would appear automatically using normal TypeScript/Node resolution rules. In the latter case, the metapackage would continue to supply the same kind of import to make all the transitive dependencies “visible” to the type system.

Future work

With this structure in place, we can begin publishing types in a stable way as soon as our tooling supports it, with the following basic pieces to be done:

  • Update CI to test against multiple supported versions of TypeScript. Until we begin publishing types as stable, this will not necessarily follow our SemVer policy for TS support versions.

  • Introduce a types key which points to a types/stable/index.d.ts which in turn imports modules in the same way as the preview types files does.

  • Iteratively begin publishing types from source and removing them from the preview directory once we have cleared the prerequisites.


It’s happening! 🚀

Copy the types from DefinitelyTyped, and set up a bare minimum config
to make it possible to iterate on them. This particular commit fails all
type checking and has an enormous amount of work to do, but provides a
foundation on which we can iterate.
- Improve the way we do a minimal representation of the Ember Object
  primitives and utilities.
- Introduce our own copy of `@ember/string`.
- Start workk on many of the tests for `Ember` namespace re-exports.
- Stop distinguishing between Octane and everything else: we only care
  about Octane anyway in our public *TS* API, per RFC 0800.
This file is a hot mess, and provides more and further evidence of two
increasingly-obvious realities from a TS POV:

- The Ember namespace is 99% useless; people should just use the
  module imports instead.
- The Classic types are *awful*, because Classic *code* was a mess
  compared to Octane, because ES5 was a mess.
These allows us to make `get` or `getProperties` correctly handle index
access the same way direct field access would. This gets us back to the
same level of safety we had with our old `UnwrapComputedProperty*`
types for this kind of index access: those types did *many* things, but
included among them was distributing across the requested properties.
While we would prefer this to work, it is not a hard necessity, for (at
least) two reasons:

1. Use of `Ember.get` vs. the import from `@ember/object` is rare and
   should be discouraged.

2. Use of `get` is itself fairly unusual to *need* these days; most
   uses either will not hit this case, as in the deep key lookup (which
   is always just `unknown` anyways) or should be trivially replaced
   with direct property access (since Ember 3.1!).
Unlike in the previous pass, this does not try to handle index access
'correctly', instead choosing to just push people to use normal JS if
they want that safety.
- fix runloop test
- provide an internal-only type for Resolver` that should be
  compatible with the one in `@types/ember-resolver`
1.  Introduce a root import, `types/preview/index.d.ts`, which itself
    imports every module which is part of Ember's type definitions.
    Then wrap each module definition in a `declare module '<name>' {}`
    so that importing it makes the module visible in the type system.
    This makes it so that users can use the types by simply writing a
    single import:

    ```ts
    import 'ember-source/types/preview';
    ```

    This approach will scale forward to all sorts of future work in TS
    in Ember, including publishing at `ember-source/types/stable`, or
    for future editions e.g. at `ember-source/types/polaris`. Moreover,
    this allows these to happen *simultaneously*. That is, as we publish
    types from source, those can go directly into the `stable` directory
    and users can simply have both imports present:

    ```ts
    import 'ember-source/types/preview';
    import 'ember-source/types/stable';
    ```

    Similarly, because this relies on module declarations, module
    merging works and this will allow us to at some point publish
    stable types supporting *only* new editions, with the default type
    import being at `stable` but users being able to opt into types
    which support the old edition:

    ```ts
    import 'ember-source/types/stable';
    import 'ember-source/types/classic';
    ```

2.  Extract the type tests for the published types into a `type-tests`
    directory, which actually *uses* those mechanics to validate that
    the types are all importable and usable as expected with that
    single import statement.

    Do *not* include that directory in `files` in `package.json`, so the
    tests are never published, only the type definitions themselves.

Making those changes and getting the test suite passing also
highlighted a number of errors in the existing type definitions (and
tests!) for `EmberArray` and its related types, which had historically
been masked by the way our tests incorporated the prototype extensions:
*all* arrays appeared to act as a `NativeArray`. Fixing that involved
pulling those type tests out into a dedicated test package, and then
fixing all the bugs in the existing type definitions, including
restructuring to match the *actual* structure of Ember's internals.

However, fixing the *regular* types for arrays *also* highlighted that
it is currently impossible to properly represent the array prototype
extensions. Accordingly, those are excluded from this, and will be
addressed in some other way.
These *do not work*. They will be removed in the next commit, but are
added here for historical purposes.

They do not work specifically because they introduce a circularity in
the definition of `NativeArray`: `NativeArray` must extends from a
*subset* of `Array` to correctly represent its public API and behavior,
but introducing the `Array` prototype extension in turn must extend from
`NativeArray`, which results in type errors because `NativeArray`
extends from the subset rather than the full API of `Array`.

This "worked" in the DefinitelyTyped version for two reasons:

1.  All of our type tests actually just assumed the array prototype
    extensions were enabled (as discussed in the previous commit).

2.  The type definition for `NativeArray` was *wrong*: at minimum it was
    substantially out of date; possibly it has always been incorrect.
    (It may have been incorrect *intentionally*, precisely to allow the
    prototype extension to work. The details are lost to time.)

In any case, these type tests represent the "correct" APIs for the
prototype extensions, so are committed here and removed in the next
commit for historical purposes.
See previous commit message for discussion of *why* these are being
removed. If we decide to re-introduce these types later, it will require
not only adding back in these tests but also taking some fairly distinct
approach to their inclusion, given the problems with the way they work.
Use `typesVersions` to resolve `preview` to `types/preview`; we can use
`types` for stable and more `typesVersions` (or, eventually, `exports`)
for other similar schemes in the future.
We continue to maintain support for these import locations in the types
on DefinitelyTyped, but remove them here so that users trying the
preview types do not accidentally depend on them.
While having some guidance for the ambient types here would be needful
for the long term, we do not intend to *keep* these all that long; this
is intended as transitional. If we end up maintaining the ambient types
for a long time, we can of course revisit this. In any case, we will
want to run linting on any type tests we introduce for *stable* types
(at a future `type-tests/stable` location). Net, exclude everything in
`types` and the tests in `type-tests/preview`.
Copy link
Contributor

@dfreeman dfreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The high-level change makes sense to me, just a couple of questions mainly for my own edification. Super excited to see this getting ready to land!

types/preview/index.d.ts Outdated Show resolved Hide resolved
types/preview/ember/index.d.ts Show resolved Hide resolved
The corresponding module declarations were removed earlier; this simply
drops the 'side-effect' imports from `types/preview/index.d.ts`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature TypeScript Work on Ember’s types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants