vite-plugin-pwa: `injectManifest` doesn't include enough plugins

See #229 for an example of this - this plugin has a whitelist of allowed plugins when using injectManifest:

https://github.com/antfu/vite-plugin-pwa/blob/main/src/modules.ts#L41-L50

That issue needed at least vite:asset for it to work, possibly more.

I actually remember now debugging this about a year ago before abandoning this plugin and rolling my own to support my service worker setup. Your plugin is way, way, way better than mine 😸 so I wanna get it working.

This works for me for the moment:

  const includedPluginNames = [
+   "commonjs",
+   "vite:asset",
+   "vite:json",
    "alias",
    "vite:resolve",
    "vite:esbuild",
    "replace",
    "vite:define",
    "rollup-plugin-dynamic-import-variables",
    "vite:esbuild-transpile",
    "vite:terser"
  ];

Honestly I am confused about this approach - I’m assuming there was a good reason to whitelist the plugins at some point, but I’m not seeing it. I’m also not that familiar with vite and rollup, so I don’t really know the implications. In my hand-rolled plugin I just blacklisted my own plugin, and that worked fine.

I see a few options really to solve this:

  1. Just filter out vite-plugin-pwa*
  2. Option 1 plus filtering out any known problem plugins from vite’s default set.
  3. Add a config option to easily add back plugins by name when needed.

I’d probably order them the same way as far as my preference of solution, I just don’t know if the first one is feasible…although it’s the easiest too.

In general though, users can use whatever plugins they want, and the whitelist feels like a pretty hostile approach - if you rely on any particular plugin for your service worker than you’re out of luck here.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 31 (15 by maintainers)

Commits related to this issue

Most upvoted comments

@o-alexandrov @gund @keithlayne We’re switching injetManifest to build the service worker using a new custom Vite build, adding a new option to injectManifest to configure plugins?: Plugin | Plugin[], you only need to configure plugins in vite config file and pwa options, I’ll ping you again when PR in the repo, I need to do some tests.

We’re also including original define entries in the new build configuration.