feat:Actually ship no JS by default#3696
Conversation
- Introduced `hmrClientEntry` to the `BuildCache` interface for development mode. - Updated the `Context` class to conditionally add preload headers based on client runtime needs. - Moved HMR client logic in `dev_hmr.ts`. - Enhanced the `RenderState` to track client navigation requirements and runtime needs. - Added tests to ensure no client JS is emitted for static pages without islands or client navigation.
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
See comment about the lockfile
|
I could not run |
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Nice work — this fixes a real gap between Fresh's marketing ("zero JS by default") and reality. The approach of checking islands.size > 0 || clientNavEnabled is clean, and the HMR split is a good call so dev mode still gets live reload on static pages.
A few things to consider:
Logic issue in FreshRuntimeScript: After the PR, when needsClientRuntime is false, the function falls through past the if (RENDER_STATE!.needsClientRuntime) block to the HMR/production-static returns at the bottom. But when needsClientRuntime is true, the full-document branch already returns inside the block — so that's fine. However, the partial response path (line ~582) returns early and doesn't check needsClientRuntime at all. That seems correct for partials, but worth a comment noting that partials always assume the client runtime is already loaded by the parent page.
f-client-nav detection only checks "true" string: The diff hook checks (vnode.props)[CLIENT_NAV_ATTR] === "true", but earlier in the same file (line ~145-146), the attribute is coerced via String(vnode.props[CLIENT_NAV_ATTR]). So passing f-client-nav as a boolean true (JSX f-client-nav or f-client-nav={true}) gets stringified to "true" before this check runs — that works. But the test uses <html f-client-nav> which in Preact/JSX becomes true (boolean). Worth verifying this ordering is guaranteed (diff hook runs after vnode hook), and maybe add a brief comment explaining the dependency.
f-client-nav={false} test: The test asserts no boot script, which is correct. But f-client-nav={false} still gets stringified to "false" and rendered into the HTML as f-client-nav="false". The detection correctly skips it since "false" !== "true". Good.
Minor: The isJsMediaType exhaustive return at the end of deno.ts — this is unrelated to the feature but fine as a drive-by fix for the type checker.
Overall this looks good to merge once the ordering guarantee between the vnode hook and diff hook is confirmed/documented.
|
Thank you for the review! I'll process your feedback when I've got some time in the next couple of days. |
…onses handling in Fresh runtime.
Partials browser tests failed because full-document responses no longer emitted the Fresh boot script when pages used <Partial> without reliable f-client-nav detection under jsx precompile. Treat any non–partial-request document that renders <Partial> as needing the client runtime. Vite dev Tailwind tests failed because the HMR-only script path never loaded the client entry (and thus CSS side-effect imports). Emit the full inline boot whenever hmrClientEntry is set, and align Link modulepreload headers. Update Tailwind dev server tests to accept Vite 7’s vite-module-id on injected styles in addition to data-vite-dev-id.
|
@bartlomieju Comments added and tests fixed :) |
bartlomieju
changed the title
The partial branch already returns early, so the second if was unreachable for partial responses. Using else-if makes this mutual exclusivity explicit to future readers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Nice work, thank you
|
yahooooo :)) |
3 participants
In many places, Fresh marketing claims the framework ships no JS by default, this was false.
This PR reworks core parts of the framework to check whether JS is needed on the client with these criteria:
f-client-nav)If either of those those criteria are met, JavaScript will be loaded as normal, but if both are false, then the
client-entryscript is omitted from the page.Fixes #3662