Repository navigation
chore: emit compiled TS to build/ instead of next to sources - #116
Conversation
10cb0c1 to
4b15713
Compare
|
I agree splitting emit target is a good practice, but not sure if modifying workspaced packages to use the build output is the best approach across all packages? |
|
Hi @so0k, thanks for the comment. Can you clarify what your concern is? The workspace packages were already using the build output already, this PR doesn't change that. |
4b15713 to
ea36697
Compare
|
I've been going a bit back and forth on this. In regards to the
In regards to the
|
|
Thanks for the feedback @jsteinich. In response to your comments: Changes from
|
|
I'm fine with the changes. |
|
it’s ok, my main concern was the requirement that you’d have to build before cross pkg TS resolver works? but maybe the tsconfig already takes care of this i’m more familiar with TurboRepo recommended set up where internal pkgs are JIT compiled (package.json points to src/*ts files instead of build but i think all of the CDKTF pkgs are published so they all need to build if you want to avoid heavy lifting in tsconfig / eslint set up i haven’t tried the changes locally but if you have no concerns then I withdraw my concern |
|
For info, you had to build the packages already before this change, as their |
**Breaking changes** ### Python 3.8 is no longer supported We have dropped support for Python 3.8, please update to Python 3.9 or higher. ### Go 1.18-1.24 are no longer supported We have dropped support for Go 1.18 - 1.24, please update to Go 1.25 or higher. ### TypeScript versions less than 4.5 are no longer suppored Provider generation now uses features from TypeScript 4.5. Note that there isn't an official minimum supported TypeScript at this time and using the latest 5.x version is recommended. **Upcoming Breaking Changes** Node version 22.x will become the minimum in an upcoming release as 20.x is now EoL. ### feat - feat(provider-generator): use type modifier on imports [\#150](#150) - feat(lib): add StringMapMap class [\#110](#110) ### chore - chore: clean up yarn install warnings [\#149](#149) - chore(deps): bump the github-actions-backward-compatible group with 6 updates [\#146](#146) - chore: Fix eslint warnings - phase 5 [\#140](#140) - chore: Fix eslint warnings - phase 4 [\#139](#139) - chore: Fix eslint warnings - phase 3 [\#138](#138) - chore: Fix eslint warnings - phase 2 [\#137](#137) - chore: Fix eslint warnings - phase 1 [\#135](#135) - chore(provider-generator): Update retry to terraform init for transient GitHub errors [\#134](#134) - chore: Remove unused dependencies from packages - phase 1 [\#133](#133) - chore(provider-generator): Add retry to terraform init for transient GitHub errors [\#132](#132) - chore(provider-generator): enable typescript declarationMap [\#131](#131) - chore: Fix Java flaky integration test [\#130](#130) - chore: Add missing dependencies to packages [\#129](#129) - chore: Standardise test scripts to only run tests [\#128](#128) - chore: Remove edited from the examples workflow pull_request types [\#127](#127) - chore: Add retry to build-example [\#126](#126) - chore: Bump verdaccio version and add npm install retries [\#122](#122) - chore: Remove `edited` from the test workflow pull_request types [\#121](#121) - chore: Replace depcheck CI check with knip [\#120](#120) - chore: Update test providers to support darwin arm64 [\#119](#119) - chore: Refine eslint configuration [\#118](#118) - chore: emit compiled TS to build/ instead of next to sources [\#116](#116) - chore: Add VSCode Settings and Extension Recommendations [\#114](#114) - chore: Fix eslint auto-fixable issues [\#113](#113) - chore: use new docker image in builds [\#108](#108) - chore: Update docker image to have Go 1.25 and Node 22 [\#97](#97) - chore: Upgrade dependencies for all [\#91](#91) - chore: Upgrade jest to v30 [\#89](#89) - chore(deps): bump the github-actions-backward-compatible group with 4 updates [\#88](#88) - chore(deps): bump the github-actions-breaking group with 5 updates [\#87](#87) - chore: add migration context to bug report issue templates [skip ci] [\#86](#86) - chore: npm-check-updates && yarn upgrade [\#84](#84) - chore: Upgrade dependencies for all [\#83](#83) - chore(lib): add test showing no failure if resource scoped to app [\#73](#73) - chore(deps): Run npx yarn-audit-fix --force [\#59](#59) - chore(cli): Replace node-pty with simple interactive process [\#55](#55) - chore: Upgrade dependencies for JSII [\#20](#20) ### fix - fix(cli): anchor jest testPathIgnorePatterns in typescript template [\#147](#147) - fix(tests): Fix app.test and json-to-hcl.test race condition [\#125](#125) - fix(lib): ForExpression missing iteratorContext and OperatorExpression drops falsy right operands [\#112](#112) - fix(cli): support pre-built provider metadata with cdktn key [\#107](#107) - fix(cli): don't check cdktf-cli version [\#99](#99) - fix(deps): replace archiver with yazl [\#95](#95) - fix(lib): fail on constructs outside of stacks (under feature flag) [\#93](#93) - fix(gha): Fix GitHub release creation [\#90](#90)
### Description ## Summary Migrates the `cdktn` package to the standard `src/` (sources) + `lib/` (build output) layout used by every other workspace package. Sources and compile output were previously co-located in `lib/`, which caused stale `.js`/`.d.ts` artifacts to confuse knip after every local `yarn build` (false-positive "unused dependency" reports for `json-stable-stringify`, `yazl`, `@types/json-stable-stringify`, `@types/semver` because TS module resolution was preferring the stale `.d.ts` typings over the actual `.ts` sources). ## Why - After [#116](#116) migrated other packages to `src/` → `build/`, `cdktn` was the only holdout still emitting compiled output next to sources. - Mixed layout meant `knip` would silently fail locally after a build, even though it was clean in CI (CI runs knip on a fresh checkout with no compiled artifacts). ## What changed **Source layout:** - 85 `.ts` source files moved: `packages/cdktn/lib/**/*.ts` → `packages/cdktn/src/**/*.ts` (subdir layout preserved: `backends/`, `functions/`, `hcl/`, `private/`, `synthesize/`, `testing/`, `tokens/`, `validations/`) - `package.json` `"main"` and `"types"` continue to point at `lib/index.js` / `lib/index.d.ts` — distribution surface is unchanged. **Build configuration ([packages/cdktn/package.json](packages/cdktn/package.json)):** - Added `"jsii"."tsc": { "outDir": "lib", "rootDir": "src" }` — JSII regenerates `tsconfig.json` to compile from `src/` and emit to `lib/`. - Added `"jsii"."excludeTypescript": ["src/**/__tests__/**"]` — three colocated `__tests__` files inside `src/testing/` import from `test/helper/` (outside `rootDir`); excluding them keeps JSII's `rootDir` constraint satisfied. Jest still picks them up via `testMatch`. **Test wiring:** - 33 test files + 5 helpers updated: `from "../lib"` → `from "../src"` (and `../../lib` → `../../src` for nested `test/helper/` and `test/tokens/` files). - Added [packages/cdktn/tsconfig.test.json](packages/cdktn/tsconfig.test.json) — extends the JSII-generated `tsconfig.json` but widens `rootDir` to `.` and excludes `lib/**/*.d.ts`. Required because (a) test files live outside `rootDir: src` and ts-jest can't compute output paths for out-of-tree files, and (b) without excluding `lib/`, ts-jest sees both `src/terraform-count.ts` and `lib/terraform-count.d.ts` as separate type declarations and rejects assignment between them. - Updated [jest.config.js](packages/cdktn/jest.config.js) to point ts-jest at `tsconfig.test.json`. **Knip + docs:** - [knip.jsonc](knip.jsonc) — `packages/cdktn` workspace project glob updated to `["src/**/*.ts", "test/**/*.ts"]`; entry list dropped (knip auto-detects `src/index.ts`). - [CLAUDE.md](CLAUDE.md), [CONTRIBUTING.md](CONTRIBUTING.md), [tools/generate-function-bindings/README.md](tools/generate-function-bindings/README.md) — `cdktn/lib/...` paths in docs updated to `cdktn/src/...`. ### 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 - [ ] 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/tree/main/website) 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 <!-- If this is still a work in progress, feel free to open a draft PR until you're able to check off all the items on the list above --> --------- Co-authored-by: Simon Heather <simon.heather@yulife.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 #115
Description
build/folder in the seven@cdktn/*packages so that.js/.d.tsno longer land next to.tssources. Each package now setsrootDir: ./srcandoutDir: ./build, withmain/typespointing atbuild/.lib/→src/in@cdktn/provider-generator,@cdktn/hcl2cdk, and@cdktn/hcl2jsonso every package consistently usessrc/for sources andbuild/for output.@cdktn/hcl2cdkno longer imports@cdktn/hcl2json/lib/syntax-tree(collapsed onto the existingTFExpressionSyntaxTreenamespace import), and threehcl2jsontest files move from../libto../src.Changes per package
@cdktn/cli-coremain:src/lib/index.js→build/lib/index.js@cdktn/commons@cdktn/provider-schema@cdktn/hcl-toolswasm/untouched;build-go.shunchanged@cdktn/provider-generatorlib→srcrename;postbuildpath updated; dropped vestigial cross-package include for../commons/src/module-schema.ts(commons re-exports it viaindex.ts)@cdktn/hcl2cdklib→srcrename; narrowed tsconfigincludefrom**/*.tstosrc/**/*.tssotest/is no longer compiled to disk (ts-jest runs tests directly)@cdktn/hcl2jsonlib→srcrename; Go WASM glue untouchedChecklist