fix: build service workers in IIFE format

[meticoeus]

Fixes #6654 for imported graphql package in service worker code.

The documented service worker registration code uses classic format in a production build but SW is built in es module format. Vite produces an export statement that breaks classic mode registration when at least some external packages are imported into service worker code (nostr-tools and graphql reported in linked issue). I've changed the vite build option for the service worker to use 'iife' format instead of 'es'. I've locally tested dev and production builds and things appear to be working normally. I'm not sure what further testing is needed for this PR to be acceptable.

Reference: SW es module support is still missing in several major browsers including Firefox and Safari. source.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check
  • I could not get the test suite to pass on my machine with or without the change. Random tests would fail, usually browser tests would timeout. Node: 20.9.0 OS: Pop!_OS 22.04 LTS x86_64.

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8a7ac4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benmccann]

I discussed this PR with the rest of the team and there was some concern that Vite shouldn't be including an export in the ESM version and this is papering over some issue. Could you see if it works with es and preserveEntrySignatures: false?

[meticoeus]

I changed lib.formats back to ['es'] and added rollupOptions.preserveEntrySignatures: false and it still adds the export default f() to the end of the output. It seems importing any external module is causing it to add this for some reason.

I'm not sure why you would build as an ES module but then register it as a classic JS file though? Shouldn't it be built as an IIFE format anyway until browser support is ready for service worker ES Modules? It seems like it is just working accidentally in the case that you haven't imported anything.

[benmccann]

An ES module and classic JS file should be indistinguishable if there are no exports. I think the question is why is there an export because there shouldn't be one. The IIFE works and maybe we'll end up going with it out of practicality, but it does make the output infinitesimally larger and slower and shouldn't really be necessary to wrap everything in a function that way.

[benmccann]

oh, it looks like there's a merge conflict on this PR that will need to be addressed as well

[benmccann]

once the CI turns green again we can merge this as a workaround for vitejs/vite#15379

[benmccann]

I didn't think about it, but we rely on ESM for dev mode. Still I think this code path is only called during build, so we're probably safe with this change still

[Rich-Harris]

huh? it's the opposite, the service worker is only built in prod mode?

looking at vitejs/vite#15379 (comment), it seems that we may be zeroing in on a solution. if we're close to one, i'd much rather adopt that rather than bloat out everyone's service workers unnecessarily, even if the overhead is relatively small

[Rich-Harris]

i have a solution locally, will revert this PR and open a new one

[benmccann]

huh? it's the opposite, the service worker is only built in prod mode?

Yes, that's what I meant to write. I updated my comment above to make sense now