Skip to content

fix: make addWatchFile() work (fix #7024)#9723

Merged
patak-cat merged 6 commits intovitejs:mainfrom
zqianem:fix/addWatchFile
Nov 16, 2022
Merged

fix: make addWatchFile() work (fix #7024)#9723
patak-cat merged 6 commits intovitejs:mainfrom
zqianem:fix/addWatchFile

Conversation

@zqianem
Copy link
Contributor

@zqianem zqianem commented Aug 18, 2022 •
edited
Loading

Description

fixes #7024 by ensuring addWatchFile adds files to the module graph

Additional context

The intention of db4ba56 was to allow plugins to add files to the module graph via addWatchFile() so that changes to those files would cause HMR updates. However, for files without any ES imports, the import analysis returns early and the files passed to addWatchFile() in this._addedImports are never added to the graph. Checking for the existence of this._addedImports fixes this.

Additionally, the new forceSkipImportAnalysis flag is needed to mark the newly created modules as not needing additional analysis so that the HMR update can propogate (see #8898).

I could try to add the tests from #4461 if they are still relevant. Cherry-picked the test from #4461 and verified that it fails without 33938a3.

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.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • 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).
  • Ideally, include relevant tests that fail without this PR but pass with it.

zqianem added a commit to byrd-polar/fluid-earth that referenced this pull request Aug 23, 2022
@zqianem
Support HMR for GLSL files
fb6a285 
Now edits to GLSL files will cause dev server to automatically update
the page, as expected. Previously only worked for the GLSL files
directly imported by JS.

Done by patching dependencies; should try to get some version of these
changes merged upstream.

See vitejs/vite#9723
@zqianem zqianem changed the title fix(dev): make addWatchFile() work fix: make addWatchFile() work (fix #7024) Sep 22, 2022
@zqianem
Copy link
Contributor Author

zqianem commented Sep 22, 2022

Rebased onto latest main and re-ordered the commits (add tests first) for easier verification of fix.

@zqianem
Copy link
Contributor Author

zqianem commented Nov 16, 2022

Rebased again to verify that the fix is still necessary (the added test fails without) and that it still works. I think the failed tests are due to flakiness — different tests fail on the same commit in my fork.

@patak-dev could you please take a look at this? I'm worried that this PR has missed the triage process due to lack of labels and wasn't sure how to reach out; apologies if this is already on the radar. Thanks!

@patak-cat patak-cat added p3-minor-bug An edge case that only affects very specific usage (priority) feat: hmr labels Nov 16, 2022
@patak-cat
Copy link
Member

patak-cat commented Nov 16, 2022

@zqianem thanks for the ping (and for the patience). Looks good to me 👍🏼
I will send it to #contributing to https://chat.vitejs.dev to get another review or merge it tomorrow if not because we can test it safely during the beta.

@patak-cat patak-cat merged commit 34db08b into vitejs:main Nov 16, 2022
fc pushed a commit to fc/vite that referenced this pull request Nov 23, 2022
@zqianem @mattcompiles
fix: make addWatchFile() work (fix vitejs#7024) (vitejs#9723)
d9028db 
Co-authored-by: Matt Jones <mattjones701@gmail.com>
@sapphi-red sapphi-red mentioned this pull request Nov 25, 2022
Closed
9 tasks
@zqianem zqianem mentioned this pull request Jan 2, 2023
Closed
@Pauan Pauan mentioned this pull request Jul 27, 2023
Open
techdev-loop pushed a commit to techdev-loop/WebGL-Fluid-Earth-Visualization that referenced this pull request Mar 26, 2025
Support HMR for GLSL files
d0a800a 
Now edits to GLSL files will cause dev server to automatically update
the page, as expected. Previously only worked for the GLSL files
directly imported by JS.

Done by patching dependencies; should try to get some version of these
changes merged upstream.

See vitejs/vite#9723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patak-cat patak-cat patak-cat approved these changes

@bluwy bluwy bluwy approved these changes

Assignees

No one assigned

Labels

feat: hmr p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

addWatchFile can not be used to watch files outside of the module graph

4 participants

@zqianem @patak-cat @bluwy @mattcompiles