Introduce more caching and deferral into jsx checking #25302
Merged
Conversation
This file contains 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
| @@ -16381,6 +16385,10 @@ namespace ts { | |||
| else { | |||
| checkExpression(node.closingElement.tagName); | |||
| } | |||
| } | |||
|
|
|||
| function checkJsxElement(node: JsxElement, _checkMode: CheckMode | undefined): Type { | |||
There was a problem hiding this comment.
Safe to remove _checkMode now?
There was a problem hiding this comment.
Yes, but I'll need to add it back in a week or two when I add more specific return types for jsx elements and once again track down all the places I need to flow it through again. I figured I'd just leave it here for later.
There was a problem hiding this comment.
Might deserve a comment in case Andy sees it and removes it 😉
RyanCavanaugh
approved these changes
Jun 28, 2018
sandersn
approved these changes
Jun 28, 2018
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Since, as of right now, the attributes and tag type of a jsx element do not play into the expression type, this defers checking of jsx elements. This deferral allows us to know that it is safe to cache many intermediate results while checking those tags (as we will not be in an inferential or contextual context), which we now do. We cache the result of
getJsxNamespaceAt, if possible (as that resolution took a measurable slice of time when done as often as it was, since we were doing it a few times on the same tag name), and thesourceAttributesType(where we were spending slightly more than half our jsx checking time when it was uncached, as calculating the attributes type was done both during overload resolution (for stateless components) and checking). In extreme cases like a project that was shown to us, I see an order of magnitude improvement in semantic diagnostic time - in one file down from 4s to 0.5s.This probably fixes #25085, since it seems to fix a similar issue in another project, though I can't know for sure.
For future work in this area,
resolveCustomJsxElementAttributesTypeis about the only jsx-specific thing left that takes a measurable slice of time (though not very much compared to what I just started caching). It already caches the signatures it uses internally; but it may be worth adding a second layer of caching to it in the future.