Skip to content
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

Declaration emit inlines types incorrectly #57779

Open
eps1lon opened this issue Mar 14, 2024 · 2 comments
Open

Declaration emit inlines types incorrectly #57779

eps1lon opened this issue Mar 14, 2024 · 2 comments
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Milestone

Comments

@eps1lon
Copy link
Contributor

eps1lon commented Mar 14, 2024

πŸ”Ž Search Terms

declaration emit inline

πŸ•— Version & Regression Information

I was unable to test this prior to 4.4 since React types are no longer compatible with that version.

⏯ Playground Link

No response

πŸ’» Code

import * as React from "react";

export class Component extends React.Component<{ children: React.ReactNode }> {
  render() {
    if (Math.random()) {
      return <div>{this.props.children}</div>;
    } else {
      return this.props.children;
    }
  }
}

Full repro: https://github.com/eps1lon/ts-module-auto-export/tree/bad-inline

πŸ™ Actual behavior

Return value from render() is not referencing React.ReactNode but inlines its (incomplete) union members:

import * as React from "react";
export declare class Component extends React.Component<{
    children: React.ReactNode;
}> {
    render(): string | number | boolean | Iterable<React.ReactNode> | import("react/jsx-runtime").JSX.Element;
}

πŸ™‚ Expected behavior

Return type references the type alias. Inlining union members is incorrect (we may add or remove members or treat members as internal, local types) generally and here specially not even using all members.

export declare class Component extends React.Component<{
  children: React.ReactNode;
}> {
  render(): React.ReactNode | import("react/jsx-runtime").JSX.Element;
}

Additional information about the issue

The fact that it inlines the type here is a special case of #37151. #37151 is a more general since it's not always clear when inlining is correct or not. I'd be curious to know what would break if TS would not stop inlining in this special case.

But this issue is also broader since the inline is incomplete.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases labels Mar 15, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 15, 2024
@RyanCavanaugh
Copy link
Member

What's incomplete about this? I don't see any missing things in the union

@eps1lon
Copy link
Contributor Author

eps1lon commented Mar 15, 2024

-string | number | boolean | Iterable<React.ReactNode>
+string | number | boolean | Iterable<React.ReactNode> | React.ReactElement | null | undefined | React.ReactPortal

React.ReactElement is not the same as React.JSX.Element since they're different interfaces.

But that's not the focus here. The problem is that TypeScript inlines in the first place here. Especially because the same version of the published types can have different versions of ReactNode due to module augmentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Projects
None yet
Development

No branches or pull requests

2 participants