-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Ensure factory symbol remains unused in react-jsx mode when not referenced in user code #41905
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
Conversation
…enced in user code
@@ -0,0 +1,16 @@ | |||
// @esModuleInterop: true | |||
// @noUnusedLocals: true | |||
// @jsx: react-jsx,react-jsxdev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯 TIL
@@ -26912,6 +26912,9 @@ namespace ts { | |||
errorOutputContainer); | |||
|
|||
function checkTagNameDoesNotExpectTooManyArguments(): boolean { | |||
if (getJsxNamespaceContainerForImplicitImport(node)) { | |||
return true; // factory is implicitly jsx/jsxdev - assume it fits the bill, since we don't strongly look for the jsx/jsxs/jsxDEV factory APIs anywhere else (at least not yet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, but it’s unintuitive as to why it fixes this particular bug. Something later in this function marked React
as referenced, for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - looking up it's type to see if it had call signatures.
Thanks for fixing this! |
So far, it’s slated for 4.2.0. When the rest of the team returns from vacation next week, we could discuss |
One of our checks was assuming the old-style jsx emit unconditionally and looking up the jsx factory entity name, which in turn would flag it as referenced.
Fixes #41882