From 84851831b468d0b7b15c5af953ea1c0735ef1d5d Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Fri, 16 Dec 2022 09:58:43 -0700 Subject: [PATCH] Feature: introduce native (ambient) TS types - 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). --- .github/workflows/ci.yml | 23 +++ README.md | 3 + docs/migration.md | 46 ++++++ package.json | 40 ++++- types/index.d.ts | 272 +++++++++++++++++++++++++++++++ types/local-types.d.ts | 2 + types/tests.ts | 344 +++++++++++++++++++++++++++++++++++++++ types/tsconfig.json | 11 ++ yarn.lock | 147 ++++++++++++----- 9 files changed, 842 insertions(+), 46 deletions(-) create mode 100644 types/index.d.ts create mode 100644 types/local-types.d.ts create mode 100644 types/tests.ts create mode 100644 types/tsconfig.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f4a8c00..e6d3ddb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,3 +64,26 @@ jobs: run: yarn install --frozen-lockfile - name: test run: node_modules/.bin/ember try:one ${{ matrix.ember-try-scenario }} --skip-cleanup + + types: + runs-on: ubuntu-latest + + needs: test + + strategy: + fail-fast: false + matrix: + ts-version: + - 4.8 + - 4.9 + - next + + steps: + - uses: actions/checkout@v2 + - uses: volta-cli/action@v4 + - name: install dependencies + run: yarn install --frozen-lockfile + - name: install TS version + run: yarn install --dev typescript@${{matrix.ts-version}} + - name: test types + run: yarn test:ts diff --git a/README.md b/README.md index 65a9d6e0..85f6449f 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,9 @@ Requirements * Ember.js v3.28 or above * Ember CLI v3.28 or above * Node.js v14 or above +- TypeScript 4.8 and 4.9 + - SemVer policy: [simple majors](https://www.semver-ts.org/#simple-majors) + - The public API is defined by the [Usage][#usage] section below. If you need support for Node 13 or older Ember CLI versions please use v4.x of this addon. diff --git a/docs/migration.md b/docs/migration.md index 8e8300e9..1db27f29 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -2,6 +2,52 @@ Migration Guide ============================================================================== +Migrating to native TypeScript support in v6.1.0 +------------------------------------------------------------------------------ + +The types for the QUnit `TestContext` provided by the `ember-qunit` and `@ember/test-helpers` types on DefinitelyTyped made a choice to prioritize convenience over robustness when it came to what methods and values were available on `this` in any given test: they made *all* methods availabe regardless of what your setup actually involved. For example, this totally invalid code would have passed the type checker: + +```ts +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { hbs } from 'ember-cli-htmlbars'; + +module('bad times', function (hooks) { + setupTest(hooks); + + test('this will not *run* correctly', async function (assert) { + await this.render(hbs`

whoopsie

`); + }); +}) +``` + +To resolve this, you need to explicitly specify what `this` is for different kinds of tests: + +```ts +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import type { RenderingTextContext } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +module('better times', function (hooks) { + setupTest(hooks); + + test( + 'this will not *run* correctly', + async function (this: RenderingTextContext, assert) { + await this.render(hbs`

whoopsie

`); + } + ); +}) +``` + +While annoying, this is accurate and prevents the annoying mismatch. Combined with support for using local scope with `