Skip to content

update typescript and other npm deps to latest#6389

Merged
anonrig merged 1 commit intomainfrom
yagiz/update-typescript-6
Mar 25, 2026
Merged

update typescript and other npm deps to latest#6389
anonrig merged 1 commit intomainfrom
yagiz/update-typescript-6

Conversation

@anonrig
Copy link
Copy Markdown
Member

@anonrig anonrig commented Mar 23, 2026

Updates a lot of dependencies (including Typescript 6.0.2)

@anonrig anonrig requested review from a team as code owners March 23, 2026 17:58
@anonrig anonrig requested review from fhanau, jasnell and npaun March 23, 2026 17:59
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR updates TypeScript to 6.0.2 and bumps several other npm dependencies, with corresponding type fixes for the stricter Uint8Array<ArrayBuffer> generics and a workaround for the Scheduler name collision.

Issues (ranked by severity):

  1. [MEDIUM] src/pyodide/internal/pool/builtin_wrappers.ts:113-114 — Type signature not updated to match the changed lib.ts export. The wrapper still accepts/returns bare Uint8Array (i.e., Uint8Array<ArrayBufferLike>) while the inner function (getRandomValuesType from lib.ts) was narrowed to Uint8Array<ArrayBuffer>. Under TS 6.0's stricter typed-array generics, passing Uint8Array<ArrayBufferLike> to a function expecting Uint8Array<ArrayBuffer> is a type error. This doesn't break the build because pyodide's tsconfig has noEmitOnError: false, but it introduces a latent type inconsistency.

    Suggested fix — update the wrapper to match:

    export function getRandomValues(Module: Module, arr: Uint8Array<ArrayBuffer>): Uint8Array<ArrayBuffer> {
      return getRandomValuesInner(Module, arr);
    }
  2. [LOW] internal_timers_promises.ts — The unknown double-cast workaround for the Scheduler name collision works but is fragile. The TODO comment is appreciated; just noting the tech debt.


This review was generated by an AI assistant and may contain inaccuracies.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 23, 2026

Review posted successfully on PR #6389. Here's a summary of my findings:

1 actionable issue found:

  • [MEDIUM] src/pyodide/internal/pool/builtin_wrappers.ts:113-114 — The getRandomValues wrapper function wasn't updated to match the narrowed Uint8Array<ArrayBuffer> signature in lib.ts. This is a latent type inconsistency that's only masked by pyodide's noEmitOnError: false.

  • [LOW] The Scheduler name collision workaround in internal_timers_promises.ts using unknown cast is fragile but adequately documented with a TODO.

github run

@anonrig anonrig enabled auto-merge (squash) March 23, 2026 18:03
Copy link
Copy Markdown
Contributor

@fhanau fhanau left a comment

Choose a reason for hiding this comment

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

Changes LGTM – it looks like rules_ts has already been updated to include TS6.0 (see aspect-build/rules_ts@6c02feb), so if we're willing to wait for the next rules_ts release manually adding ts_integrity could be avoided

@anonrig anonrig force-pushed the yagiz/update-typescript-6 branch from c23cd14 to ce3496a Compare March 25, 2026 16:26
@anonrig
Copy link
Copy Markdown
Member Author

anonrig commented Mar 25, 2026

Changes LGTM – it looks like rules_ts has already been updated to include TS6.0 (see aspect-build/rules_ts@6c02feb), so if we're willing to wait for the next rules_ts release manually adding ts_integrity could be avoided

It might not be released for couple of weeks. We can always change this line.

@anonrig anonrig disabled auto-merge March 25, 2026 19:27
@anonrig anonrig merged commit 5e2a624 into main Mar 25, 2026
22 of 24 checks passed
@anonrig anonrig deleted the yagiz/update-typescript-6 branch March 25, 2026 19:28
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.

2 participants