build: upgrade to native typescript v7 🚀#8357
Conversation
typescript-eslint does not yet support native TS and expects `tsc` to be available, so we need to use an alias for now. We'll probably just migrate to oxlint soon anyway.
📝 WalkthroughWalkthroughThe PR aligns build and typecheck scripts with the native TypeScript compiler, updates TypeScript dependencies, and adjusts compiler configuration. Source changes refine runtime type narrowing, optional access, callback defaults, and value conversion. Unit tests remove redundant type assertions and add targeted lint suppressions for mocked methods. Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| env = {}, | ||
| options, | ||
| settings, | ||
| timeline = 'build', |
There was a problem hiding this comment.
Both callers pass a string literal here and the type is already marked as required
| "sourceMap": true | ||
| "rootDir": "src", | ||
| "sourceMap": true, | ||
| "tsBuildInfoFile": "./tsconfig.build.tsbuildinfo" |
There was a problem hiding this comment.
This isn't strictly related, but I noticed that we were publishing this to npm and it's 370 KB (~15% of the package). This fixes that by not writing it to dist/.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0cc5c3969
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "scripts": { | ||
| "_format": "prettier --loglevel=warn \"{src,tools,scripts,tests,e2e,.github}/**/*.{mjs,cjs,js,mts,md,yml,json,html,ts}\" \"*.{mjs,cjs,js,mts,yml,json,html,ts}\" \".*.{mjs,cjs,js,yml,json,html,ts}\" \"!CHANGELOG.md\" \"!**/*/package-lock.json\" \"!.github/**/*.md\"", | ||
| "build": "tsc --project tsconfig.build.json", | ||
| "build": "node ./node_modules/typescript-native/bin/tsc --project tsconfig.build.json", |
There was a problem hiding this comment.
When npm run build starts using the TS 7 compiler here, TS 6/7 defaults types to an empty list unless the tsconfig sets it explicitly; this repo's base config only sets typeRoots. In the GitHub workflows I checked, unit-tests, integration-tests, e2e-tests, and release jobs all run npm run build after npm ci, so the build program no longer loads @types/node and fails on the many node:* imports/NodeJS references before emitting dist. Add an explicit types entry (for example node plus any needed ambient packages, or *) before switching the build/typecheck scripts to TS 7.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/utils/env/index.ts (1)
195-205: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReflect the runtime default in the parameter type.
envelopeItemsdefaults to[], and the upstream API response is explicitly documented as potentially undefined, but the public parameter remains required. Make it optional (or explicitlyEnvelopeItem[] | undefined) and remove the suppression once the contract matches runtime behavior.🤖 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 `@src/utils/env/index.ts` around lines 195 - 205, Update the formatEnvelopeData parameter type so envelopeItems is optional or explicitly allows undefined, matching its default [] and upstream response contract. Then remove the eslint-disable suppression while preserving the existing defaulting and filtering behavior.
🤖 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 `@package.json`:
- Line 35: Update the package.json clean script to remove
tsconfig.build.tsbuildinfo in addition to the dist/ directory, ensuring both
generated outputs and the TypeScript incremental build cache are deleted.
In `@src/commands/api/api.ts`:
- Around line 14-15: Remove the implementation-describing comment immediately
above the dynamic method-name guard in the API command code, leaving the
existing guard and type definitions unchanged.
In `@src/commands/base-command.ts`:
- Around line 121-122: Update the workspace package-count check in the base
command to retain the nullability guard for workspace.packages before reading
length or indexing the first package. Preserve the existing behavior for
undefined packages while returning the sole package path when packages contains
exactly one entry.
---
Nitpick comments:
In `@src/utils/env/index.ts`:
- Around line 195-205: Update the formatEnvelopeData parameter type so
envelopeItems is optional or explicitly allows undefined, matching its default
[] and upstream response contract. Then remove the eslint-disable suppression
while preserving the existing defaulting and filtering behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ad53d8d-acf4-4458-9c53-c2d67aa7c1e7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.gitignorepackage.jsonsrc/commands/api/api.tssrc/commands/base-command.tssrc/commands/database/util/psql-formatter.tssrc/commands/env/env.tssrc/commands/logs/sources/edge-functions.tssrc/lib/functions/netlify-function.tssrc/utils/deploy/drop-api.tssrc/utils/deploy/upload-source-zip.tssrc/utils/env/index.tssrc/utils/run-build.tstests/unit/commands/database/db-status.test.tstests/unit/commands/login/login-check.test.tstests/unit/commands/login/login-request.test.tstests/unit/recipes/ai-context/download-context-files.test.tstests/unit/utils/live-tunnel.test.tstsconfig.base.jsontsconfig.build.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
💤 Files with no reviewable changes (1)
- tsconfig.base.json
| const isValidApiMethod = (api: NetlifyAPI, apiMethod: string): apiMethod is ApiMethodName => | ||
| Object.hasOwn(api, apiMethod) | ||
|
|
||
| // The method name comes from CLI args, so the value is read dynamically (as `unknown`); this guard |
There was a problem hiding this comment.
"single, honest call signature"? HUH? decomment-slop pls 🙏
| return value | ||
| } | ||
| return String(value as number | boolean | bigint) | ||
| // TODO(serhalp): Either narrow the type (only number/bigint/boolean reach here at runtime) or keep this. |
There was a problem hiding this comment.
do you have a linear issue created or is this just a for funzies todo
There was a problem hiding this comment.
I do not subscribe to the philosophy that a todo must have a tracking issue, and I am more than willing to have a debate on this topic on a public stage at a time and place of your choosing. I choose Nathan as my second.
| */ | ||
| export const formatEnvelopeData = ({ | ||
| context = 'dev', | ||
| // TODO(serhalp): Either fix `fetchEnvelopeItems` (it casts a possibly-undefined API response as `EnvelopeItem[]`) or |
There was a problem hiding this comment.
same q as above re: funzies
| }> | ||
| export async function runNetlifyBuild({ | ||
| command, | ||
| // TODO(serhalp): Either remove this default (the param is required and every known caller passes it) or keep it if a |
2 participants
Summary
The native Go rewrite of TypeScript was recently released as stable v7.
This upgrades this repo's build/typechecking tooling from v5 to v7, resulting in a significant speedup in build and typechecking time:
(Each is a median of 5 runs on an Apple M3 Pro, TS invoked directly)
typescript-eslint does not yet support native TS and expects the
tscCLI to be available, so we need to use an alias for now. We'll probably just migrate to oxlint soon anyway.