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.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR aligns build and typecheck scripts with the native TypeScript compiler, updates TypeScript dependencies, and adjusts compiler configuration. Source changes refine API validation, site state handling, 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 Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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".
![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
| 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.
There was a problem hiding this comment.
(and I don't want to risk behavioural changes in this PR)
| */ | ||
| 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad2fc78180
ℹ️ 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".
| "sourceMap": true | ||
| "rootDir": "src", | ||
| "sourceMap": true, | ||
| "tsBuildInfoFile": "./tsconfig.build.tsbuildinfo" |
There was a problem hiding this comment.
Because this moves the incremental build-info file outside dist/, the existing npm run clean script—which only removes dist/—now leaves the cache intact. After a successful build, running npm run clean && npm run build causes the compiler to regard unchanged inputs as already emitted, so it can recreate no dist files and leave the CLI unbuildable. Keep the cache under dist or delete tsconfig.build.tsbuildinfo in the clean script.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/commands/api/api.ts (2)
41-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the malformed invalid-method error.
The message currently renders two closing quotes, for example
"getSite"" is not a valid api method.Proposed fix
- `"${apiMethodName}"" is not a valid api method. Run "netlify api --list" to see available methods`, + `"${apiMethodName}" is not a valid api method. Run "netlify api --list" to see available methods`,🤖 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/commands/api/api.ts` around lines 41 - 43, The invalid-method error message in the api method validation path has an extra closing quote after apiMethodName. Update the logAndThrowError message to render exactly one pair of quotes around apiMethodName while preserving the existing text and behavior.
52-58: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not send raw
--datapreviews to telemetry.The preview is interpolated into the error passed to
logAndThrowError, which reports that error throughreportError. Malformed input can therefore expose secrets or PII. Redact or remove the preview, and update the corresponding expectations intests/unit/commands/api/api.test.tsLines 35-48.🤖 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/commands/api/api.ts` around lines 52 - 58, Remove or redact the user-supplied preview from the invalid JSON error constructed in the API command before it reaches logAndThrowError and reportError. Keep the validation guidance intact without exposing raw --data content, and update the corresponding expectations in the API command unit tests.
🤖 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 `@src/commands/api/api.ts`:
- Around line 47-65: Update the --data handling in the API command to
distinguish an omitted value from an empty string, so explicitly provided empty
input is rejected rather than replaced with {}. After JSON.parse in the
options.data branch, validate that payload is a non-null object and not an
array; route invalid strings and parsed non-object values through
logAndThrowError, and invoke the API only with a valid object or the existing
default for omitted data.
---
Outside diff comments:
In `@src/commands/api/api.ts`:
- Around line 41-43: The invalid-method error message in the api method
validation path has an extra closing quote after apiMethodName. Update the
logAndThrowError message to render exactly one pair of quotes around
apiMethodName while preserving the existing text and behavior.
- Around line 52-58: Remove or redact the user-supplied preview from the invalid
JSON error constructed in the API command before it reaches logAndThrowError and
reportError. Keep the validation guidance intact without exposing raw --data
content, and update the corresponding expectations in the API command unit
tests.
🪄 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: b70b4150-217f-4ee5-ad9e-dddb7f5d97ba
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonsrc/commands/api/api.tssrc/commands/base-command.tssrc/commands/database/util/psql-formatter.tssrc/commands/env/env.tssrc/utils/env/index.tssrc/utils/run-build.tstests/unit/commands/api/api.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/env/index.ts
- src/commands/database/util/psql-formatter.ts
| @@ -59,7 +64,7 @@ Note: key=value pairs are not accepted; use JSON syntax instead.`, | |||
| payload = {} | |||
| } | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject empty and non-object --data values.
if (options.data) treats --data "" as absent and sends {}. JSON.parse also accepts null, arrays, and primitives, although the error explicitly says this flag expects an object. Validate presence explicitly and require a non-null, non-array object before invoking the API.
🤖 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/commands/api/api.ts` around lines 47 - 65, Update the --data handling in
the API command to distinguish an omitted value from an empty string, so
explicitly provided empty input is rejected rather than replaced with {}. After
JSON.parse in the options.data branch, validate that payload is a non-null
object and not an array; route invalid strings and parsed non-object values
through logAndThrowError, and invoke the API only with a valid object or the
existing default for omitted data.
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.