kit icon indicating copy to clipboard operation
kit copied to clipboard

[bug] service workers built in wrong format

Open Giszmo opened this issue 2 years ago • 7 comments

Describe the bug

In src/service-worker.ts I have the sole line:

import { relayPool } from 'nostr-tools'

but the bundle (build/service-worker.js) contains:

export default Fl();

The same happens if I import nostr from 'nostr-tools' (or probably anything other nostr-tools.

My bundle is free of exports when importing Dexie from 'dexie' or a bunch of other imports.

I tried to figure out in the vite discord if this "transient" export might be a feature of sorts but I'm more and more convinced it is a bug in the bundler.

Reproduction

https://github.com/Giszmo/tmpForBugReport is a blank svelteKit project with the added dependency nostr-tools and src/service-worker.ts` consisting of only an import from that dependency.

Logs

$ npm run build && npm run preview 
... (no errors)

Chromium dev console:


Uncaught SyntaxError: Unexpected token 'export' (at service-worker.js:6207:1)


System Info

$ npx envinfo --system --binaries --browsers --npmPackages "{svelte,@sveltejs/*,vite}"
Need to install the following packages:
  [email protected]
Ok to proceed? (y) 

  System:
    OS: Linux 5.10 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
    Memory: 17.28 GB / 31.33 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 18.6.0 - ~/.nvm/versions/node/v18.6.0/bin/node
    npm: 8.13.2 - ~/.nvm/versions/node/v18.6.0/bin/npm
  Browsers:
    Chromium: 103.0.5060.53
    Firefox: 96.0.3
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.71 
    @sveltejs/kit: next => 1.0.0-next.476 
    svelte: ^3.46.0 => 3.50.0 
    vite: ^3.1.0 => 3.1.0 

Severity

blocking an upgrade

Additional Information

I need to move nostr stuff into a service-worker and this bug is blocking this necessary upgrade. On vite discord I couldn't find how to prevent this "transitive" export.

Giszmo avatar Sep 07 '22 19:09 Giszmo

SvelteKit just uses Vite for bundling, so you should really file this issue in the Vite issue tracker: https://github.com/sveltejs/kit#bug-reporting

benmccann avatar Sep 08 '22 04:09 benmccann

Seems the issue is not bundling, but service worker registration: https://github.com/vitejs/vite/issues/10035#issuecomment-1240818293

https://github.com/sveltejs/kit/blob/177a5a9f8219f3f9633d8f8dc879f8472f74d6a2/packages/kit/src/runtime/server/page/render.js#L216

benmccann avatar Sep 08 '22 19:09 benmccann

@benmccann was my PR merged about including the scope? the problem here is that you cannot use type module, will work only in chrome.

https://github.com/sveltejs/kit/pull/2281/files#diff-7803acf5d8fee14eba98e6e587035ef1f0bb35ddcccb9dd78b5ccf968beb18f0R77

userquin avatar Sep 08 '22 19:09 userquin

@userquin was able to track it down to the readable-stream polyfill using global when service workers require self and patched it on his local test with the following to do a find and replace in the library:

unknown

benmccann avatar Sep 08 '22 21:09 benmccann

@benmccann filed issue https://github.com/nodejs/readable-stream/issues/487

userquin avatar Sep 08 '22 21:09 userquin

sw should be built using iife format, using pwa plugin also generates the export default in the sw, we use es format with exports: 'none' and inlineDynamicImports: true in the Rollup build, and seems Rollup is unable to generate the correct sw (it seems the package is using some ancient package).

Rollup build in pwa plugin: https://github.com/antfu/vite-plugin-pwa/blob/main/src/modules.ts#L108

To fix it, I also patch the pwa plugin here: https://github.com/userquin/nostroid-with-pwa-plugin/blob/main/scripts/patch.ts#L40

Also requested a new feature in the pwa repo to allow change the format.

userquin avatar Sep 08 '22 23:09 userquin

Summary: @Gizmo is using nostr-tools package in the service worker, I've created this repo https://github.com/userquin/nostroid-with-pwa-plugin to try import the package:

  • I enable dev options in vite-plugin-pwa: the plugin will delegate the build to Vite registering the service worker with type module (enabled in devOptions.type option in pwa plugin): the service worker fails when installing, readable-stream with the global error
  • once readable-stream patched, the service worker with type module works in dev
  • next step was build the service worker and test it with vite preview: the service worker on build was generated with export default; applying the patch to pwa plugin switching Rollup build format from es to iife works

userquin avatar Sep 09 '22 08:09 userquin

I am also seeing this bug when importing graphql and graphql-request to build a graphql server in the service worker (serving iDB data). I am migrating from a pre-release build where the SW worked with svelte-kit build to now vite build which introduced the issue.

I've switched to vite-pwa-plugin for now but hope this gets fixed at some point as the out of the box experience was much nicer to get things working without using workbox.

For those who similarly need a workaround I got offline support with a custom service worker working using this example.

meticoeus avatar Feb 01 '23 21:02 meticoeus

Changing /packages/kit/src/exports/vite/build/build_service_worker.js to produce an iife format build seems to fix this bug. I've created a PR but I'm not sure if this requires any kind of testing to be added or modified.

meticoeus avatar Nov 27 '23 21:11 meticoeus

Yeah, looks like ESM only works in Chrome: https://caniuse.com/mdn-api_serviceworker_ecmascript_modules

benmccann avatar Nov 28 '23 23:11 benmccann

@meticoeus can you share a project that reproduces this with graphql? The repo in the original issue description has been deleted

benmccann avatar Nov 30 '23 16:11 benmccann

@meticoeus can you share a project that reproduces this with graphql? The repo in the original issue description has been deleted

repo: https://github.com/meticoeus/sveltekit-sw-export-bug-repro

I've narrowed down a reproduce-able example that causes the bug when certain lodash-es functions are imported into the service worker. I tested several other packages I use plus nostr but none of the others caused the issue in the lastest version of SvelteKit either all together or individually. Testing my actual project with lodash-es removed from the service worker fixes the issue as well. I originally set up a full mini graphql server in the service worker using idb to store the Counter data but this non trivial usage of the code still didn't cause the issue so I left it out after finding lodash-es seemed to be the culprit (this can be seen in the first commit to the linked repo).

Packages tested: dataloader graphql graphql-request graphql-scalars idb lodash-es nostr

lodash isFunction didn't cause an issue but map and orderBy did. I don't actually use map but I was looking for other non trivial functions to import to test the issue.

All of the test code is in /src/service-worker/activateEvent.ts. I just (un)commented the import and the usage of the import to see if it caused an issue or not. Only the 2 lodash functions are active to demonstrate the issue but I left the others commented out for quick verification testing.

I also noticed in my testing that when it adds export, it also changes the output quite a bit by wrapping a lot of the code, nut not all, into the function that is called in the export default f() line at the end. This is pretty clearly something vite is doing for some reason but I haven't attempted to dig into vite's source code to figure out what yet.

meticoeus avatar Dec 04 '23 22:12 meticoeus

Thank you for that! I filed a bug upstream: https://github.com/vitejs/vite/issues/15379

benmccann avatar Dec 18 '23 20:12 benmccann

will close this with https://github.com/sveltejs/kit/pull/11129 as a workaround

benmccann avatar Dec 18 '23 23:12 benmccann

Glad to see the root cause is getting looked into. Thanks!

meticoeus avatar Dec 19 '23 20:12 meticoeus

just FYI, we ended up swapping out your workaround for another one: https://github.com/sveltejs/kit/pull/11400

benmccann avatar Dec 19 '23 20:12 benmccann