vite-pwa / vite-plugin-pwa

Zero-config PWA for Vite
https://vite-pwa-org.netlify.app/
MIT License
3.1k stars 204 forks source link

"injectManifest" strategy producing a service worker that fails in production #631

Open antti5 opened 8 months ago

antti5 commented 8 months ago

I recently migrated my React application from CRA to Vite, using Vite 5.0.10 and for the service worker vite-plugin-pwa version 0.17.4.

I'm using my own simple service worker, and I have my own code that handles the service worker registration and update prompts. So I'm trying to use the plugin to simply build the service worker.

My vite.config.js has the following for the plugin:

VitePWA({
   strategies: 'injectManifest',
   srcDir: 'src',
   filename: 'service-worker.js',
   // Don't inject service worker registration code
   injectRegister: null,
   devOptions: {
      enabled: true,
      type: 'module',
      // Include index.html in __WB_MANIFEST in development
      navigateFallback: 'index.html'
   }
})

When I run my app in development, the service worker works perfectly.

However, when I try "npm run preview" to test my build, or if deploy my application, the service worker fails upon being loaded due to trying to access process (Uncaught ReferenceError: process is not defined).

When I check the service worker file that is produced by the plugin in the build directory, I can indeed see several references to process.env, process.platform, etc.

I've tried to follow the plugin documentation as closely as I can, and I've run out of ideas. The service worker is very simple and only has a minimal set of import, all from Workbox:

import { precacheAndRoute, createHandlerBoundToURL } from 'workbox-precaching';
import { clientsClaim } from 'workbox-core';
import { registerRoute } from 'workbox-routing';
import { CacheFirst } from 'workbox-strategies';
import { ExpirationPlugin } from 'workbox-expiration';
userquin commented 8 months ago

@antti5 try adding this entry to vite define:

define: {
    'process.env.NODE_ENV': process.env.NODE_ENV === 'production' 
      ? '"production"'
      : '"development"'
  }
antti5 commented 8 months ago

@userquin I tried this, but it does not change the built service worker in any way.

userquin commented 8 months ago

can you provide a minimal reproduction?

antti5 commented 8 months ago

@userquin I can, but it will take a little while... Maybe until tomorrow.

It might be a good idea anyway, since it might help me pinpointing the problem myself.

antti5 commented 8 months ago

@userquin So as it happens, I DID find the culprit when I built my minimal reproduction.

In my service worker, I'm using preval.macro to write the build time of the service worker to console. This is sometimes helpful when debugging the service worker itself.

import preval from 'preval.macro';

const buildTime = preval`module.exports = new Intl.DateTimeFormat('en-GB', { year: 'numeric', month: 'short', day: 'numeric', hour: 'numeric', minute: 'numeric', second: 'numeric', timeZone: 'UTC', timeZoneName: 'short', hour12: false }).format(new Date())`;
console.info('Service worker build time:', buildTime);

As soon as I added this to my minimal reproduction, the original problem surfaced. The service worker produced by the build grew greatly in size -- from about 50 kB to about 300 kB -- including a lot of code that accesses process.

I did not expect this because preval.macro is something that works at build time only, and should only produce that one string value in my service worker code above.

I can drop preval.macro as an interim solution, but I think this may be a bug in the PWA plugin? Or is there something that I can add to my vite.config.js to make this work?

userquin commented 8 months ago

You can add rollupFormat iife in the injectManifest option, check the pr to add a new vite build: it is a bug with esbuild

userquin commented 8 months ago

@antti5 can you shared small repro? I can test it with #629

userquin commented 8 months ago

Why do you need that import? You can use SW_BUILD_DATE in Vite define entry + declare const SW_BUILD_DATE: string in some dts

imagen

imagen

userquin commented 8 months ago

anyway, the build time is wrong in your code, you need to provide it in the build

antti5 commented 8 months ago

I have the preval.macro simply because I had it in my service worker from before I was using Vite.

I just last week migrated my React application from CRA to Vite, and I have no prior experience with Vite. Most of the migration was very smooth, like literally a 2-hour job to have everything working in development. It was only with the first deployment when I ran into this problem with the service worker.

Here's a repro with a minimal service worker uses preval.macro causing the service worker evaluation to fail: https://github.com/antti5/vite_sw_repro.git

To test, just run:

npm install npm run build npm run preview

...and in the console you should see the error about process not being found. You may have to reload once.

Also with the build there are externalization warnings that dissappear if preval.macro is removed from the service worker.

userquin commented 8 months ago

preval.macro is CJS only, babel-plugin-preval is CJS only (with wrong package exports), the problem is about using babel-plugin-macros it seems a node only package (using path and process), this shouldn't work at runtime

userquin commented 8 months ago

OK, I get it working, you also need to include macros plugin in injectManifest.plugins PWA option using both, es or iife formats:

injectManifest: {
  rollupFormat: 'iife', // or rollupFormat: 'es' or just removing the option
  plugins: [macros()]
}

imagen

userquin commented 8 months ago

imagen