fix(vite-plugin): keep watching config changes after a failed restart#14610
Open
martijnwalraven wants to merge 2 commits into
Open
fix(vite-plugin): keep watching config changes after a failed restart#14610martijnwalraven wants to merge 2 commits into
martijnwalraven wants to merge 2 commits into
Conversation
The config-change handler — which watches the Worker config files, local dev vars (.dev.vars/.env), and the assets configuration — unregistered itself from the watcher before calling viteDevServer.restart(). When the restart fails (an invalid updated config makes Vite's restartServer catch the replacement server's creation failure, log 'server restart failed', and resolve with the current server and its watcher left in place; the same behavior on Vite 6, 7, and 8), nothing ever re-registered the handler: only a successfully created server runs configureServer again. From that point every config change, including the one fixing the config, was silently ignored while source HMR kept working — the session looked healthy while serving a config that no longer matched the file. The handler now stays registered for its server's whole life and guards against re-entrant restarts with an in-flight flag instead. The guard silences only this server's handler while its own restart is in flight — the same window the previous code silenced by unregistering; a change landing in that window can still be picked up by the replacement server's own handler once it is registered, as before. After a failed restart the next change re-attempts. On a successful restart the old watcher is closed and the new server registers its own handler, so the stale handler never double-fires — the same lifecycle the export-shape restart in dev.ts already relies on. The regression test drives the exact sequence through the config-changes playground: break the config (main pointing at a missing file), await the reported error, fix the config, and assert the new var value is served. It fails on the previous code — at the first step already when it runs after the existing broken-config test, whose failed restart has permanently killed config watching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 8d84aca The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
dario-piotrowicz
and removed request for
a team
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
REVIEW.md: changeset titles are plain descriptions, no conventional commit prefixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a dev-server bug where the plugin permanently stops watching config changes after one failed restart.
The bug.
configureServerinplugins/config.tsregisters a change handler covering the Worker config files, local dev vars (.dev.vars/.env), and the assets configuration. It removed itself from the watcher (watcher.off) before callingviteDevServer.restart(). When the restart fails — e.g. the updatedwrangler.jsonis invalid — Vite'srestartServercatches the replacement server's creation failure internally, logsserver restart failed, and resolves with the current server (and its watcher) still in place; only a successfully created server runsconfigureServeragain, so nothing re-registers the handler. From that point on, every config change — including the one that fixes the config — is silently ignored. Source HMR keeps working, which makes the session look healthy while the server keeps serving a config that no longer matches the file. The behavior is the same on Vite 6, 7, and 8.Repro (before this change): start
vite dev, editwrangler.jsonto something invalid (or amainthat doesn't resolve), then fix it → the fix is never applied; only a manual dev-server restart recovers.The fix. The handler stays registered for its server's whole life and guards against re-entrant restarts with an in-flight flag instead of unregistering. The guard silences only this server's handler while its own restart is in flight — the same window the previous code silenced by unregistering (a change landing mid-restart can still be picked up by the replacement server's handler once it is registered, as before). After a failed restart the next change re-attempts; after a successful restart the old watcher is closed and the new server has registered its own handler, so the stale handler never double-fires — the same lifecycle the export-shape restart in
dev.tsalready relies on.The test extends the
config-changesplayground with the exact sequence: break the config (main→ missing file), await the reported error, fix the config, assert the new var value is served. On the previous code it fails — notably at the first step already when running after the existing "reports errors in updates" test, whose failed restart has already killed config watching.config-changesplayground; fails before the fix, passes after)🤖 Generated with Claude Code