Skip to content

fix watcher race condition #56

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

Merged
merged 2 commits into from
Jan 6, 2021
Merged

Conversation

marcoemrich
Copy link
Contributor

@marcoemrich marcoemrich commented Dec 30, 2020

see issue #55
on some sytems the .compiler.log was not readable after
chokidar called sendUpdateDiagnostics. Setting
awaitWriteFinish as an option fixes the issue

on some sytems the compiler was not readable after
chokidar called sendUpdateDiagnostics. Setting
"awaitWriteFinish" as an option fixes the issue
@marcoemrich marcoemrich changed the title fix watcher race condition. see issue #55 fix watcher race condition Dec 30, 2020
@chenglou
Copy link
Member

chenglou commented Dec 30, 2020

Thanks! This heuristics is really unfortunate. And 500ms is a lot. I'd rather long poll than use a watcher in this case, as the watcher has other problems (e.g. not detecting the creation of the lib/bs directory at all). Are you sure 500ms is needed?

@marcoemrich
Copy link
Contributor Author

ups, no! I already updated to 1ms. Seems it didn't make it upstream. Sorry - I update the PR.

@chenglou
Copy link
Member

chenglou commented Jan 6, 2021

Cool, thanks! Merging. Let's hope there's no report of new odd behaviors. Watchers are scary things we're thinking of removing one day...

@chenglou chenglou merged commit f8fc490 into rescript-lang:master Jan 6, 2021
@marcoemrich
Copy link
Contributor Author

Thanks for Merging! Yes, watchers are really hard to use, every file system could behave differently. I'm just happy it works right now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants