feat: add client-only islands support#3783
Open
bartlomieju wants to merge 10 commits into
Open
Conversation
Remove the 78-line Babel `inlineEnvVarsPlugin` and replace it with Vite's built-in `define` configuration for `process.env.FRESH_PUBLIC_*` and `import.meta.env.FRESH_PUBLIC_*` patterns. A lightweight regex-based Vite plugin handles `Deno.env.get()` calls which can't use `define`. Env file loading moved from `configResolved` to `config` so define entries are available during Vite's config resolution phase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of transforming CJS to ESM via a 960-line Babel plugin, let Vite handle CJS packages natively by: 1. Removing `meta.deno` from file:// resolved paths in deno.ts so Vite loads npm packages from disk instead of through @deno/loader 2. Removing `noExternal: true` so Vite externalizes npm packages in SSR dev mode (Node.js handles CJS natively via require()) 3. Removing `noDiscovery: true` so Vite's dependency optimizer can pre-bundle CJS packages for the client 4. Applying resolve.alias before Deno resolution so react -> preact/compat works even when packages are externalized Also converts local .cjs test fixtures to ESM since they no longer go through the CJS transform. Deletes ~1,800 lines. Eliminates the #1 source of npm compat bugs (#3619, #3653, #3505, #3478, #3449). Known regressions (2 tests): radix-ui and remote island need investigation for duplicate preact instances with externalization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l for radix-ui Apply Vite's resolve.alias config (e.g. react -> preact/compat) in deno.ts before calling @deno/loader, so the alias works even when packages are externalized in SSR mode. Also add noExternal for @radix-ui packages in the SSR environment since they depend on React compat aliases being applied. WIP: radix test still failing — alias format from Vite config needs further investigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add lightweight CJS shim in deno.ts load hook for dev mode: wraps CJS files in node_modules with module/exports/require so Vite's SSR module runner can evaluate them. Only ~30 lines vs the old 960-line Babel CJS transform. Only runs in dev mode — build mode uses Rollup's @rollup/plugin-commonjs natively. - Restore ssr.noExternal: true so resolve.alias (react -> preact/compat) is applied consistently in SSR. Without it, Node.js require() bypasses aliases and loads real react@19.1.1. - Apply resolve.alias in deno.ts resolveId before @deno/loader runs, so aliased specifiers (react, react-dom) resolve to preact/compat through the Deno resolution pipeline. - Remove environment-level noExternal (was duplicated). Test results: 35/36 dev tests pass, 31/31 build tests pass. The 1 failing test (remote island) is pre-existing on main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of disabling the dependency optimizer entirely (noDiscovery), exclude only preact ecosystem packages from optimization. This allows CJS packages like mime-db to be pre-bundled for the browser while preventing duplicate preact instances when remote (JSR) islands resolve deps to /@fs/ paths. Also extends the CJS shim to work in both SSR (with createRequire) and client (with stub require) environments. All dev server tests pass (35/36 — 1 pre-existing flaky failure on remote island that also fails on main). All 31 build tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The dependency optimizer causes duplicate preact instances when remote (JSR) islands resolve deps to /@fs/ paths while the optimizer bundles to /.vite/deps/. Restore noDiscovery: true to prevent this. For CJS packages used in client-side islands (like mime-db), convert require() calls to ESM import statements via regex so browsers can load them. The SSR shim continues to use Node.js createRequire. All tests pass: 36/36 dev, 31/31 build, 15/15 patches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `links` field referenced a sibling directory that only exists on the author's machine, causing `deno install` to fail in CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Islands marked with `export const clientOnly = true` skip server-side rendering entirely — Fresh renders an empty placeholder on the server and the component renders normally on the client. This supports libraries like Monaco Editor that reference browser globals at the module top level.
lunadogbot
suggested changes
May 12, 2026
Contributor
lunadogbot
left a comment
lunadogbot
left a comment
There was a problem hiding this comment.
Feature core is small and clean: SSR renders <div></div> + a :c marker (packages/fresh/src/runtime/server/preact_hooks.ts:300-314), the reviver picks it up, and the existing render() path populates the empty container. Fixture + tests cover both SSR (placeholder + marker) and client (component runs).
Two things to address:
-
IslandReq.clientOnlyis dead.packages/fresh/src/runtime/client/reviver.ts:24,273-279parses the:cflag and stores it on the request, but no client path reads it —revive()atreviver.ts:115always callsrender()unconditionally. The feature works "by accident" because rendering into the empty<div>patches it to the component output. Either drop the field and the:cmarker variant (the empty SSR<div>alone is enough; the existing path handles it), or wireclientOnlythrough to a code path that actually branches on it. -
Scope. Title is
feat: add client-only islands supportbut the diff also deletes the CJS→ESM and inline-env-vars Babel patches (~1950 LOC acrosspackages/plugin-vite/src/plugins/patches/commonjs.ts,commonjs_test.ts,inline_env_vars.ts,inline_env_vars_test.ts) and rewritespackages/plugin-vite/src/mod.ts+plugins/deno.tsto use Vitedefine+ssr.noExternal. The CJS transform was just improved in #3697 (d6b0a16); reversing it is a strategy change that deserves its own PR and its own title. Reviewed standalone, the islands change is ~150 LOC.
- nit:
deno.lockrollspreact10.29.1 → 10.29.0 (and@preact/signals/preact-render-to-stringfollow). Not a deps PR; looks accidental.
4 tasks
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.
Summary
export const clientOnly = truein island files<div>during SSR instead of executing the component, avoiding crashes from libraries that reference browser globals (e.g.document) at the module top levelrender()defineand removing the CJS→ESM Babel transformTest plan
:cmarker flag is presentno_client_jsandheadtest suites pass