[FEATURE beta] Introduce preview types #20193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`.
The corresponding module declarations were removed earlier; this simply drops the 'side-effect' imports from `types/preview/index.d.ts`.
wagenet
approved these changes
Sep 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This back-ports #20180 to the 4.8 beta branch, per discussion among Framework Core. (It was intended to land before we cut
4.8.0-beta.1but landed just after.) We’ll cut4.8.0-beta.2with this included, and I’ll publish a blog post about it!