eslint-plugin-monorepo icon indicating copy to clipboard operation
eslint-plugin-monorepo copied to clipboard

Speed up no-relative-import rule

Open snowystinger opened this issue 4 years ago • 3 comments

First, thank you for these rules, they've caught many issues before they became issues. I work on a very large monorepo https://github.com/adobe/react-spectrum and our eslint has slowed down substantially. Eslint performance today:

Rule                                        | Time (ms) | Relative
:-------------------------------------------|----------:|--------:
monorepo/no-relative-import                 |  6535.571 |    48.0%
no-redeclare                                |   614.720 |     4.5%
rulesdir/imports                            |   547.905 |     4.0%
@typescript-eslint/no-unused-vars           |   436.368 |     3.2%
jsdoc/require-description-complete-sentence |   435.446 |     3.2%
indent-legacy                               |   286.457 |     2.1%
react/jsx-indent                            |   251.766 |     1.8%
jsdoc/check-alignment                       |   249.314 |     1.8%
jsdoc/check-indentation                     |   224.696 |     1.6%
jsdoc/check-tag-names                       |   221.656 |     1.6%

I put together this PR in order to improve our overall runtime. This does two things to speed up performance of this rule:

  1. minimatch is a glob expressions conversion into JavaScript RegExp, this is useful for complex things, however, the way it's used here is minimatch(filePath, path.join(pkg.location, '**')) this would lead to a glob of ${pkg.location}/** and we want to know if filePath matches that glob this is the same as isInside, which is faster overall though the time saved isn't that big, but ~7% of a several minute run is nothing to be sad about

  2. this rule only cares about relative imports, I may be wrong about this, but I believe all relative imports will have '../' in them, therefore we can shortcut the entire rule if that isn't in the import path this is where the big time savings came from

After minimatch replacement:

Rule                                        | Time (ms) | Relative
:-------------------------------------------|----------:|--------:
monorepo/no-relative-import                 |  5251.391 |    41.3%
no-redeclare                                |   638.222 |     5.0%
rulesdir/imports                            |   518.794 |     4.1%
jsdoc/require-description-complete-sentence |   460.373 |     3.6%
@typescript-eslint/no-unused-vars           |   444.902 |     3.5%
indent-legacy                               |   317.514 |     2.5%
react/jsx-indent                            |   264.220 |     2.1%
jsdoc/check-tag-names                       |   257.819 |     2.0%
jsdoc/check-alignment                       |   257.492 |     2.0%
jsdoc/check-indentation                     |   237.808 |     1.9%

After '../' shortcut:

Rule                                        | Time (ms) | Relative
:-------------------------------------------|----------:|--------:
no-redeclare                                |   698.225 |     9.2%
monorepo/no-relative-import                 |   480.147 |     6.3%
rulesdir/imports                            |   477.050 |     6.3%
@typescript-eslint/no-unused-vars           |   456.465 |     6.0%
jsdoc/require-description-complete-sentence |   416.341 |     5.5%
indent-legacy                               |   287.541 |     3.8%
react/jsx-indent                            |   273.187 |     3.6%
jsdoc/check-indentation                     |   258.600 |     3.4%
jsdoc/check-tag-names                       |   252.098 |     3.3%
jsdoc/check-alignment                       |   232.608 |     3.1%

snowystinger avatar Apr 17 '21 00:04 snowystinger

Just a friendly bump :)

snowystinger avatar Sep 01 '21 21:09 snowystinger

another friendly bump :) would love to get rid of our patch-package

snowystinger avatar Nov 19 '21 02:11 snowystinger

FWIW I published a fork at @jdb8/eslint-plugin-monorepo to npm if it's useful: https://github.com/jdb8/eslint-plugin-monorepo

It contains this PR and https://github.com/azz/eslint-plugin-monorepo/pull/29, which should provide some additional perf improvements.

jdb8 avatar Nov 19 '21 03:11 jdb8