4 ms·

Author here. Thanks for the feedback. The reason I wrote this article is because we _can_ do something about it. Through avoiding throwing lots of wasteful err

Author here. Thanks for the feedback. The reason I wrote this article is because we _can_ do something about it. Through avoiding throwing lots of wasteful error objects and adding a little bit of caching, the time it took to lint the project became 30% faster. Those changes were applied locally to a couple of popular third party tools for module resolution in node_modules. That's how the 30% speedup was achieved. That said if module resolution wasn't as complex in node to begin with, the speedup would surely be a little greater. I'm hoping that this post sparks a bit of discussion on that and some node contributors already voiced interest on twitter to think more about that.

acemarke 4y ago
Great investigation work and excellent writeup! Out of curiosity: did anyone end up filing issues or PRs against these tools as a result of either of your articles?
- mhagemeister 4y ago
  Thanks, happy you enjoyed the article! The PRs for the previous article were all merged. I originally wanted to do the same for this one, but I'm not sure if I have the time to fix all of them. Updating to a newer version of `resolve` already addresses the most notable issue with throwing more errors than necessary, but many parts of the ecosystem still use an old version. Manged to land this PR in another package though https://github.com/import-js/eslint-import-resolver-typescript/pull/206 https://github.com/import-js/eslint-import-resolver-typescri... . Performance there could be easily improved further there. Another PR to an eslint plugin was unfortunately rejected as it broke node 4 support https://github.com/import-js/eslint-plugin-import/pull/2654 https://github.com/import-js/eslint-plugin-import/pull/2654 . The other popular package that's used for module resolution is `enhanced-resolve` by the webpack folks and they expect the consumer to deal with passing the appropriate options. So there isn't really a single place to fix this.
  - epmatsw 4y ago
    Ugh. ESLint-plugin-import, I should have known. Not at all surprising that a perf fix wouldn’t be prioritized there unfortunately.
    - gratitoad 4y ago
      Can I ask why that’s not at all surprising to you?
stephen 4y ago
Awesome! I see the PRs you've linked to; sorry, I'd misinterpreted the post as experiments/"what if" exploration and not "there are patches landing soon". That's great!
- mhagemeister 4y ago
  No worries, that's valuable feedback. I should have made it more clear in the article that it's not just theory.

acemarke 4y ago

Great investigation work and excellent writeup! Out of curiosity: did anyone end up filing issues or PRs against these tools as a result of either of your articles?

mhagemeister 4y ago
Thanks, happy you enjoyed the article! The PRs for the previous article were all merged. I originally wanted to do the same for this one, but I'm not sure if I have the time to fix all of them. Updating to a newer version of `resolve` already addresses the most notable issue with throwing more errors than necessary, but many parts of the ecosystem still use an old version. Manged to land this PR in another package though https://github.com/import-js/eslint-import-resolver-typescript/pull/206 https://github.com/import-js/eslint-import-resolver-typescri... . Performance there could be easily improved further there. Another PR to an eslint plugin was unfortunately rejected as it broke node 4 support https://github.com/import-js/eslint-plugin-import/pull/2654 https://github.com/import-js/eslint-plugin-import/pull/2654 . The other popular package that's used for module resolution is `enhanced-resolve` by the webpack folks and they expect the consumer to deal with passing the appropriate options. So there isn't really a single place to fix this.
- epmatsw 4y ago
  Ugh. ESLint-plugin-import, I should have known. Not at all surprising that a perf fix wouldn’t be prioritized there unfortunately.
  - gratitoad 4y ago
    Can I ask why that’s not at all surprising to you?

stephen 4y ago

Awesome! I see the PRs you've linked to; sorry, I'd misinterpreted the post as experiments/"what if" exploration and not "there are patches landing soon". That's great!

mhagemeister 4y ago
No worries, that's valuable feedback. I should have made it more clear in the article that it's not just theory.