Skip to content

fix(plugin-vite): register co-located islands added while dev server runs - #3903

Open
limkaa4 wants to merge 1 commit into
freshframework:mainfrom
limkaa4:fix/vite-colocated-island-watcher
Open

fix(plugin-vite): register co-located islands added while dev server runs#3903
limkaa4 wants to merge 1 commit into
freshframework:mainfrom
limkaa4:fix/vite-colocated-island-watcher

Conversation

@limkaa4

@limkaa4 limkaa4 commented Sep 12, 2026

Copy link
Copy Markdown

Problem

Creating a new island inside a (_islands) folder while the dev server is
running does nothing — the island stays unregistered until the server is
restarted. Editing an existing island works fine, so it reads like flaky
HMR rather than a missing registration.

The layout is the documented one from
concepts/file-routing.md:

routes/
├── (marketing)/
│   └── (_islands)/
│       └── interactive-stats.tsx
└── shop/
    └── (_islands)/
        └── cart.tsx

Cause

isIslandPath() in client_snapshot.ts gates what the watcher feeds into
the client snapshot, and its co-located branch is unreachable:

const relIsland = path.relative(options.islandsDir, filePath);
if (!relIsland.startsWith("..")) return true;

const relRoutes = path.relative(options.routeDir, filePath);

if (!relIsland.startsWith("..") && relRoutes.includes("(_islands)")) {
  return true;
}

We only reach the second if when relIsland does start with "..", so
re-testing the same value is always false. relRoutes is computed and
then never used for the decision.

The initial crawl in fs_crawl.ts handles these islands correctly, which is
why a restart fixes it — only files created after startup are affected.

Fix

Test relRoutes, and match (_islands) as a path segment rather than a
substring, matching GROUP_REG in fs_crawl.ts. Files outside both
directories now bail out early instead of relying on a substring check.

Tests

client_snapshot_test.ts covers the top-level islands/ dir, both
documented co-located placements (inside a route group and inside a plain
route folder), and the negative cases — (_components), a (_islands)x
substring, and a matching folder outside both directories.

Verified the tests fail against the current implementation and pass with the
fix. deno fmt, deno lint, deno task check:types and
deno test -A packages/plugin-vite/src/plugins/ are clean.

No integration test: the neighbouring dev-server HMR test is already marked
ignore: true for flakiness, and the regression is fully captured by the
pure function.

🤖 Generated with Claude Code

…runs

`isIslandPath()` decides whether a file the watcher reports should join the
client snapshot. Its second branch, the one meant to cover co-located
islands, could never run:

    const relIsland = path.relative(options.islandsDir, filePath);
    if (!relIsland.startsWith("..")) return true;
    ...
    if (!relIsland.startsWith("..") && relRoutes.includes("(_islands)")) {

Control only reaches the second check when `relIsland` does start with
"..", so the re-test of the same value is always false. A file created in
a `(_islands)` folder was therefore dropped by the watcher, and the island
stayed unregistered until the dev server restarted — while editing an
existing island kept working, which makes it look like flaky HMR rather
than a missing registration.

The layout itself is documented in `concepts/file-routing.md`, and the
initial crawl in `fs_crawl.ts` handles it correctly, so this only affects
files created after the dev server is up.

Test against `relRoutes` instead, and match `(_islands)` as a path segment
rather than a substring, the same way `GROUP_REG` does in `fs_crawl.ts`.
The added tests cover both documented placements: a route group and a
plain route folder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant