-
Notifications
You must be signed in to change notification settings - Fork 3
The editor goes blank when fix when save is enabled #24
Comments
To diagnose this, I'd like to ask a couple questions:
If none of the above explains the issue, then I'll do my best to track this down. It's odd because, to the best of my understanding, the fix-on-save behavior has not substantially changed in this package from what it was in |
I did notice this is on Atom v1.63.0-nightly1 which has an updated version of electron. |
@UziTech unrelatedly, 1.0.6 isn't showing up when I check for updates, and the page on atom.io gives me a 500 :-/ |
@savetheclocktower ya atom is having issues with the website. Unfortunately I don't have access to do anything about it. I think the latest version is part of the same problem. |
I have noticed this being an issue as well, but only on files with several thousand lines of code. Smaller documents have not (yet) had an issue. I am on the latest version of atom, 1.60.0 |
Happy to run more scenarios to help diagnose. Let me know. |
Thanks for the information, @eaviles. This is stumping me so far. We are using the same approach for fixes that linter-eslint uses, which is to let the ESLint engine handle fixing and writing the new file. The way we communicate with the worker is slightly different, but I don't see how that matters. I haven't gone digging into the ESLint source code, but I would assume that the file is being rewritten atomically. If the file is being written atomically, then it stands to reason that Atom isn't picking up on the filesystem change too quickly — that once it realizes the files has changed, the new file contents are there to consume. If so, then the momentarily blank editor pane would suggest a bug in Atom somewhere, but I'm not sure of the form it would take — or, in @dpkrowe's case, why it would manifest only some of the time. There's one possible way to sidestep this: we could rely on ESLint to make the fixes, but to hand the new file to us as a string rather than save it to disk. We could then replace the entire contents of the file within Atom and perform a second save. This feels silly, but if this ends up being a widespread issue, it'd almost certainly be worth doing. Leaving this open for now so that other people can weigh in with their experiences. Maybe we can narrow down the conditions under which it happens. |
Hi, Any progress on this? I'm having the same issue. It only happens on large files, around 800+ lines. Atom version: 1.58.0 |
@savetheclocktower do you think that this could be a side effect of removing the queue? That being that the process if running multiple tasks at once it just can't write the fixes fast enough to the file system? |
@scagood I think that there's still an implicit queueing based on how ndjson works — there's one worker per project, and as far as I know it's synchronously linting/fixing each file in turn if a bunch of them request to be linted at the same time. Only about 90% certain of that, though, so if you showed me evidence otherwise, I wouldn't accuse you of lies. I haven't had the time to try a serious stress test, but if nobody else figures it out eventually I'm just going to lock myself in a room with this bug and have it out. |
Just for a bit of an explanation from my thought process, the promise queue I added initially was used for queueing jobs, not for queing the json chunks (the chunks queueing was actually done by the Stream#write call). index.js:148-152 So before the jobs would get stalled like this (stalling is done it atom, and is highlighted in red): I think the way I would start to attempt to test this hypothesis would be by recording the start time and the end time of the jobs. Then storing that in a way that we can then include in the debug output. |
Yeah, I get the distinction, and I think I was just mistaken. My faulty recollection was that methods like That may or may not be causing us problems, but it sounds like this bug is happening when someone is saving a single file, not just in the midst of a bunch of linting jobs. I really don't want to go digging into the ESLint internals to figure out exactly how it's writing files and whether the approach is different than it used to be, but I might have to. |
mmm, you're right, I have also had this happen when saving singular files. I am just trying to come up with some metric to help work this out 😅 Interestingly I have only had this when working on linux (ubuntu) but never when I am on mac, the main difference I can think of (excluding the os ofcs) is when I am on linux I have a load more plugins loaded 🤔 |
Maybe the specifics of the filesystem are the difference? Maybe it's the speed of the hard drive? |
I have not seen this issue in a very long time. @eaviles @UziTech @dpkrowe @cmens have any of you seen this recently, if not, can we close this issue? @savetheclocktower I dont think you experienced this, right? |
I never experienced this. Let's see if other people weigh in. If nobody has had this happen in a while, then we can close this ticket. |
Issue Type
Bug
Issue Description
Whenever I have "Fix errors on save" enabled, the entire editor window goes blank before appearing again with the corrected file. The editor stays blank for about 1 second and when is back the current cursor and scroll position is reset which makes the option unusable.
Bug Checklist
eslint
CLI gives the proper result, whilelinter-eslint-node
does notLinter Eslint Node: Debug
command from the Command Palette belowThe text was updated successfully, but these errors were encountered: