Skip to content

Port implicit any type arguments in JavaScript #1242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Jun 19, 2025

This restores the behavior of filling in missing type arguments (and default type arguments set to {} and unknown) as any in JavaScript files.

It should fix the behavior in #1090, but and unbreaks JS code I've tested elsewhere where things like new Set result in a Set<unknown> instead of a Set<any>.

Some of this is already partially controlled by flags like noImplicitAny - however, I do wonder if we should consider adding another --strict flag for this behavior in JavaScript files. My intuition is that JS devs using checking and --strict do want the same behavior as TypeScript, and would opt in for tighter checking. The use of JS is circumstantial around their build needs/preferences, not a signal of whether they want looser checking.

Fixes #1090

@Copilot Copilot AI review requested due to automatic review settings June 19, 2025 21:15
Copy link
Contributor

@Copilot 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 restores JavaScript behavior by treating missing or default type arguments as any instead of unknown or {}, improving interoperability with existing JS code (e.g., new Set infers Set<any>). Key changes include:

  • Extending fillMissingTypeArguments with a JS file check to use any defaults in JS.
  • Updating numerous baseline tests to reflect any defaults in JS contexts.
  • Adjusting calls throughout internal/checker to pass the new isJavaScript flag.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/checker/relater.go Pass JS context flag to fillMissingTypeArguments in relation logic
internal/checker/inference.go Pass JS context flag to fillMissingTypeArguments in inference
internal/checker/jsx.go Update JSX instantiation calls to include JS context flag
internal/checker/checker.go Widespread updates to signature instantiation, default constructors, and type argument logic to use JS flag
testdata/baselines/reference/submoduleAccepted/... Baselines updated to expect any defaults and altered error counts
Comments suppressed due to low confidence (1)

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Jun 20, 2025

Minimal repro for the crash is

// @filename: foo.js
export class A<T> {}

export class B extends A {
    constructor() {
        super();
    }
}

I haven't fixed it in the latest commits, but I was in the neighborhood and found a few places I had missed (or might as well have cleared out some diffs)

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Based on the diffs, this sure seems correct to me. I'm not sure if this stuff was intentionally removed or not, though, but I feel like this should be here. Maybe @sandersn has an opinion, however.

(Also, baselines need a reup.)

+
+
+out/file.d.ts(4,23): error TS2314: Generic type 'Array<T>' requires 1 type argument(s).
Copy link
Member

Choose a reason for hiding this comment

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

Hm, this is exciting; wonder how easy it'd be to fix this?

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.

Using a class based react component imported from a jsx files produce errors
2 participants