Skip to content

ci: add typecheck step - #1030

Open
araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:ci-typecheck
Open

ci: add typecheck step#1030
araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:ci-typecheck

Conversation

@araujogui

Copy link
Copy Markdown
Member

No CI job ran tsc, so type errors could land on main. One had: erasableSyntaxOnly (#252) retroactively made a pre-existing angle-bracket cast in src/utils/object.ts a TS1294 error.

Adds npm run typecheck (tsc --noEmit over both projects) and wires it into the Tests workflow. Also fixes the errors it surfaced:

  • src/utils/object.ts — angle-bracket cast to as.
  • e2e-tests/tsconfig.json — its types replaced the inherited worker-configuration.d.ts instead of extending it, dropping every Workers global.
  • src/worker.tswithSentry<Env> passed one explicit type arg, so the handler type param fell back to its default ExportedHandler<Env> with an optional fetch. Passing typeof handler fixes the 20 worker.fetch is possibly undefined errors without touching the tests.
  • Dropped CACHING: false from the e2e tests; nothing reads it.

Nothing in CI ran tsc, so `erasableSyntaxOnly` (added in nodejs#252) left an
unnoticed TS1294 on a pre-existing angle-bracket cast.

Fixes the existing errors and gates them in CI.
@araujogui
araujogui requested a review from a team as a code owner September 12, 2026 15:11
Copilot AI lite review requested due to automatic review settings September 12, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@MattIPv4
MattIPv4 requested a review from flakey5 September 12, 2026 15:31
@ovflowd

ovflowd commented Sep 12, 2026

Copy link
Copy Markdown
Member

I'm fine fast-tracking this as it improves resilience/CI. @nodejs/web-infra 👍 / 👎 for fast-tracking

@MattIPv4

Copy link
Copy Markdown
Member

👎 I don't see the need to fast-track this, it can wait, I'd like @flakey5 to review it if they have the time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants