Skip to content
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

fix(server): avoid chokidar throttling on startup #15347

Merged
merged 1 commit into from Dec 14, 2023

Conversation

mattkubej
Copy link
Contributor

@mattkubej mattkubej commented Dec 14, 2023

Description

Vite dev server producing unexpected hmr updates on startup without changes. Chokidar is throttling directory reads on initial watch, producing add events, and vite responds with hmr updates to the add events. Deduping the array passed to chokidar.watch on server creation fixes the issue.

Additional context

We noticed during the startup process of the vite dev server with our project that it would regularly and seemingly randomly produce hmr update logs, even though no changes were being made during the startup process. What was discovered was that chokidar was throttling directory reads during the initial watch on vite server creation. Throttled events from chokidar result in add events emitted even with ignoreInitial set to true. Vite listens for these add events and triggers an hmr update for the associated file.

The throttling is occurring, because the files/directories passed to chokidar.watch could and in our case did contain duplicate directories. chokidar does not appear to dedupe the array passed to it, so chokidar ended up duplicatively reading the same directories via _handleRead. It appears the intention of the throttling within _handleRead is to catch changes that may have occurred during the initial watch process. So, chokidar treated the duplicative reads as throttled changes (i.e. initialAdd as false and emits an add event), which informed vite, and vite responded with an hmr update.

The default configuration of vite appears to encounter this, which is where we hit it. As our root option was process.cwd() and envDir was defaulting to root. So, chokidar.watch receives the root directory twice via root and config.envDir.

This PR uses a Set to remove any duplicate strings from the array of files/directories passed to chokidar.watch to avoid this problem (i.e. remove the duplicate config.envDir entry).

The workaround for this ahead of this change is to not have root and config.envDir share the same value (i.e. set config.envDir to a different directory).

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link

stackblitz bot commented Dec 14, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev
Copy link
Member

Great find! This also reduced by half the time to setup the watcher on my M1. We were discussing with @sapphi-red and @ArnaudBarre about this because in Windows it takes a very long time to setup the watcher. Let's merge and release this one. I think we may be able to do even better later if we not only de-duplicate but also check if the files are inside another and skip them... normally we will probably then end up only with root.

@patak-dev patak-dev merged commit 56a5740 into vitejs:main Dec 14, 2023
13 checks passed
@patak-dev patak-dev added the p3-significant High priority enhancement (priority) label Dec 14, 2023
@wojtekmaj
Copy link

Wouldn't it be beneficial for the whole ecosystem if we copied this fix over to chokidar itself? Or is there a good reason it's not deduping the items on its own?

@patak-dev
Copy link
Member

@wojtekmaj totally, it would be great if this would be upstreamed

@mattkubej
Copy link
Contributor Author

mattkubej commented Dec 14, 2023

Definitely could and likely should to make it more defensive. This would be the spot it seems the change would need to be made. However, I think it would still be good for vite to have this deduplication logic and not rely on chokidar to do it.

Tangentially, it might be worth considering allowing for pluggable file watchers in vite (e.g. allowing to swap chokidar for watchman).

@patak-dev
Copy link
Member

@mattkubej we were looking into migrating to Parcel Watcher, see:

If you think it is important to make the watcher pluggable, you could start a discussion with some use cases at one point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-significant High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants