fix(build): resolve TypeScript 6.0.3 migration CI failures#545
Conversation
Bumps [typescript](https://github.com/microsoft/TypeScript) from 5.9.3 to 6.0.3. - [Release notes](https://github.com/microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v5.9.3...v6.0.3) --- updated-dependencies: - dependency-name: typescript dependency-version: 6.0.3 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…pt-6.0.3' into fix/typescript-6-migration-checks
- Update jest.config.js inline tsconfig ignoreDeprecations from 5.0 to 6.0 - Exclude test files from validation/types package build tsconfigs - Add spec/test excludes to hooks tsconfig.build.json for consistency - Update workspace-manifests test assertions for TypeScript 6.0.3
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
github-actions
Bot
added
documentation
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| emitDecoratorMetadata: false, | ||
| experimentalDecorators: true, | ||
| ignoreDeprecations: '5.0', | ||
| ignoreDeprecations: '6.0', |
There was a problem hiding this comment.
The ignoreDeprecations option is not a valid TypeScript compiler option and will be ignored by ts-jest and TypeScript. This setting is likely intended for Node.js or another tool. To avoid confusion and ensure maintainability, remove this option from the tsconfig section and, if necessary, set it in the appropriate configuration (e.g., Node.js runtime flags).
Recommended change:
// Remove this line from the tsconfig section
// ignoreDeprecations: '6.0',| const jestConfig = require(path.join(repoRoot, 'jest.config.js')); | ||
| const transform = jestConfig.transform['^.+\\.(ts|tsx|js|jsx)$']; | ||
|
|
||
| expect(transform[1].tsconfig.ignoreDeprecations).toBe('5.0'); | ||
| expect(transform[1].tsconfig.ignoreDeprecations).toBe('6.0'); | ||
| }); | ||
|
|
||
| it('declares a root TypeScript version for Jest transforms', () => { | ||
| const manifest = readJson('package.json'); | ||
|
|
||
| expect(manifest.devDependencies).toHaveProperty('typescript', '5.9.3'); | ||
| expect(manifest.devDependencies).toHaveProperty('typescript', '6.0.3'); | ||
| }); |
There was a problem hiding this comment.
These tests assume that configuration files (e.g., jest.config.js, package.json) and their internal structures are always present and valid. If these files are missing or their structure changes, the tests will throw runtime errors rather than failing gracefully. Consider adding checks to ensure the files and expected properties exist before making assertions, so test failures are informative and not due to uncaught exceptions.
| const jestConfig = require(path.join(repoRoot, 'jest.config.js')); | ||
| const transform = jestConfig.transform['^.+\\.(ts|tsx|js|jsx)$']; | ||
|
|
||
| expect(transform[1].tsconfig.ignoreDeprecations).toBe('5.0'); | ||
| expect(transform[1].tsconfig.ignoreDeprecations).toBe('6.0'); | ||
| }); | ||
|
|
||
| it('declares a root TypeScript version for Jest transforms', () => { | ||
| const manifest = readJson('package.json'); | ||
|
|
||
| expect(manifest.devDependencies).toHaveProperty('typescript', '5.9.3'); | ||
| expect(manifest.devDependencies).toHaveProperty('typescript', '6.0.3'); | ||
| }); |
There was a problem hiding this comment.
The tests are tightly coupled to specific configuration values (e.g., TypeScript version '6.0.3', Jest transform settings). This can make the tests brittle and require frequent updates as configuration changes. Consider parameterizing these values or documenting the rationale for hardcoding them to improve maintainability.
📝 WalkthroughWalkthroughTypeScript is upgraded from 5.9.3 to 6.0.3 across workspace packages. Compiler deprecation settings, Node16 module configuration, test-file exclusions, and related Jest manifest assertions are updated. ChangesTypeScript 6 migration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/validation/tsconfig.json (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
__tests__appears in bothincludeandexclude, making the include entry ineffective.TypeScript's
excludetakes precedence overinclude, so the__tests__directory is fully excluded from compilation despite being listed ininclude. This contradiction was introduced by adding__tests__to theexcludearray. Either remove"__tests__"fromincludeif tests should be excluded, or remove it fromexcludeif tests should be type-checked.♻️ Proposed fix (assuming tests should be excluded from this config)
-{"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist","__tests__","**/*.spec.ts","**/*.test.ts"]} +{"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src"],"exclude":["node_modules","dist","__tests__","**/*.spec.ts","**/*.test.ts"]}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/validation/tsconfig.json` at line 1, Resolve the contradictory TypeScript configuration in the validation package’s tsconfig: choose whether tests should be compiled, then remove "__tests__" from either the include or exclude array accordingly. Based on the review’s proposed assumption, remove "__tests__" from include while keeping it excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/types/tsconfig.json`:
- Line 1: Resolve the contradictory test configuration in the package TypeScript
configs. In tsconfig.json, preserve test type-checking by removing __tests__ and
test-file patterns from exclude; if build-only exclusions are required, add a
separate tsconfig.build.json extending tsconfig.json with include set to src and
the existing test exclusions. At minimum, remove __tests__ from include if the
current exclusion strategy is intentional.
---
Nitpick comments:
In `@packages/validation/tsconfig.json`:
- Line 1: Resolve the contradictory TypeScript configuration in the validation
package’s tsconfig: choose whether tests should be compiled, then remove
"__tests__" from either the include or exclude array accordingly. Based on the
review’s proposed assumption, remove "__tests__" from include while keeping it
excluded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bd1fa0fd-4615-4fa8-9a0a-612f44b40e79
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
apps/api/package.jsonapps/web/package.jsonjest.config.jspackage.jsonpackages/accounting/package.jsonpackages/config-eslint/package.jsonpackages/config-typescript/base.jsonpackages/database/package.jsonpackages/hooks/package.jsonpackages/hooks/tsconfig.build.jsonpackages/shared/package.jsonpackages/shared/tsconfig.jsonpackages/sync/package.jsonpackages/types/package.jsonpackages/types/tsconfig.jsonpackages/utils/package.jsonpackages/utils/tsconfig.build.jsonpackages/validation/package.jsonpackages/validation/tsconfig.jsonscripts/__tests__/workspace-manifests.test.js
| @@ -1,2 +1,2 @@ | |||
| {"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist"]} | |||
| {"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist","__tests__","**/*.spec.ts","**/*.test.ts"]} | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Contradictory include and exclude for __tests__.
"__tests__" appears in both the include array (["src", "__tests__"]) and the exclude array. In TypeScript, exclude filters files matched by include, so __tests__ is effectively excluded — but listing it in include is contradictory and misleading.
Additionally, unlike packages/hooks which uses a separate tsconfig.build.json for build-time test exclusions, this is the base tsconfig.json. Excluding __tests__ and test files here means they will not be type-checked by the IDE or tsc --noEmit. If a separate tsconfig.build.json exists for the types package, these exclusions should live there instead. If not, test files lose type-checking coverage.
🔧 Proposed fix
If this is the only config (no separate build config), remove test exclusions to keep type checking, and create a tsconfig.build.json instead:
-{"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist","__tests__","**/*.spec.ts","**/*.test.ts"]}
+{"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist"]}Then create packages/types/tsconfig.build.json:
{
"extends": "./tsconfig.json",
"include": ["src"],
"exclude": ["node_modules", "dist", "__tests__", "**/*.spec.ts", "**/*.test.ts"]
}If the intent is to exclude test files from this single config, at minimum remove "__tests__" from include to eliminate the contradiction:
-"include":["src","__tests__"]
+"include":["src"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist","__tests__","**/*.spec.ts","**/*.test.ts"]} | |
| {"extends":"../config-typescript/base.json","compilerOptions":{"lib":["ES2022"],"outDir":"./dist","rootDir":"."},"include":["src","__tests__"],"exclude":["node_modules","dist"]} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/types/tsconfig.json` at line 1, Resolve the contradictory test
configuration in the package TypeScript configs. In tsconfig.json, preserve test
type-checking by removing __tests__ and test-file patterns from exclude; if
build-only exclusions are required, add a separate tsconfig.build.json extending
tsconfig.json with include set to src and the existing test exclusions. At
minimum, remove __tests__ from include if the current exclusion strategy is
intentional.
1 participant
Summary
Fixes the CI failures blocking the TypeScript 6.0.3 bump in #277 so the upgrade can land safely.
Root causes
jest.config.jsinline tsconfig usedignoreDeprecations: "5.0", which TypeScript 6.0 does not accept for the newbaseUrl/moduleResolution=node10deprecation warnings.packages/validation/tsconfig.jsonandpackages/types/tsconfig.jsonincluded__tests__and did not exclude*.spec.ts/*.test.ts, so the Docker buildtscstep failed with missing Jest global types.packages/hooks/tsconfig.build.jsononly excluded__tests__and was inconsistent with other package build configs.scripts/__tests__/workspace-manifests.test.jsstill asserted the old TypeScript 5.9.3 values.Changes
jest.config.js:ignoreDeprecations5.0→6.0packages/validation/tsconfig.json: exclude__tests__,**/*.spec.ts,**/*.test.tspackages/types/tsconfig.json: exclude__tests__,**/*.spec.ts,**/*.test.tspackages/hooks/tsconfig.build.json: exclude**/*.spec.ts,**/*.test.tsscripts/__tests__/workspace-manifests.test.js: update assertions to TypeScript 6.0.3 valuesVerification
pnpm test— 399/399 suites passed, 2416/2416 tests passedpnpm qa:commit— all quality gates passedpnpm type-check— all workspaces type-checked successfullydocker build -t smart-erp-next:ts6-check .— image built successfullySupersedes and closes #277.
Summary by CodeRabbit
Chores
Tests