Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing a CommonJS node_module in a custom service worker turns it into an ES module #615

Closed
kgroat opened this issue Dec 3, 2023 · 5 comments · Fixed by #629
Closed

Comments

@kgroat
Copy link

kgroat commented Dec 3, 2023

I've discovered that creating a custom service worker using the injectManifest strategy can lead to an issue when importing a node_module that is in the CommonJS format. Specifically, the service worker will end up as an ES module, with an export default, which will result in an error when registered in the browser. The built asset will look something like this:

var t = (l, o) => () => (o || l((o = { exports: {} }).exports, o), o.exports);
var _ = t((r, e) => {
  // Your compiled code here
});
export default _();

I've created this minimal reproduction case here.

I've discovered that adding this to the config gets rid of the default export, but I'm not sure if it has other side-effects:

VitePWA({
  // the rest of your config up here...

  injectManifest: {
    rollupFormat: 'iife'
  }
})
@userquin
Copy link
Member

userquin commented Dec 19, 2023

It seems a problem with esbuild: vitejs/vite#15379

EDIT: SvelteKit now builds the service worker using also iife format.

@Rich-Harris
Copy link

SvelteKit now builds the service worker using also iife format.

FYI we're reverting that change in favour of telling esbuild that the filename ends with .mjs sveltejs/kit#11400

@userquin
Copy link
Member

@Rich-Harris good to know it, thx

How about .mts when using TS?

@Rich-Harris
Copy link

that wouldn't make sense — this is the output filename. it's purely a signal to esbuild that prevents it from erroneously adding an unwanted export declaration

@userquin
Copy link
Member

yeah, forgot it

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 a pull request may close this issue.

3 participants