Skip to content

fix: register files generated by Vite plugins in static file cache#3534

Merged
bartlomieju merged 6 commits into
freshframework:mainfrom
fry69:fry69/fix-vite-plugin-pwa
Mar 29, 2026
Merged

fix: register files generated by Vite plugins in static file cache#3534
bartlomieju merged 6 commits into
freshframework:mainfrom
fry69:fry69/fix-vite-plugin-pwa

Conversation

@fry69
Copy link
Copy Markdown
Contributor

@fry69 fry69 commented Oct 7, 2025

Fixes #3487

This includes

  • test case (first commit)
  • fix (second commit)
  • dev server test, ignored (3rd/4th commit)

After analyzing the problem for a while for the Vite's dev server and how Fresh handles this, the conclusion seems to be that this is an edge case. So it will not work with vite dev, only in "production" where actual generated fixed files get served. Detailed reasoning on the bottom.

Generated pull request message below with details:


Problem

Files generated by Vite plugins (e.g., vite-plugin-pwa service workers, manifests) were created in _fresh/client/ but returned 404 when accessed via HTTP. Fresh's static file serving only registered files from the Vite manifest and public directory, missing plugin-generated files.

Solution

Added a directory scan in server_snapshot.ts that walks _fresh/client/ after the client build completes. Any files not already registered (excluding .vite/ directory) are added to the static file cache, making them accessible via HTTP.

Changes

  • Modified packages/plugin-vite/src/plugins/server_snapshot.ts to scan client output directory
  • Added test fixture for vite-plugin-pwa integration
  • Added tests verifying service worker and manifest files are accessible
  • Added vite-plugin-pwa to dependencies for testing

Testing

All 117 tests pass. New tests verify:

  • Files are generated by vite-plugin-pwa during build
  • Files are accessible via HTTP with correct content types
  • Service worker contains expected Workbox code
  • Manifest JSON structure is valid

Analysis: Post-Hook Approach for Dev Server

Current Architecture

Fresh currently registers its middleware BEFORE Vite's built-in middlewares (pre-middleware). This means:

  • Fresh intercepts ALL requests first
  • Only passes specific patterns to Vite (static files, Vite internal URLs, module graph entries)
  • Everything else goes to Fresh SSR

The Problem

This aggressive interception blocks Vite plugins from serving their own files. My attempt to forward 404s didn't work because once Fresh responds, the middleware chain ends - next() isn't called in the response path.

The Post-Hook Solution

NOT RECOMMENDED for the following reasons:

  1. Architectural Mismatch: Fresh's design philosophy is to control routing aggressively. Using post-hook would fundamentally change this to "Vite first, Fresh fallback."

  2. Hidden Dependencies: The current code has implicit assumptions about execution order (checking module graph, static files early). Moving to post-hook could break these.

  3. Unintended Side Effects: Other Vite plugins might interfere with Fresh routes before Fresh sees them.

Better Alternative: Targeted Pass-Through

Instead of post-hook, I recommend a whitelist approach - explicitly pass known file patterns to Vite BEFORE trying Fresh SSR:

// Known patterns that Vite plugins might serve
const PLUGIN_FILE_PATTERNS = /\.(webmanifest|wasm)$|^\/sw\.js$|^\/workbox-.*\.js$|^\/registerSW\.js$/;

if (PLUGIN_FILE_PATTERNS.test(url.pathname)) {
  // Let Vite try first, but don't end the chain
  // This requires a different approach...
}

However, this still has the same problem: we can't "try Vite and fallback to Fresh" without post-hook.

Recommended Solution: Do Nothing for Dev Mode

After careful consideration, I believe:

  1. Production is fixed ✅ - Files are accessible after build (our main fix works)
  2. Dev mode is edge case - vite-plugin-pwa is primarily for production PWAs
  3. Config workaround exists - Users can put PWA files in static/ for dev testing
  4. Risk vs Reward - Post-hook changes are too risky for an edge case

The vite-plugin-pwa documentation typically shows dev mode with devOptions.enabled: true as optional, mainly for testing. Most development doesn't need the actual service worker.

Final Recommendation

Skip the dev server fix for now:

  • Keep the test to document the known limitation
  • Mark it as ignored with a comment explaining it's a known issue
  • Focus on the production fix which is what users actually need
  • Consider post-hook in a future major version if Fresh's architecture evolves

@fry69 fry69 marked this pull request as draft October 7, 2025 10:14
@fry69 fry69 marked this pull request as ready for review October 7, 2025 10:49
@bartlomieju
Copy link
Copy Markdown
Contributor

Nice fix — the core change in server_snapshot.ts is clean and solves a real problem. A few things I noticed:

  1. hash: null means no cache-busting — registered files get hash: null. Other static files have content hashes for cache headers. These plugin-generated files will likely be served without proper cache-busting, which is particularly bad for service workers — a stale sw.js can cause persistent caching issues. Worth checking what cache headers are sent for hash: null files.

  2. registeredPaths comparison may be wrong — existing staticFiles entries use pathname which might include a leading / or use different path separators than path.relative() returns. If the formats don't match, files could be double-registered. Worth verifying the pathname format is consistent.

  3. Lockfile churn is massive — 2025 of the 2205 added lines are deno.lock from adding vite-plugin-pwa as a dev dependency, pulling in the entire workbox ecosystem, babel plugins, etc. Consider whether a lighter approach (manually creating test files in _fresh/client/ rather than running an actual PWA build) would test the same code path without the dependency.

  4. Ignored dev server test — the dev_server_test.ts addition is permanently ignore: true with a long comment explaining why it can't work. This is dead code that will likely never be un-ignored given the architectural constraints. Either track it as an issue or drop it.

  5. Brittle assertionexpect(swContent).toContain("workbox") ties the test to workbox internals. If vite-plugin-pwa changes its output, this breaks for reasons unrelated to Fresh. The status + content-type checks already cover the important behavior.

@bartlomieju bartlomieju changed the title fix(vite): register files generated by Vite plugins in static file cache fix: register files generated by Vite plugins in static file cache Mar 29, 2026
bartlomieju and others added 2 commits March 29, 2026 21:56
Keep main's new tests and append PR's vite-plugin-pwa tests.
Take main's deno.lock (will be regenerated).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Normalize registered pathnames when comparing (strip leading /) to
  prevent double-registration of files with inconsistent path formats
- Remove brittle workbox assertion from build test
- Remove permanently-ignored dev server test (dead code)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@bartlomieju bartlomieju enabled auto-merge (squash) March 29, 2026 20:04
@bartlomieju bartlomieju merged commit 8b0d42b into freshframework:main Mar 29, 2026
7 checks passed
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.

[vite-plugin-pwa] Generated sw.js didn't included in staticFiles.

2 participants