-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[PoC] Use chokidar instead of the fsevents/fsevents_win combo #12190
Conversation
/cc @MiguelCastillo |
@ficristo Ha! build failed because
Maybe trigger the build again and see what happens. |
I had totally missed the |
Actually it seems to me that the file watcher is recursive on Windows and OSX. |
@petetnt Could you try your extension https://github.com/petetnt/corrupt-brackets-history with this PR? |
@ficristo Tested it, first corruption occurred on the 116th try 😭 Otherwise I had no issues. |
@ficristo let me take a step back. File watching is a very critical path piece functionality that we cannot regress on. So, it is important that we understand the impact of this proposed changes. Do you think you can help us understand:
These three questions are very important before we continue going down this path. Thank you so much for understanding why I am being very cautious about these changes :) |
@MiguelCastillo This was an intersting thing to explore for me. I don't mind if it takes it's time. Currently:
With chokidar:
This probably will require an update to CEF where only OSX 64 bit is supported. |
@ficristo Thats an awesome set of information. I think it makes sense to gather some metrics to understand how we will be affected. Updating CEF is a PITA. We should explore if we have any plans to update soon. |
I've tried to add the preferences for the paths to ignore but I seems to have encountered some circular reference problems... |
@ficristo I've build shell using adobe/brackets-shell#543 and used this PR as a base to run tests. I can see some fs watching logged to the console but running the tests fails: |
Use chokidar 1.6.0 installed with npm@3 and npm install --production
@zaggino I fixed the tests. |
Thanks @ficristo, will continue testing this today. |
Replaced by #12647. |
This change still cause significant cpu usage.
Only looked at the Activity Monitor on OSX or the Task Manager on Windows.
See #12187