From 5bb493428c332c29a62969ffd6b55cb9567889ad Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 21 Sep 2022 16:49:52 -0600 Subject: [PATCH 1/3] [FEATURE] Test all supported TS versions in CI As part of implementing RFC 0800, introduce support for testing all supported versions of TypeScript against our types. For the moment, the only additional supported version is the TypeScript nightly (`next`) build. When we get to publishing stable types, the current version will be the first version we support, and we will add new versions to the matrix over time in line with the "rolling window" support policy. Additionally, *remove* type checking from the `lint` scripts and create a new set of `type-check` scripts instead. This is pragmatic, not philosophical: we treat it differently from the "lints" here in that we need to be able to run it repeatedly against different versions, whereas the lints are all one-and-done. --- .github/workflows/ci.yml | 88 +++++++++++++++++++++++++++++----- .github/workflows/night-ts.yml | 18 +++++++ package.json | 8 ++-- 3 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/night-ts.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c20a33eb655..67c6864ebc0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,40 @@ jobs: - name: linting run: yarn lint + types: + name: Type Checking (current version) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 12.x + cache: yarn + - name: install dependencies + run: yarn install --frozen-lockfile --non-interactive + - name: Check types + run: yarn type-check + + types-range: + name: Type Checking (other supported versions) + runs-on: ubuntu-latest + needs: ['types'] + strategy: + matrix: + ts-version: ['next'] + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 12.x + cache: yarn + - name: install dependencies + run: yarn install --frozen-lockfile --non-interactive + - name: install TS@${{matrix.ts-version}} + run: yarn add -D typescript@${{ matrix.ts-version }} + - name: Check types with TS@${{matrix.ts-version}} + run: yarn type-check + basic-test: name: Debug and Prebuilt (All Tests by Package + Canary Features) runs-on: ubuntu-latest @@ -61,7 +95,7 @@ jobs: browserstack-test: name: Browserstack Tests (Safari, Edge) runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -87,7 +121,7 @@ jobs: smoke-test: name: Smoke tests (Full Ember Apps) runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v2 @@ -110,7 +144,7 @@ jobs: production-test: name: Production (All Tests + Canary Features) runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -130,7 +164,7 @@ jobs: production-debug-render-test: name: Production (All Tests + Canary Features with Debug Render Tree) runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -151,7 +185,7 @@ jobs: extend-prototypes-test: name: Extend Prototypes runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -172,7 +206,7 @@ jobs: node-test: name: Node.js Tests runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -212,7 +246,7 @@ jobs: browser-test: name: Browser Tests (Firefox) runs-on: ubuntu-latest - needs: [basic-test, lint] + needs: [basic-test, lint, types] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -236,7 +270,18 @@ jobs: deploy-tag: name: Deploy tags to npm runs-on: ubuntu-latest - needs: [basic-test, lint, browserstack-test, production-test, production-debug-render-test, extend-prototypes-test, node-test, blueprint-test, browser-test] + needs: + [ + basic-test, + lint, + browserstack-test, + production-test, + production-debug-render-test, + extend-prototypes-test, + node-test, + blueprint-test, + browser-test, + ] if: startsWith(github.ref, 'refs/tags/v') steps: - uses: actions/checkout@v3 @@ -257,7 +302,18 @@ jobs: publish: name: Publish channel to s3 runs-on: ubuntu-latest - needs: [basic-test, lint, browserstack-test, production-test, production-debug-render-test, extend-prototypes-test, node-test, blueprint-test, browser-test] + needs: + [ + basic-test, + lint, + browserstack-test, + production-test, + production-debug-render-test, + extend-prototypes-test, + node-test, + blueprint-test, + browser-test, + ] # Only run on pushes to branches that are not from the cron workflow if: github.event_name == 'push' && contains(github.ref, 'cron') != true steps: @@ -280,7 +336,18 @@ jobs: publish-alpha: name: Publish alpha from default branch runs-on: ubuntu-latest - needs: [basic-test, lint, browserstack-test, production-test, production-debug-render-test, extend-prototypes-test, node-test, blueprint-test, browser-test] + needs: + [ + basic-test, + lint, + browserstack-test, + production-test, + production-debug-render-test, + extend-prototypes-test, + node-test, + blueprint-test, + browser-test, + ] # Only run on pushes to master if: github.event_name == 'push' && github.ref == 'refs/heads/master' steps: @@ -301,4 +368,3 @@ jobs: S3_BUCKET_NAME: 'builds.emberjs.com' S3_SECRET_ACCESS_KEY: ${{ secrets.S3_SECRET_ACCESS_KEY}} S3_ACCESS_KEY_ID: ${{ secrets.S3_ACCESS_KEY_ID}} - diff --git a/.github/workflows/night-ts.yml b/.github/workflows/night-ts.yml new file mode 100644 index 00000000000..c3b5f3cea45 --- /dev/null +++ b/.github/workflows/night-ts.yml @@ -0,0 +1,18 @@ +name: Nightly TypeScript Run + +jobs: + ts-next: + name: typescript@next + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: volta-cli/action@v1 + - run: yarn install --frozen-lockfile --non-interactive + - run: yarn add -D typescript@next + - run: yarn type-check + +# ...nightly at midnight +on: + schedule: + - cron: 0 0 * * * + workflow_dispatch: diff --git a/package.json b/package.json index 9bbfd57496f..293b8e2d78a 100644 --- a/package.json +++ b/package.json @@ -41,16 +41,16 @@ "lint:docs": "qunit tests/docs/coverage-test.js", "lint:eslint": "eslint --report-unused-disable-directives --cache .", "lint:eslint:fix": "npm-run-all \"lint:eslint --fix\"", - "lint:tsc:stable": "tsc --noEmit", - "lint:tsc:preview": "tsc --noEmit --project type-tests/preview", - "lint:tsc": "npm-run-all lint:tsc:*", "lint:fix": "npm-run-all lint:*:fix", "test": "node bin/run-tests.js", "test:blueprints:js": "mocha node-tests/blueprints/**/*-test.js", "test:blueprints:ts": "EMBER_TYPESCRIPT_BLUEPRINTS=true yarn test:blueprints:js", "test:blueprints": "yarn test:blueprints:js && yarn test:blueprints:ts", "test:node": "qunit tests/node/**/*-test.js", - "test:browserstack": "node bin/run-browserstack-tests.js" + "test:browserstack": "node bin/run-browserstack-tests.js", + "type-check:stable": "tsc --noEmit", + "type-check:preview": "tsc --noEmit --project type-tests/preview", + "type-check": "npm-run-all type-check:*" }, "dependencies": { "@babel/helper-module-imports": "^7.16.7", From 9921e72527239e0261094258dd69ae1d1a9ad269 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 26 Sep 2022 15:50:32 -0600 Subject: [PATCH 2/3] Fix internal issues caught by TS 4.9 TS 4.9 understands the `in` operator and catches two issues for us which earlier versions did not: 1. We were checking `name in model` in `Route#serialize` without checking that `model` there is an object. Given that a route's model is *not* guaranteed to be an object, this was a runtime error waiting to happen. `'a' in 2` will produce `TypeError: 2 is not an Object.' 2. We were checking for `Symbol.iterator in Array` in an internal `iterate()` utility in the `@ember/debug/data-adapter` package. This check is *always* true when targeting ES6 or newer for arrays, which would *appear* to makie the `else` branch a no-op on all supported browsers. Unfortunately, at least some consumers of this API implement a narrower contract for iterables (presumably due to legacy needs to interop with IE11 in Ember < 4.x). In those cases, we really have an iterable, *not* an array. --- packages/@ember/debug/data-adapter.ts | 17 ++++++++++++++++- packages/@ember/routing/route.ts | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/@ember/debug/data-adapter.ts b/packages/@ember/debug/data-adapter.ts index bc3e4c8a7a1..a7cf8ace322 100644 --- a/packages/@ember/debug/data-adapter.ts +++ b/packages/@ember/debug/data-adapter.ts @@ -10,6 +10,7 @@ import { A as emberA } from '@ember/array'; import type { Cache } from '@glimmer/validator'; import { consumeTag, createCache, getValue, tagFor, untrack } from '@glimmer/validator'; import type ContainerDebugAdapter from '@ember/debug/container-debug-adapter'; +import { assert } from '.'; /** @module @ember/debug/data-adapter @@ -34,13 +35,27 @@ type WrappedRecord = { type RecordCallback = (records: Array<{ columnValues: object; object: T }>) => void; +// Represents the base contract for iterables as understood in the GLimmer VM +// historically. This is *not* the public API for it, because there *is* no +// public API for it. Recent versions of Glimmer simply use `Symbol.iterator`, +// but some older consumers still use this basic shape. +interface GlimmerIterable { + length: number; + forEach(fn: (value: T) => void): void; +} + function iterate(arr: Array, fn: (value: T) => void): void { if (Symbol.iterator in arr) { for (let item of arr) { fn(item); } } else { - arr.forEach(fn); + // SAFETY: this cast required to work this way to interop between TS 4.8 + // and 4.9. When we drop support for 4.8, it will narrow correctly via the + // use of the `in` operator above. (Preferably we will solve this by just + // switching to require `Symbol.iterator` instead.) + assert('', typeof (arr as unknown as GlimmerIterable).forEach === 'function'); + (arr as unknown as GlimmerIterable).forEach(fn); } } diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 1e22eebda1d..9e45c5dec7f 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -353,7 +353,7 @@ class Route if (params.length === 1) { let [name] = params; assert('has name', name); - if (name in model) { + if (typeof model === 'object' && name in model) { object[name] = get(model, name); } else if (/_id$/.test(name)) { object[name] = get(model, 'id'); From 87417ab4cc458cecf870e45f47b10d8a06ade668 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 26 Sep 2022 20:35:31 -0600 Subject: [PATCH 3/3] Update to latest @types/node for TS 4.9 fixes --- package.json | 1 + yarn.lock | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 293b8e2d78a..e33c9434d3b 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,7 @@ "@glimmer/validator": "0.84.2", "@simple-dom/document": "^1.4.0", "@tsconfig/ember": "^1.0.1", + "@types/node": "^18.7.23", "@types/qunit": "^2.19.2", "@types/rsvp": "^4.0.4", "@typescript-eslint/eslint-plugin": "^5.38.0", diff --git a/yarn.lock b/yarn.lock index 69feb1907ec..fe1de64e150 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1666,6 +1666,11 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-16.7.10.tgz#7aa732cc47341c12a16b7d562f519c2383b6d4fc" integrity sha512-S63Dlv4zIPb8x6MMTgDq5WWRJQe56iBEY0O3SOFA9JrRienkOVDXSXBjjJw6HTNQYSE2JI6GMCR6LVbIMHJVvA== +"@types/node@^18.7.23": + version "18.7.23" + resolved "https://registry.yarnpkg.com/@types/node/-/node-18.7.23.tgz#75c580983846181ebe5f4abc40fe9dfb2d65665f" + integrity sha512-DWNcCHolDq0ZKGizjx2DZjR/PqsYwAcYUJmfMWqtVU2MBMG5Mo+xFZrhGId5r/O5HOuMPyQEcM6KUBp5lBZZBg== + "@types/node@^9.6.0": version "9.6.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-9.6.0.tgz#d3480ee666df9784b1001a1872a2f6ccefb6c2d7" @@ -7708,9 +7713,9 @@ morgan@^1.10.0: on-headers "~1.0.2" mout@^1.0.0: - version "1.2.4" - resolved "https://registry.yarnpkg.com/mout/-/mout-1.2.4.tgz#9ffd261c4d6509e7ebcbf6b641a89b36ecdf8155" - integrity sha512-mZb9uOruMWgn/fw28DG4/yE3Kehfk1zKCLhuDU2O3vlKdnBBr4XaOCqVTflJ5aODavGUPqFHZgrFX3NJVuxGhQ== + version "1.2.3" + resolved "https://registry.yarnpkg.com/mout/-/mout-1.2.3.tgz#bd1477d8c7f2db5fcf43c91de30b6cc746b78e10" + integrity sha512-vtE+eZcSj/sBkIp6gxB87MznryWP+gHIp0XX9SKrzA5TAkvz6y7VTuNruBjYdJozd8NY5i9XVIsn8cn3SwNjzg== ms@2.0.0: version "2.0.0"