Skip to content

TypeScript ignores type of JSX spread props if the JSX tag is non-empty #44782

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

Closed
bluenote10 opened this issue Jun 28, 2021 · 6 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@bluenote10
Copy link

Bug Report

🔎 Search Terms

jsx empty

Possibly related issues:

Related SO question: Why does TypeScript ignore the type of JSX spread props if the JSX tag is not empty?

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about JSX.

⏯ Playground Link

Playground link with relevant code

💻 Code

This assumes JSX: React

const badProps = {
  badProp: "foo",
};

// This doesn't compile; as expected.
function Test1() {
  return <div {...badProps} />;
}

// This does compile; not expected.
function Test2() {
  return <div {...badProps}> </div>;
}

🙁 Actual behavior

The expression in Test2 compiles without error.

🙂 Expected behavior

Type checks should apply to both empty and non-empty usages, and result in the same compilation error.

@andrewbranch
Copy link
Member

Duplicate of #29883

The one that has an error has an error for a separate reason; the real behavior you’re talking about is the fact that excess properties are generally allowed on JSX elements if they come from spread. The former example just happens to error because it has no good properties (whereas the latter has a matching children property), so it triggers our separate “this really looks like you have no idea what function/element you’re calling/creating” logic.

@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Jun 28, 2021
@bluenote10
Copy link
Author

Indeed #29883 would solve it. On the other hand #29883 is a feature request, while this particular behavior feels more like a bug.

@andrewbranch
Copy link
Member

#29883 is framed in an odd way, but just a few comments down it’s argued and broadly 👍’d that it feels like a bug. Either way, this is behavior that we do know about and at one point did on purpose I guess, so changing it is “suggestion,” not a “bug fix” 🤷

@bluenote10
Copy link
Author

The bug aspect could be fixed independently by allowing excess properties in general. Technically this can't break any existing code. From a user perspective it would be clearer to allow excess properties entirely or not at all. That the language treats <div {...badProps}> </div> differently from <div {...badProps}></div> is highly unexpected.

Of course the cleaner / desired solution is to disallow them (via an option).

@andrewbranch
Copy link
Member

That’s not an excess-property thing; that’s a common-property thing, and the example you found just looks extra strange because of the way JSX obscures what’s going on under the hood. You can suppress the error not only by making the element non-empty, but by adding any other recognized prop:

<div {...badProps}></div>                       // error: no properties in common
<div {...badProps} className="whatever"> </div> // no error, because source and target have overlap with `children`
<div {...badProps} className="whatever"></div>  // no error, because source and target have overlap with `className`

The “no properties in common logic” exists to catch likely mistakes where there would otherwise be no error: playground. There’s no type-theoretical reason to treat these two objects differently, since excess properties are allowed and all the target properties are optional. But when there’s zero overlap, it looks quite likely to be a mistake, so we flag it.

So yes, you can often find strange examples that look highly inconsistent with each other when you put them side-by-side, but in this case that inconsistency is an unrelated feature, not a bug.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants