Order lint warnings by most recently edited file (#2378)#2409
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
Wow, that’s really neat! I’m going to review this when I get some extra time (currently busy with other things). Thanks for PR! |
|
When looking at the console I want to see the most important thing last. So for me it would be more useful to have the most recent file last. |
Do you mean browser console? I think in the browser console we might as well hide anything except last edited file. |
| ...compilation.warnings, | ||
| ].sort((warning1, warning2) => { | ||
| if (!warning1._lastModifiedDate) { | ||
| warning1._lastModifiedDate = fs.statSync( |
There was a problem hiding this comment.
Is there a way to avoid these calls every time? I think webpack may expose this information in a cached way via its own "filesystem" object. Or maybe there's some other global object that already contains last edit times. Could you check for this?
There was a problem hiding this comment.
I now use compilation.fileTimestamps. I still have fs.statSync as a fallback because fileTimestamps is empty on the first compile for some reason :/. But the next few compiles are using webpack's stats.
|
I understand your point @ulrikstrid : if you have warnings in a lot of files, you might scroll past the first file in the terminal, so having the order reversed might be more practical. |
|
Yeah, I ment the terminal and not the browser. |
|
It would be nice if there was some way to specify an option for sort direction somewhere. (More generally, it would be nice if create-react-app/react-scripts had a config file to specify options of many kinds. It would likely reduce many folks' need to eject and allow them to continue benefitting from new developments in CRA. If such a config file already exists, please tell me.) |
|
Every option doubles the amount of things that can go wrong and that we need to test for. We intentionally limit how much is customisable to keep the project manageable and stable. I encourage you to look into alternatives like Neutrino if you prefer to customise things. |
| compiler.plugin('after-compile', (compilation, callback) => { | ||
| // Order compilation warnings by most recently modified | ||
| compilation.warnings = [ | ||
| ...compilation.warnings, |
There was a problem hiding this comment.
since it sorts in place, is it necessary to clone the array?
| ].sort((warning1, warning2) => { | ||
| const warning1Time = compilation.fileTimestamps[ | ||
| warning1.module.resource | ||
| ] || fs.statSync(warning1.module.resource).mtime.getTime(); | ||
| const warning2Time = compilation.fileTimestamps[ | ||
| warning2.module.resource | ||
| ] || fs.statSync(warning2.module.resource).mtime.getTime(); | ||
|
|
||
| return warning1Time < warning2Time ? 1 : -1; |
There was a problem hiding this comment.
| ].sort((warning1, warning2) => { | |
| const warning1Time = compilation.fileTimestamps[ | |
| warning1.module.resource | |
| ] || fs.statSync(warning1.module.resource).mtime.getTime(); | |
| const warning2Time = compilation.fileTimestamps[ | |
| warning2.module.resource | |
| ] || fs.statSync(warning2.module.resource).mtime.getTime(); | |
| return warning1Time < warning2Time ? 1 : -1; | |
| ].sort((...warnings) => { | |
| const [a, b] = warnings.map(({ module: { resource } }) => ( | |
| compilation.fileTimestamps[resource] || fs.statSync(resource).mtime.getTime() | |
| )); | |
| return a === b ? 0 : a < b ? 1 : -1; |

Hi
I've added sorting to the warning messages by last file modified time. This makes recently edited file pop up at the top of the error list. This affects logs both in the terminal and in the browser console (which seems more intuitive anyway).
I've checked by running
npm-startin the project and modifyingtemplates/index.jsandtemplates/App.jsalternatively, verifying that the order is coherent (see screenshots).And then after editing the
index.jsfileI've also checked in the browser console of course. I'm working on a mac.
If this seems like the proper resolution for this bug, I can check on more platforms (like windows) !
Cheers