fix: improve Deno module resolution in Vite plugin#3734
Merged
Conversation
Replaces Fresh's custom 400-line deno.ts plugin with @deno/vite-plugin for Deno module resolution/loading, keeping only Fresh-specific transforms: - createFreshOnLoad: Preact JSX babel transform for client-side code - freshSsrTransform: SSR precompile JSX re-transform plugin - @deno/loader updated to ^0.4.0 (aligned with @deno/vite-plugin) Requires unpublished @deno/vite-plugin changes: - Per-environment loaders (environments option) - onLoad hook for post-load transforms - https:// import rewriting (deno-http:: prefix) - ./resolver export for parseDenoSpecifier/isDenoSpecifier Currently uses local path imports (../../../../deno-vite-plugin/dist/) which must be replaced with npm:@deno/vite-plugin once published. Tested: Fresh www site works in both dev and production build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Added resolveId hook to freshSsrTransform to resolve bare specifiers (like @marvinh-test/fresh-island) from Fresh's virtual modules through @deno/loader. Also added eager Workspace creation via module-level import to ensure correct deno.json discovery before Vite changes CWD. Still blocked: @deno/loader Workspace created from freshSsrTransform cannot resolve bare specifiers from the workspace import map. The same code works when run directly via deno eval from the same CWD. May be a @deno/loader instance isolation issue (different WASM instances from different import paths don't share config discovery). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause found: the server-snapshot virtual module generates import statements with bare specifiers (e.g. "@marvinh-test/fresh-island") for remote islands. When Vite's SSR module runner evaluates this code, it bypasses our resolveId hook for npm-scoped bare specifiers. The fix: server-snapshot.ts should pre-resolve island specifiers to their full paths (using this.resolve() in the load hook) before generating the virtual module code. This way the import in the generated code uses a resolved path, not a bare specifier. Also confirmed: @deno/loader Workspace correctly discovers workspace configs from any CWD — the config discovery is NOT the issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The fresh-island:: and fresh-client-island:: resolveId handlers were returning bare specifiers (e.g. "@marvinh-test/fresh-island") which Vite couldn't resolve through the deno plugin. Now they use this.resolve() to fully resolve through the plugin pipeline. All 9 demo CJS/library compat routes now work: - it_works, ioredis, pg, redis, supabase_pg, qs, stripe, commonjs, radix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dev server tests: - Add commonjs route test (pure CJS module.exports) - Add maxmind CJS test Production build test: - Add CJS packages production regression test covering commonjs, qs, stripe, pg, ioredis, redis, supabase_pg — verifying that CJS packages that work in dev also work in production builds (#3653) Closes #3653 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Fix isDev closure bug: createFreshOnLoad now takes a getter (() => isDev) instead of the value, so it reads isDev at call time after the config() hook has set it correctly. 2. Replace local path imports (../../../../deno-vite-plugin/dist/) with npm:@deno/vite-plugin and add it to deno.json imports. 3. Add debug logging to SSR transform error handler instead of silently swallowing errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The package is now published with the onLoad callback, DenoPluginOptions type, and /resolver subpath export that this PR depends on. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ugin # Conflicts: # deno.lock # packages/plugin-vite/tests/build_test.ts # packages/plugin-vite/tests/dev_server_test.ts
bartlomieju
changed the title
The @std/async@1.2.0 breaks @astral/astral@0.5.5 by rejecting Infinity as maxAttempts in retry(). Keep the lockfile close to main's resolved versions to avoid pulling in the incompatible version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes compatibility with @std/async@1.2.0 which rejects Infinity as maxAttempts in retry(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ugin # Conflicts: # deno.lock # packages/plugin-vite/tests/build_test.ts
This was referenced Mar 30, 2026
Draft
build-id/mod.ts uses Deno.env.get() which throws in the browser. The old deno.ts plugin transformed this away, but @deno/vite-plugin serves it to the client untransformed. Also: - Add exclude option to @deno/vite-plugin config to skip Fresh virtual module IDs (fresh-island::, fresh-client-island::, fresh:) - Skip Fresh virtual module prefixes in freshSsrTransform resolveId Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…o.ts Instead of replacing Fresh's deno.ts with @deno/vite-plugin entirely, import only the resolveDeno function from @deno/vite-plugin/resolver for better npm subpath resolution (e.g. npm:preact@^10/jsx-runtime). Keep Fresh's existing load/transform pipeline which correctly: - Uses \0deno:: virtual modules that esbuild skips - Applies Babel JSX transforms in the load hook - Handles SSR precompile in the transform hook Changes to deno.ts: - Import resolveDeno from @deno/vite-plugin/resolver for npm: specifiers - Pass importer file URL to loader.resolve() for workspace import maps - Fix operator precedence in babelTransform (.jsx files in SSR) Removed: - deno_transforms.ts (onLoad callback, freshSsrTransform plugin) - @deno/vite-plugin as a Vite plugin (only used as resolver library) - CJS production build test (pre-existing limitation) - build-id Deno guard (not needed — buildIdPlugin virtualizes it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
Note on CJS module compatibilityThe CJS production build test was removed because ioredis fails with Root causeFresh's
How to fixThis could be addressed by PR #3697 which improves the CJS-to-ESM Babel transform, or by enhancing In dev mode, CJS packages work fine because Vite's dev server handles module loading differently (through its own dependency optimizer). |
bartlomieju
changed the title
@deno/loader already handles npm subpath resolution correctly (e.g. npm:preact@^10/jsx-runtime). No need for @deno/vite-plugin as an intermediary — the three fixes in deno.ts are self-contained. Co-Authored-By: Claude Opus 4.6 (1M context) <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
1 participant
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
Three targeted fixes to Fresh's Vite plugin for better module resolution, plus new test coverage.
1. Workspace import map resolution
Pass the importer's file URL to
loader.resolve()so bare specifiers from workspace member packages resolve through the correct import map — not just the rootdeno.json.Fixes #3654
2. Remote island resolution
Use
this.resolve(spec)in thefresh-island::andfresh-client-island::resolveId handlers (server_snapshot.ts/client_snapshot.ts) so JSR remote islands get proper\0deno::specifiers. Without this, remote islands resolved to bare specifiers that Vite couldn't load.Fixes #3437
3. Babel JSX operator precedence fix
!ssr && id.endsWith(".tsx") || id.endsWith(".jsx")→!ssr && (id.endsWith(".tsx") || id.endsWith(".jsx")). The missing parentheses caused.jsxfiles to incorrectly get Preact JSX transforms during SSR.Other changes
@deno/loaderfrom ^0.3.10 to ^0.4.0 (needed for workspace resolution fix)@astral/astralfrom ^0.5.5 to ^0.5.6 (fixes@std/async@1.2.0compat)Test plan
deno checkpassesNote on CJS issues
This PR does not fix CJS module compatibility issues in production builds. The new CJS tests verify dev server behavior (which uses Vite's built-in optimizer). Production CJS fixes need improvements to the CJS-to-ESM Babel transform in
commonjs.ts— see PR #3697.🤖 Generated with Claude Code