Repository navigation
feat(cli): move the TypeScript template to tsx and TypeScript 6 - #396
Conversation
f38b65f to
ebd510d
Compare
|
One non-blocking suggestion on the two new Java dependency declarations — the fix itself is correct and necessary (jsii 6 publishes these at
implementation "javax.annotation:javax.annotation-api:1.3.2"
implementation "org.jetbrains:annotations:19.0.0"and api 'javax.annotation:javax.annotation-api:1.3.2'
api 'org.jetbrains:annotations:19.0.0'jsii-pacmak publishes both artifacts at Maven compileOnly "javax.annotation:javax.annotation-api:1.3.2"
compileOnly "org.jetbrains:annotations:19.0.0"The practical difference:
Not a blocker: all three scopes compile the generated sources (the |
ebd510d to
9a50385
Compare
|
Thanks — applied, both are now Agreed on the reasoning: One thing I checked before switching the example from Also in this push: the 28 integration fixtures under |
9a50385 to
1a0dc36
Compare
### Related issue No issue — noticed while investigating #373. Independent of #395/#396; based on `main`. ### Description The published `cdktn` tarball carries three files that are build output rather than package content: | file | why it should not ship | | --- | --- | | `tsconfig.json` | Generated by jsii on every build, and `.gitignore`'d for exactly that reason. Publishing it asserts compiler settings consumers should not adopt, and it tracks whichever jsii major built the release. | | `tsconfig.test.json` | Extends the above and points at test sources the tarball does not ship, so it is dangling by construction. | | `lib/tsconfig.tsbuildinfo` | Incremental-build state. Embeds absolute paths from the build machine, which also makes the tarball differ between builds of identical sources. | `packages/cdktn/.npmignore` already excluded `*.ts`, `dist` and `coverage` — it simply had no entry for these. Worth noting for anyone tracing this: npm ignores `.gitignore` entirely once an `.npmignore` exists, which is why the `tsconfig.json` entry already present in `packages/cdktn/.gitignore` had no effect on packaging. ### Why this is safe Nothing resolves a dependency's shipped tsconfig. jsii derives symbol ids from `jsii.tsc` in `package.json` and from the assembly metadata (`jsii/lib/common/symbol-id.js`), never from a tsconfig in the tarball — which is what made constructs 10.8.0 break when it dropped `jsii.tsc.outDir` from its `package.json`. A grep across `packages/`, `tools/` and `test/` finds no reader of a dependency's tsconfig. Verified against the real packaging path (`scripts/pack.mjs`, not just `npm pack --dry-run`): ``` before: 269 files offenders: lib/tsconfig.tsbuildinfo, tsconfig.json, tsconfig.test.json after: 266 files offenders: (none) lib/index.js ✓ lib/index.d.ts ✓ .jsii ✓ ``` ### Scope Only `cdktn` is changed. The other published packages were checked: - `@cdktn/cli-core` ships `templates/typescript/tsconfig.json`, which is the init template's config and **must** keep shipping. - `@cdktn/hcl2cdk` and `@cdktn/provider-generator` ship their own checked-in `tsconfig.json`. Those are real source files, accurate for those packages, and neither has an `.npmignore` — adding one would override the `.gitignore` fallback and change what ships. Left alone deliberately. - Every other package already excludes these. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
so0k
left a comment
There was a problem hiding this comment.
Approving. One non-blocking finding and a sequencing note.
Non-blocking: inaccurate range comment
packages/@cdktn/cli-core/templates/typescript/.hooks.sscaff.js:31-33
// Mirrors the `constructs` peer dependency range declared by the cdktn
// package. Keep both in sync.
installDeps([npm_cdktf, `constructs@10`], false, silent);The two are not equivalent: constructs@10 permits 10.0.0 onward, while the cdktn peer range is ^10.6.0. A fresh install resolves the newest 10.x, so there is no runtime defect — but the "mirrors / keep in sync" instruction is misleading for whoever maintains it next. Either drop the mirroring claim or state the actual relationship (the template intentionally installs a wider spec and relies on the peer range to bound it).
The shortened comments beside the Java compileOnly dependencies keep the rationale that matters (generated import sources need the annotations; jsii 6 publishes them at provided scope; the shared common module is the only one generating sources) — good shrink.
Sequencing
This branch contains #395's commits and targets main (its diff is a superset of #395), so #395 merges first. Since the repo squash-merges, this branch will need a rebase onto main once #395 lands; happy to do that push (maintainer edits are enabled).
### Related issue Fixes #373 Part of #78 (library half; the init-template half is #396) ### Description Moves the jsii toolchain to 6.0. The `cdktn` library is compiled with TypeScript 6; every other package stays on TypeScript 5.x, since jsii bundles its own compiler and nothing forces them to move. | package | before | after | | --- | --- | --- | | jsii | `~5.9.0` | `~6.0.0` | | jsii-rosetta | `~5.9.0` | `~6.0.0` | | jsii-pacmak / codemaker | `1.128.0` | `1.140.0` | | jsii-docgen | `^10.4.20` | `^10.12.6` | | constructs (peer) | `>=10.6.0 <10.8.0` | `^10.6.0` | | TypeScript (jsii packages) | `^5.0.4` | `~6.0.0` | #### Dropping the `constructs` cap instead of widening it The `<10.8.0` cap from #363 is removed rather than replaced with a disjunction, because only 10.8.0 itself was ever broken: | version | `pkg.jsii.tsc` | assembly `metadata.tscOutDir` | | --- | --- | --- | | 10.6.0 | `{outDir, rootDir}` | absent | | **10.8.0** | **absent** | **absent** | | 10.8.1 | `{outDir, rootDir}` | `"lib"` | 10.8.0 satisfies neither lookup path, which is why `cdktn convert` emitted `constructs.Construct` instead of `software.constructs.Construct`. 10.8.1 restores both, is not deprecated on npm (10.8.0 is, with "This version has a bug that breaks downstream jsii consumers"), and is the higher version — so no range resolver can select 10.8.0 without an explicit pin. Keeping the range a simple caret avoids propagating a disjunction into pre-built providers' peer ranges. The `cdktn` devDependency stays at the range floor (`10.6.0`) so the assembly is compiled against the oldest `constructs` it claims to support. That is also what jsii's `metadata/missing-dev-dependency` check wants, so the build is warning-free. Separately, cdktn's own assembly now emits `metadata.tscOutDir` under jsii 6, so it is itself immune to this class of bug for its downstream consumers regardless of package.json shape. #### Class fields now use define semantics jsii 6 emits ES2023, so `useDefineForClassFields` is on. Impact was checked against a build of the same commit with the old toolchain: - The emitted `.d.ts` files are **byte-identical**; the public type surface does not move, and TypeScript 5.4.5 / 5.9.3 consumers still type-check against it cleanly with `skipLibCheck: false`. - `TS2612` (field overwriting a base property) only exists under define semantics — a check jsii 6 newly turns on. It passes across the library, the generated provider bindings, and the cross-package chain into `constructs`. - The compiler's blind spot — dynamically installed base properties — is empty here: all 12 such sites in the library are `Object.defineProperty(this, SOME_SYMBOL, ...)`, and symbol keys cannot collide with the string-named fields define semantics emits. - Generated provider attributes are emitted as `private _x?` backing fields plus `get`/`set` accessors, so the surface that scales to thousands of classes is unaffected by construction. #### Not included The init template stays on TypeScript 5.x here. Moving it to 6 requires replacing ts-node first, which is #396. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes `jsii` reports `Errors: 0 | Warnings: 0`. Integration suites have not been run locally — they need Terraform plus the Java/Python/.NET toolchains — so CI is the first full exercise of the pacmak path for those languages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: vincent de smet <vincent.drl@gmail.com>
ts-node has had no release since v10.9.2 (December 2023) and no commit to main since that month; its maintainer has been asking for sponsorship to restart development since October 2024 (TypeStrong/ts-node#2140). It is already broken outright on TypeScript 7 (TypeStrong/ts-node#2174), and on TypeScript 6 it injects a deprecated `moduleResolution: node10` that the compiler now rejects, which breaks `cdktn synth`. tsx is actively released, has overtaken ts-node in usage, and is the runner Node's own documentation uses to demonstrate full TypeScript support. It is already a devDependency in this repository via @cdktn/hcl2cdk. tsx transpiles without type checking, so `cdktn synth` no longer type checks the app. That is what `npm run build` / `npm run compile` are for, and it matches how this repository runs its own tests. This only affects newly generated projects. Existing projects keep ts-node in their cdktf.json and continue to work until they move to TypeScript 6, at which point they need the same one-line change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New projects now install TypeScript with `>=5.0.0 <7.0.0` instead of `5.x`, which resolves to 6.x. 7.x is the native port and is not supported by jsii yet, so the range is capped below it. This is what open-constructs#78 asked for, and it is only safe now that the template runs on tsx: ts-node fed TypeScript 6 a deprecated `moduleResolution: node10` and broke synth. Two template config changes are needed for TypeScript 6: - `types: ["node", "jest"]`, because TypeScript 6 no longer picks up ambient @types automatically, so `describe`/`it` stopped resolving. - `isolatedModules`, because ts-jest 29.x cannot emit under TypeScript 6 in language-service mode -- it force-overrides `inlineSourceMap` and `tsBuildInfoFile`, and with `noEmitOnError` set TypeScript 6 then reports `emitSkipped`, so every suite fails to run. This puts ts-jest on transpileModule; `npm run build` still type checks. Verified on a generated project against TypeScript 5.4.5, 5.9.3 and 6.0.3: `tsc`, `jest` and `cdktn synth` all pass on each. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cdktf-project.test.ts hardcodes the synth command and ships its own cdktf.json fixture, both of which pinned `npx ts-node`, so they kept exercising ts-node regardless of what the template installs. With the template no longer installing ts-node, those tests failed on TypeScript 6 with the same TS5107 the template change was meant to remove. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestDriver provisions these projects with `cdktn init --template typescript`, which no longer installs ts-node, but each fixture then copies in a cdktf.json pinning `npx ts-node main.ts`. That left the fixtures depending on a package the template no longer provides, and on the ts-node/TypeScript 6 combination the template change exists to avoid. All 28 fixtures used the identical app command, and no snapshot or test asserts on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1a0dc36 to
0b7b1f8
Compare
…late hook (#408) ### Related issue Non-blocking finding from the #396 review; the comment landed on `main` via #395. ### Description `packages/@cdktn/cli-core/templates/typescript/.hooks.sscaff.js` installs `constructs@10` and said it "mirrors the `constructs` peer dependency range declared by the cdktn package — keep both in sync". The two are not equivalent: `constructs@10` permits 10.0.0 onward, while cdktn's peer dependency is `^10.6.0`. There is no runtime defect — a fresh install resolves the newest 10.x, which satisfies the peer range — but the maintenance instruction was misleading. The comment now states the actual relationship: the peer range is the binding constraint, and only the major in the template spec has to follow cdktn's. Comment-only change; no behaviour touched. ### Testing `node --check` and `prettier --check` on the file; nothing else in the repo references the old text. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation if applicable — n/a - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective — n/a, comment-only - [x] New and existing unit tests pass locally with my changes — n/a, comment-only
The init template moved to tsx in open-constructs#396 because ts-node cannot be used with TypeScript 6: it merges a bundled @tsconfig/nodeXX default that supplies a deprecated `moduleResolution: node10`, which the compiler rejects. The examples still ran `npx ts-node main.ts`, so they demonstrated a setup the CLI no longer produces, and copying their config into a generated project would reproduce that failure. The documentation example was already on tsx; its pin is brought in line with the rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings the examples in line with the packages (open-constructs#406) and the init template (open-constructs#396), which now generates projects on TypeScript 6. Only possible now that the examples run on tsx: TypeScript 6 with ts-node fails synth with TS5107. Two config changes are needed, the same ones the template took: - `types: ["node", "jest"]`, because the tsconfigs include `__tests__` and the jest globals otherwise stop resolving. - `isolatedModules` in azure-service-bus-queue-trigger, the one example with a ts-jest preset, which cannot emit under TypeScript 6 in language-service mode. No example sets `baseUrl`, so none of that cleanup applies here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#419) ### Related issue Alignment follow-up to #396 (init template → tsx + TypeScript 6) and #406 (packages → TypeScript 6). The examples were the last part of the repo still on ts-node and TypeScript 5. ### Description Two commits, in the order they have to happen: **1. `chore(examples): run the TypeScript examples with tsx`** (`669eae2`) 21 of the 22 TypeScript examples ran `npx ts-node main.ts`. ts-node cannot be used with TypeScript 6 — it merges a bundled `@tsconfig/nodeXX` default supplying a deprecated `moduleResolution: node10`, which the compiler rejects — so the examples demonstrated a setup the CLI no longer produces, and copying their config into a generated project reproduces that failure. The `documentation` example was already on tsx; its pin is brought in line. **2. `chore(examples): move the TypeScript examples to TypeScript 6`** (`86d7436`) `typescript` goes from `^5.0.0` to `~6.0.0`, plus the same two config changes the template needed: - `types: ["node", "jest"]` — the tsconfigs `include` `__tests__`, and TypeScript 6 otherwise stops resolving the jest globals (`Cannot find name 'describe'`). - `isolatedModules` in `azure-service-bus-queue-trigger`, the only example with a ts-jest preset, which cannot emit under TypeScript 6 in language-service mode. No example sets `baseUrl`, so none of that cleanup applies here. The commits cannot be reordered or split into separate PRs: TypeScript 6 with ts-node is exactly the broken combination, and CI exercises both halves — `tools/build-example.mjs` runs `build` (→ `tsc`, which hits the `describe` error) and then `synth` (→ the app command, which hits TS5107). ### Verification Beyond type-checking, one example was taken end to end locally on TypeScript 6.0.3: ``` examples/typescript/aws-prebuilt tsc --pretty false exit 0 tsx main.ts exit 0, cdktf.out produced ``` The rest are covered by the Examples Integration matrix. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes The diff is 65 files but highly repetitive — three small edits repeated across 22 examples. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
### Related issue Loose end from #406. Noticed while auditing the remaining `ts-node` references after the TypeScript 6 work. ### Description `tools/generate-function-bindings` still ran its four codegen scripts through ts-node after #406 moved the package to `typescript ~6.0.0`: ``` fetch-metadata ts-node scripts/fetch-metadata.ts generate ts-node scripts/generate.ts update-function-matrix ts-node scripts/update-function-matrix.ts generate-function-availability ts-node scripts/generate-function-availability.ts ``` That is the pairing that broke `cdktn synth` for the init template in #396: ts-node merges a bundled `@tsconfig/nodeXX` default supplying a deprecated `moduleResolution: node10`, which TypeScript 6 rejects with `TS5107`. Nothing in CI runs these scripts — they are invoked manually via the root `generate-function-bindings` script — so the risk was silent and would have surfaced the next time someone regenerated the bindings. ### On certainty Whether ts-node actually fails here is environment-sensitive, and I could not settle it: | environment | result | | --- | --- | | CI container (`jsii-terraform`) | reproduced reliably — `tsNodeModuleResolution: Node10` → `TS5107` | | Windows | never reproduced; ts-node resolves no `moduleResolution` at all | | plain `node:22` container | not reproduced — **and the template's own known-failing tsconfig passes there too**, so this check cannot clear anything | Rather than keep chasing why the injection is environment-dependent, this moves to tsx: the same runner the init template, the integration fixtures and the examples already use. That removes the exposure regardless. ### Verification `tsc --noEmit` is clean under TypeScript 6, and three of the four scripts were executed through tsx locally: - `generate-function-availability` — ran, wrote `function-availability.generated.ts` - `generate` — ran clean - `update-function-matrix` — reached its own `Unsupported platform win32/x64` guard, a pre-existing Windows limitation in the script rather than a runner problem, so tsx had loaded and executed it Critically, `git status` showed **no changes to any tracked file** afterwards: the generated output is byte-identical to what is committed, so tsx produces the same result ts-node did. `fetch-metadata` was not run — it performs network fetches — but tsx's ability to load this package's module graph is established by the other three. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes ### Not included `packages/@cdktn/cli-core/src/lib/synth-stack.ts:118` still has a comment reading "increase memory to allow ts-node (when using TypeScript)", which is now stale since the template uses tsx. Left out to keep this scoped; happy to fold in or do separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Related issue
Fixes #78
Stacked on #395 — review/merge that first. Both branches live on a fork, so GitHub will not accept #395's branch as this PR's base; it targets
maininstead, which means the diff currently also contains #395's two commits (53dfa1djsii 6,3a4e69frosetta in the test harness). Once #395 merges, this diff reduces on its own to just the two template commits below.Review only
f2670daandbc85bb4here.Description
Two commits, separable if you'd prefer:
1. Replace ts-node with tsx (
f2670da)cdktn synthruns the app through ts-node, and ts-node cannot be used withTypeScript 6. When a project leaves
moduleResolutionunset, ts-node merges abundled
@tsconfig/nodeXXdefault that suppliesnode10, and passes it as anexplicitly specified option — which TypeScript 6 rejects:
Setting
moduleResolutionin the project's own tsconfig does not help: ts-nodeoverrides it. Instrumenting CI showed the project resolving
Node16undertscwhile ts-node still resolvedNode10from the same config.This is not a passing problem. ts-node has had no release since v10.9.2
(December 2023) and no commit to main since that month; its maintainer has
been seeking sponsorship to restart development since October 2024
(TypeStrong/ts-node#2140), and it is already broken outright on TypeScript 7
(TypeStrong/ts-node#2174).
tsx is actively released, has overtaken ts-node in weekly downloads, and is the
runner Node's own documentation uses
to demonstrate full TypeScript support. It is already a devDependency in this
repository via
@cdktn/hcl2cdk.Trade-off: tsx transpiles without type checking, so
cdktn synthno longer typechecks the app.
npm run build/npm run compilestill do, and this matcheshow the repository runs its own tests (
@swc/jest).2. Move the template to TypeScript 6 (
bc85bb4)typescript@5.xbecomes>=5.0.0 <7.0.0. Two config changes are required:types: ["node", "jest"]— TypeScript 6 no longer picks up ambient@typesautomatically, so
describe/itstopped resolving.isolatedModules— ts-jest 29.x cannot emit under TypeScript 6 inlanguage-service mode (
emitSkipped), so every suite fails to run. This putsit on
transpileModule;npm run buildstill type checks.Scope
Template only. Existing projects keep
ts-nodein theircdktf.jsonand areunaffected until they move to TypeScript 6, at which point they need the same
one-line change — worth a changelog note. The
test/typescript/*/cdktf.jsonfixtures still pin
npx ts-node main.ts; migrating those is left as afollow-up so this diff stays easy to read.
Checklist
Verified on a generated project against TypeScript 5.4.5, 5.9.3 and 6.0.3 —
tsc,jestandcdktn synthall pass on each:🤖 Generated with Claude Code