Skip to content

[jsdom] Remove DOMWindow Infinity/NaN properties that conflict with numeric index signature#74969

Open
tomquist wants to merge 3 commits into
DefinitelyTyped:masterfrom
tomquist:fix/jsdom-remove-infinity-nan
Open

[jsdom] Remove DOMWindow Infinity/NaN properties that conflict with numeric index signature#74969
tomquist wants to merge 3 commits into
DefinitelyTyped:masterfrom
tomquist:fix/jsdom-remove-infinity-nan

Conversation

@tomquist
Copy link
Copy Markdown

@tomquist tomquist commented May 8, 2026

Remove readonly ["Infinity"]: number and readonly ["NaN"]: number from DOMWindow.

These numeric-string literal property names conflict with the [index: number]: Window numeric index signature inherited from Window (via lib.dom.d.ts), producing TS2411:

Property '["Infinity"]' of type 'number' is not assignable to 'number' index type 'Window'.
Property '["NaN"]' of type 'number' is not assignable to 'number' index type 'Window'.

This has been reported since TypeScript 4.4 (#57467, #57466) and affects anyone using skipLibCheck: false. TypeScript 7 (tsgo / Corsa) now reports this error consistently — the previous tsc behavior of silently suppressing it was confirmed as a bug by @ahejlsberg, and the tsgo team closed microsoft/typescript-go#3490 as wontfix.

Removing the properties is safe: DOMWindow already declares [key: string]: any, so window["Infinity"] and window["NaN"] still resolve to any through the string index signature. No type information is lost.

Fixes #57467.

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented May 8, 2026

@tomquist Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment that I will keep updated.

1 package in this PR

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • 🕐 Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 74969,
  "author": "tomquist",
  "headCommitOid": "cb1f46689061e73b62aa694df0b1d5c798a2e9a1",
  "mergeBaseOid": "1dfa9e254d666bcd50572ec7d110882a881c3861",
  "lastPushDate": "2026-05-08T14:51:29.000Z",
  "lastActivityDate": "2026-05-11T21:02:47.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "tooManyCommits": false,
  "tooManyReviews": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "jsdom",
      "kind": "edit",
      "files": [
        {
          "path": "types/jsdom/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/jsdom/jsdom-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "leonard-thieu",
        "palmfjord",
        "ExE-Boss"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "ExE-Boss",
      "date": "2026-05-11T21:02:47.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 4407383554,
  "ciResult": "pass"
}

@typescript-bot
Copy link
Copy Markdown
Contributor

🔔 @leonard-thieu @palmfjord @ExE-Boss — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot removed the Untested Change This PR does not touch tests label May 8, 2026
@ExE-Boss
Copy link
Copy Markdown
Contributor

ExE-Boss commented May 8, 2026

The correct fix would be to change the signature in lib.dom.d.ts to exclude Infinity and NaN from the [index: number], but that’d likely depend on microsoft/TypeScript#4196 and microsoft/TypeScript#28682.

Comment thread types/jsdom/index.d.ts Outdated
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board May 11, 2026
@typescript-bot
Copy link
Copy Markdown
Contributor

@ExE-Boss Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

Copy link
Copy Markdown
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

LGTM conditional on updating the title of this PR

@typescript-bot typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Critical package Owner Approved A listed owner of this package signed off on the pull request.

Projects

Status: Needs Maintainer Review

Development

Successfully merging this pull request may close these issues.

TS2411 reported on properties declared in node_modules .d.ts files (e.g. @types/jsdom DOMWindow), where tsc 6.0 silently allows them

3 participants