Skip to content

[test] Enable type-unaware versions of disabled typed-aware lint rules - #24873

Merged
eps1lon merged 1 commit into
mui:nextfrom
eps1lon:test/enable-type-unaware-rules
Feb 12, 2021
Merged

[test] Enable type-unaware versions of disabled typed-aware lint rules#24873
eps1lon merged 1 commit into
mui:nextfrom
eps1lon:test/enable-type-unaware-rules

Conversation

@eps1lon

@eps1lon eps1lon commented Feb 12, 2021

Copy link
Copy Markdown
Member

Summary

Re-enable the following lint rules:

  • dot-notation
  • no-implied-eval
  • no-throw-literal

Context

Noticed during #24810 that we have disabled some type-aware lint rules for "performance considerations". I suspect this actually refers to the required type checking not because the rules themselves are slow.

If they were in fact only disabled to avoid type-ware linting, we should enable their type-unaware counterparts. The typescript-eslint repo says these versions "don't work with typescript" but it's unclear what they mean: do these versions have false-negatives or false-positives1? False positives would be fine since we don't expect linting to catch every error anyway. false-negatives would be more problematic but I haven't seen false-negatives yet.

1
false-negative: error reported that isn't one
false-positive: no error reported even though it is one

@eps1lon eps1lon added the test label Feb 12, 2021
@mui-pr-bot

mui-pr-bot commented Feb 12, 2021

Copy link
Copy Markdown

No bundle size changes

Generated by 🚫 dangerJS against 11874f0

@eps1lon
eps1lon marked this pull request as ready for review February 12, 2021 11:28

@oliviertassinari oliviertassinari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agree with all you said.

TIMING=1 yarn lint returns in this PR and in my env:

Rule Time (ms) Relative
import/no-cycle 43641.930 29.9%
mocha/no-nested-tests 10677.706 7.3%
mocha/no-identical-title 10191.786 7.0%
mocha/no-exclusive-tests 7090.581 4.9%
mocha/no-skipped-tests 6822.207 4.7%
import/extensions 3665.945 2.5%
mocha/no-pending-tests 3598.851 2.5%
mocha/no-async-describe 3582.821 2.5%
mocha/no-global-tests 3578.034 2.5%
mocha/no-top-level-hooks 3531.212 2.4%

On a related note, I wonder why eslint runs in 3mins now, when it was running in /2 the time in #21758 6 months ago. Maybe it's pickers related.

@eps1lon
eps1lon merged commit af457bf into mui:next Feb 12, 2021
@eps1lon
eps1lon deleted the test/enable-type-unaware-rules branch February 12, 2021 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants