-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
perf: reduce filesystem calls #229
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #229 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 291 289 -2
Branches 81 75 -6
=========================================
- Hits 291 289 -2 ☔ View full report in Codecov by Sentry. |
@ricardogobbosouza Friendly ping, do you need help here? |
I need to create a large repository to test performance |
|
||
await Promise.all( | ||
files.map(async (file) => { | ||
const result = await eslint.lintText(file.content, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, nice PR !
ESLint introduced a new Linter
class that could be better suited to this.
This class also has the advantage of supporting the new ESLint config format: https://eslint.org/blog/2022/08/new-config-system-part-3/#using-flat-config-with-the-linter-class
src/index.js
Outdated
// istanbul ignore next | ||
const content = String( | ||
(module.originalSource | ||
? module.originalSource() | ||
: // eslint-disable-next-line line-comment-position | ||
// @ts-ignore | ||
// eslint-disable-next-line no-underscore-dangle | ||
module._source | ||
).source() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to find time to give you a proper repro, I tried this on a repository of mine and hit two issues:
- Some files seem to bot have a "source" property at all
- If it has, it's not the source but something that's already slightly changed (I got the case that an "original" source had an indentation of 4 spaces instead of the 2 spaces of the actual file)
Here is the branch where I'm experimenting; https://github.com/swissquote/crafty/blob/flat-eslint
I apply my own PR to this repository through a patch : https://github.com/swissquote/crafty/blob/flat-eslint/.yarn/patches/eslint-webpack-plugin-npm-4.0.1-da1b820642.patch
I made two modifications to the original PR:
- skip getting content because of issue 1 above: https://github.com/swissquote/crafty/blob/flat-eslint/.yarn/patches/eslint-webpack-plugin-npm-4.0.1-da1b820642.patch#L118-L122
- keep "lintFiles" for now: https://github.com/swissquote/crafty/blob/flat-eslint/.yarn/patches/eslint-webpack-plugin-npm-4.0.1-da1b820642.patch#L345
Feel free to experiment with this branch in the meantime, I will try to propose a repro for the next few days and try to understand why these two issues happen
(I apply the patch on the branch here : )
@ricardogobbosouza Do we need this now? |
Use the contents of the file using
module.originalSource
ormodule._source
reducing calls to files.Now we use the function the
lintText
instead oflintFiles
*Need to add the performance improvements
This PR contains a:
Motivation / Use-Case
Resolve #157
Breaking Changes
No
Additional Info
No