Skip to content

Conversation

@sapphi-red
Copy link
Contributor

@types/node uses typeof globalThis extends { onmessage: any } ? {} : import("undici-types").RequestInit; for the type of RequestInit (this is to avoid the type conflict between RequestInit in dom and undici).
So if lib: 'dom' is included in tsconfig.json, an error is output by TypeScript for RequestInit["dispatcher"].
https://github.com/vitejs/vite/actions/runs/19419498249/job/55554030486#step:9:22

This PR fixes that error by using any for dispatcher when lib: 'dom' is used.

@Corvince
Copy link

Thanks, I got this error while writing some of the tests and wasn't sure where it originated from. But we should rework the dispatcher option, or more precisely remove it in favor of the customFetch option which isn't implemented yet. Dispatcher is a undici/nodejs specific fetch option and not part of the fetch standard. The idea of the fetch path (in contrast to the earlier undici path) was to be somewhat runtime-independent, so dispatcher should be handled either as a custom fetch or just as a regular RequestOption (which, for nodejs, includes dispatcher)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a TypeScript compilation error that occurs when lib: 'dom' is included in tsconfig.json. The issue stems from @types/node using a conditional type for RequestInit that excludes the dispatcher property when DOM types are present (to avoid conflicts between DOM's RequestInit and undici's version).

Key Changes:

  • Added a conditional type check for the Dispatcher type that returns any when DOM types are detected (via globalThis.onmessage check)
  • Added explanatory comments documenting why this workaround is necessary

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// as dispatcher property does not exist in RequestInit in that case
export type Dispatcher = (typeof globalThis extends { onmessage: any }
? any
: RequestInit)["dispatcher"];
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

The conditional type logic has a syntax error. When accessing the dispatcher property on line 108, it's being accessed on the result of the conditional type (typeof globalThis extends { onmessage: any } ? any : RequestInit), but the parenthesis closes before ["dispatcher"] is applied.

This means you're trying to do: (ConditionalType)["dispatcher"] which attempts to access the property on the conditional type result itself. Instead, it should be:

export type Dispatcher = (typeof globalThis extends { onmessage: any }
  ? any
  : RequestInit["dispatcher"]);

Move the ["dispatcher"] access inside the false branch so it only accesses the property when RequestInit is selected.

Suggested change
: RequestInit)["dispatcher"];
: RequestInit["dispatcher"]);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

(seems like nonsense by ai)

@williamstein williamstein merged commit 2e0923f into sagemathinc:main Nov 17, 2025
9 checks passed
@sapphi-red sapphi-red deleted the fix/use-any-for-dispatcher-when-lib-dom-is-used branch November 18, 2025 09:11
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.

3 participants