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

ESLint: Enabled no-dupe-disjunctions rule #2951

Merged

Conversation

RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Jun 15, 2021

This enables the regexp/no-dupe-disjunctions rule. This is the performance-heavy rule I talked about in #2924.

However, the rule is slow. Right now, it increases the total linting time 5x (npm run lint), from 5s to 25s on my machine. This doesn't seem to be a problem for live code editing/viewing.

The only commit of this PR will be just enabling the rule. This will allow us to see the problems the rule finds. I will add the fixes for these reports in another PR. This allows merging the fixes even if we should decide against this rule.


@LeaVerou @Golmote @mAAdhaTTah

@github-actions
Copy link

github-actions bot commented Jun 15, 2021

No JS Changes

Generated by 🚫 dangerJS against eec2d97

@LeaVerou
Copy link
Member

I don't understand, if it makes linting this slow, how is it not a problem for live linting? Do editors not run it?

@RunDevelopment
Copy link
Member Author

Editors do run it. It's not a problem for editors because they only run ESLint on open files. The rule is slow as in: "it takes about 10ms per regex". This time is noticeable when linting all of our 2500 regexes but not when it's only the 10~20 regexes per language definition.

@LeaVerou
Copy link
Member

Ah I see. Thanks for the explanation! I'm fine with this.

@RunDevelopment
Copy link
Member Author

In that case, I'll merge the fixes and then this rule.

@RunDevelopment RunDevelopment merged commit 42fabfe into PrismJS:master Jun 17, 2021
@RunDevelopment RunDevelopment deleted the eslint-no-dupe-disjunctions branch June 17, 2021 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants