Cache detected React version #2673
Merged
Conversation
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
ljharb
force-pushed
the
cache-react-version
branch
from
93fac86 to
c9dfc2d
Compare
lencioni
force-pushed
the
cache-react-version
branch
from
c9dfc2d to
b3f7a85
Compare
I've been timing and profiling ESLint runs at Airbnb and noticed that react/no-unknown-property is particularly slow for us when using the "detect" setting for React version. When running ESLint using TIMING=1 on a directory that contains about 500 files to be linted in it, react/no-unknown-property shows up as taking about 1700ms. Looking at the callstacks in the profiler, it seems that when this rule calls getStandardName for every JSXAttribute here, it will actually do all of the work to detect the React version every time to determine the correct set of DOM property names. Since there may be a lot of JSXAttribute nodes in a codebase, this adds up to quite a bit of work. By specifying the react version in our ESLint config, the no-unkown-property rule drops out of the top 10 slowest rules entirely, with the 10th slowest clocking in at 180ms. I think it would be a good idea to cache the React version when using detect instead of looking it up every time.
lencioni
force-pushed
the
cache-react-version
branch
from
b3f7a85 to
b8e91a5
Compare
|
thanks! going back to the original #1978 it looks like the only thing that stopped me for caching this was testing 😄 |
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.
I've been timing and profiling ESLint runs at Airbnb and noticed that
react/no-unknown-property is particularly slow for us when using the
"detect" setting for React version.
When running ESLint using TIMING=1 on a directory that contains about
500 files to be linted in it, react/no-unknown-property shows up as
taking about 1700ms.
Looking at the callstacks in the profiler, it seems that when this rule
calls getStandardName for every JSXAttribute here, it will actually do
all of the work to detect the React version every time to determine the
correct set of DOM property names. Since there may be a lot of
JSXAttribute nodes in a codebase, this adds up to quite a bit of work.
By specifying the react version in our ESLint config, the
no-unkown-property rule drops out of the top 10 slowest rules entirely,
with the 10th slowest clocking in at 180ms.
I think it would be a good idea to cache the React version when using
detect instead of looking it up every time.
Fixes #2671