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

Improve performance by skipping ParserService generation if unnecessary #201

Conversation

tarao1006
Copy link
Contributor

@tarao1006 tarao1006 commented Feb 13, 2024

Thank you for nice rule!

I would like to make adjustments to the implementation related to 'getParserServices.' If 'applyToAllComponents' is set to true in 'props-order' or 'props-shorthand,' the parserService becomes unnecessary.

Using parserService in eslint may result in longer linting completion times compared to not using it. In my case, adopting the changes in this PR reduced a task that took 12 seconds to now complete in 8 seconds.

Plus, the parserOptions in .eslintrc will be able to be removed.

So, If 'applyToAllComponents' is set to true, it will be better to skip getParserServices call. If you have the time, I would appreciate it if you could review this PR.

@tarao1006 tarao1006 force-pushed the feature/avoid-from-getting-unnecessary-services branch from 45556c3 to 549b6d0 Compare February 13, 2024 14:37
Copy link
Owner

@yukukotani yukukotani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Almost looks good, commenting just one stylistic thing


return {
JSXOpeningElement(node) {
if (!option?.applyToAllComponents && !isChakraElement(node, parserServices)) {
if (parserServices && !isChakraElement(node, parserServices)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer !option?.applyToAllComponents to remain here for intuitiveness

Suggested change
if (parserServices && !isChakraElement(node, parserServices)) {
if (!option?.applyToAllComponents && !isChakraElement(node, getParserServices(ctx, false))) {

Copy link
Contributor Author

@tarao1006 tarao1006 Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your prompt review and I agree with your proposal!

Actually, I had concerns about the cost of calling getParserServices, but I found that it is very simple function.

@yukukotani yukukotani changed the title Avoid from getting unnecessary services Avoid from getting unnecessary services for better performance Feb 13, 2024
@yukukotani yukukotani changed the title Avoid from getting unnecessary services for better performance Improve performance by skipping ParserService generation if unnecessary Feb 13, 2024
@yukukotani yukukotani merged commit d82847f into yukukotani:main Feb 13, 2024
11 checks passed
@yukukotani
Copy link
Owner

Landed in v0.11.0. Thanks for the contribution!

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

Successfully merging this pull request may close these issues.

2 participants