Skip to content
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

[Refactor] improve performance of component detection #3276

Merged
merged 1 commit into from May 16, 2022

Conversation

golopot
Copy link
Contributor

@golopot golopot commented Apr 23, 2022

Some node visitors for components detection are redundant.

Benchmark with time npx eslint . (mean of 5 runs):

- 5.06 s
+ 5.01 s

@codecov
Copy link

codecov bot commented Apr 23, 2022

Codecov Report

Merging #3276 (ea3b2a3) into master (ea3b2a3) will not change coverage.
The diff coverage is n/a.

❗ Current head ea3b2a3 differs from pull request most recent head 46ce117. Consider uploading reports for the commit 46ce117 to get more accurate results

@@           Coverage Diff           @@
##           master    #3276   +/-   ##
=======================================
  Coverage   97.72%   97.72%           
=======================================
  Files         123      123           
  Lines        8744     8744           
  Branches     3165     3165           
=======================================
  Hits         8545     8545           
  Misses        199      199           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea3b2a3...46ce117. Read the comment docs.

@ljharb
Copy link
Member

ljharb commented Apr 26, 2022

Can you elaborate on how they're redundant?

@golopot
Copy link
Contributor Author

golopot commented Apr 27, 2022

Our heuristics for detecting function components is checking if the function returns jsx or null (isReturningJSX), this function is expensive to execute. In a simple test case isReturningJSX is found to run 3 times for a simple function, ideally it should run only once.

This part is redundant

ReturnStatement(node) {
if (!utils.isReturningJSX(node)) {
return;
}
node = utils.getParentComponent();
if (!node) {
const scope = context.getScope();
components.add(scope.block, 1);
return;
}
components.add(node, 2);
},

, because checking isReturningJSX is included in getParentComponent -> getParentStatelessComponent -> getStatelessComponent -> isReturningJSX.

FunctionDeclaration(node) {
if (node.async) {
components.add(node, 0);
return;
}
node = utils.getParentComponent();
if (!node) {
return;
}
components.add(node, 1);
},

@ljharb
Copy link
Member

ljharb commented Apr 27, 2022

Would it be simpler to memoize isReturningJSX?

@golopot
Copy link
Contributor Author

golopot commented Apr 27, 2022

Would it be simpler to memoize isReturningJSX?

No, it wouldn't. Deleting redundant code is better than adding memoizing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants