Lookup JSX namespace within factory function#22207
Merged
weswigham merged 2 commits intomicrosoft:masterfrom Feb 28, 2018
Merged
Lookup JSX namespace within factory function#22207weswigham merged 2 commits intomicrosoft:masterfrom
weswigham merged 2 commits intomicrosoft:masterfrom
Conversation
d812784 to
488397f
Compare
Contributor
|
@weswigham can you share perf numbers on one of our RWC jsx code bases comparing to before #21218, to #21218 + this change. |
Member
Author
src/compiler/checker.ts
Outdated
| return getUnionType(map(instantiatedSignatures, getReturnTypeOfSignature), UnionReduction.Subtype); | ||
| } | ||
|
|
||
| function getJsxNamespaceForLocation(location: Node) { |
Member
There was a problem hiding this comment.
Super nit, the naming of getJsxNamespaceForLocation should probably match getJsxStatelessElementTypeAt / getJsxElementClassTypeAt
src/compiler/checker.ts
Outdated
| */ | ||
| function getJsxIntrinsicTagNames(): Symbol[] { | ||
| const intrinsics = getJsxType(JsxNames.IntrinsicElements); | ||
| function getJsxIntrinsicTagNames(location: Node): Symbol[] { |
Member
There was a problem hiding this comment.
Similar, getJsxIntrinsicTagNamesAt
Member
|
Very nice |
RyanCavanaugh
approved these changes
Feb 28, 2018
mhegazy
reviewed
Feb 28, 2018
| getAliasedSymbol(symbol: Symbol): Symbol; | ||
| getExportsOfModule(moduleSymbol: Symbol): Symbol[]; | ||
| getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined; | ||
| getJsxIntrinsicTagNames(): Symbol[]; |
Contributor
There was a problem hiding this comment.
please add a note about this change in the breaking changes section in https://github.com/Microsoft/TypeScript/wiki/API-Breaking-Changes
mhegazy
approved these changes
Feb 28, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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
JSXnamespace 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)