-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Lookup JSX namespace within factory function #22207
Conversation
d812784
to
488397f
Compare
@weswigham can you share perf numbers on one of our RWC jsx code bases comparing to before #21218, to #21218 + this change. |
src/compiler/checker.ts
Outdated
@@ -15358,16 +15321,27 @@ namespace ts { | |||
return getUnionType(map(instantiatedSignatures, getReturnTypeOfSignature), UnionReduction.Subtype); | |||
} | |||
|
|||
function getJsxNamespaceForLocation(location: Node) { |
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.
Super nit, the naming of getJsxNamespaceForLocation should probably match getJsxStatelessElementTypeAt / getJsxElementClassTypeAt
src/compiler/checker.ts
Outdated
} | ||
|
||
/** | ||
* Returns all the properties of the Jsx.IntrinsicElements interface | ||
*/ | ||
function getJsxIntrinsicTagNames(): Symbol[] { | ||
const intrinsics = getJsxType(JsxNames.IntrinsicElements); | ||
function getJsxIntrinsicTagNames(location: Node): Symbol[] { |
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.
Similar, getJsxIntrinsicTagNamesAt
Very nice |
@@ -1811,7 +1811,7 @@ declare namespace ts { | |||
getAliasedSymbol(symbol: Symbol): Symbol; | |||
getExportsOfModule(moduleSymbol: Symbol): Symbol[]; | |||
getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined; | |||
getJsxIntrinsicTagNames(): Symbol[]; |
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.
please add a note about this change in the breaking changes section in https://github.com/Microsoft/TypeScript/wiki/API-Breaking-Changes
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.
Done. 👍
This allows you to mix the types for multiple jsx factories in one compilation in a similar way to how you can now mix factory functions. What this change does is look for an exported
JSX
namespace under the "react namespace" of the factory function. What this means is that if the factory function isReact.createElement
, we'll look forReact.JSX
. If the factory function isp
, we'll look forp.JSX
. If a local JSX namespace cannot be found, we still fallback to the global one, if present. This does not change how we discover any JSX types beyond looking for them in an additional location.An an example, react's core exports can now be defined like so:
and you get everything you need on the consuming end with your standard
With this change, a JSX package no longer needs to pollute the global scope with types that can conflict with other packages. 😄
Fixes #18131
IMHO, this elegantly solves the aliasing problem posed in the original issue (we're just doing the lookup inside the factory function's namespace, rather than trying to trace back into the file it came from and looking beside it)