Feature: introduce native (ambient) TS types #995
Merged
+842
−46
Conversation
This file contains hidden or 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
- Add type definitions and type tests. - Explicitly specify the SemVer policy in the README. - Configure CI to run type tests against the package. - Add a note to the migration docs about how to update to use these. This will shortly be superceded by types published from source *and* using the types published by Ember itself. Publishing this ahead of this provides a path for teams to start using `ember-qunit` with TypeScript *without* pulling it from DefinitelyTyped, allows us to remove `ember-qunit` from DefinitelyTyped entirely, and does not block either of those wins on the timeline of a full TS conversion. To make this work, we need (because one of our dependencies needs) a peer and dev dep on `@glimmer/interfaces` and `@glimmer/interfaces`, so that types which use those will type check. This is: annoying in the extreme. We will want to keep thinking about how to tackle this as an ecosystem; for now, they are marked as *optional* so no one will have things blow up as a result of this at least (and that's also correct: we *only* need them for TS).
chriskrycho
commented
Dec 16, 2022
ijlee2
reviewed
Dec 17, 2022
chriskrycho
added a commit
that referenced
this pull request
Dec 19, 2022
I originally [thought][1] these `peerDependencies` were not breaking because users would be able to opt into them. Unfortunately, this is [not the case][2]: users on Ember 3.28 will have an older copy of these dependencies, so correctly validating these breaks consumers. [1]: #995 (comment) [2]: #997 (comment) We should bundle these into a v7.0.0 when we drop Node 14 and Ember 3.28, presumably in ~April 2023.
chriskrycho
added a commit
that referenced
this pull request
Dec 19, 2022
I originally [thought][1] these `peerDependencies` were not breaking because users would be able to opt into them. Unfortunately, this is [not the case][2]: users on Ember 3.28 will have an older copy of these dependencies, so correctly validating these breaks consumers. [1]: #995 (comment) [2]: #997 (comment) We should bundle these into a v7.0.0 when we drop Node 14 and Ember 3.28, presumably in ~April 2023. Fixes #997
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 will shortly be superceded by types published from source and using the types published by Ember itself. Publishing this ahead of this provides a path for teams to start using
ember-qunitwith TypeScript without pulling it from DefinitelyTyped, allows us to removeember-qunitfrom DefinitelyTyped entirely, and does not block either of those wins on the timeline of a full TS conversion.To make this work, we need (because one of our dependencies needs) a peer and dev dep on
@glimmer/interfacesand@glimmer/reference, so that types which use those will type check. This is: annoying in the extreme. We will want to keep thinking about how to tackle this as an ecosystem; for now, they are marked as optional so no one will have things blow up as a result of this at least (and that's also correct: we only need them for TS).