Workbox: .mjs is hard to use with SSR frameworks like NextJS

[sheerun]

Hello,

I'm using rc.0 version of this library.

It seems you're publishing only .mjs files for workbox-window, but .mjs files are pretty new development and are hard to use with legacy or even new versions of frameworks like more resent and stable NextJS 7.0.2. Such feamework doesn't support .mjs files in dependencies and complain for builds with "unknown export keyword" errors.

Could you for the time being publish both .mjs and .js versions on npm? It would make it far easier to use this library.

[sheerun]

Another reason is that for performance reasons for testing we use jsdom to render page on server side within node itself, without bundling the code. Currently we use Node 10 (stable!) which doesn't support .mjs files out of the box.

[sheerun]

Here's specific error I'm getting after compiling my Next.js app that requires workdbox-window. If you can suggest other solution than you publishing version with .js files, I'd be fine as well :)

/Users/sheerun/Source/search/node_modules/workbox-window/build/workbox-window.prod.mjs:1

SyntaxError: Unexpected token export
    at new Script (vm.js:84:7)
    at createScript (vm.js:264:10)
    at Object.runInThisContext (vm.js:312:10)
    at Module._compile (internal/modules/cjs/loader.js:684:28)
    at Module._compile (/Users/sheerun/Source/search/node_modules/pirates/lib/index.js:83:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:732:10)
    at Object.newLoader [as .mjs] (/Users/sheerun/Source/search/node_modules/pirates/lib/index.js:88:7)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
    at Function.Module._load (internal/modules/cjs/loader.js:552:3)
    at Function._load (/Users/sheerun/Source/search/node_modules/@sentry/node/src/integrations/console.ts:37:43)
    at Module.require (internal/modules/cjs/loader.js:657:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (/Users/sheerun/Source/search/pages/_app.js:9:1)
    at Module._compile (internal/modules/cjs/loader.js:721:30)
    at Module._compile (/Users/sheerun/Source/search/node_modules/pirates/lib/index.js:83:24)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[sheerun]

Also here's package.json of workbox-window:

  "main": "build/workbox-window.prod.mjs",
  "module": "index.mjs",

While module is indeed es6 module, I think main is supposed to be CJS-compatible script.

[philipwalton]

Sorry you're running into issues with this. We decided to use .mjs for workbox-window because every browser that supports service worker also supports importing modules and works fine with .mjs. Also every JavaScript bundler (Rollup, webpack, parcel) support modules and also works just fine with .mjs.

I wasn't aware that NextJS didn't support .mjs. I've talked to some of my colleagues who work with the NextJS team and we've already filed a PR to fix this, so hopefully that'll get merged soon.

In my opinion, since all browsers and all bundling tools already support the .mjs file extension, and since node is moving forward with .mjs for all its modules, then it doesn't make sense for tools and application frameworks to not support it as well, so they should update. And I'm pretty sure NextJS will update.

In the meantime, you might try:

• Copying the contents of Workbox.mjs into your application source and rename it to Workbox.js (obviously not ideal, but it should work as a temporary solution).
• Create a symlink between a local Workbox.js file and the one in node_modules.
• If NextJS lets you add or modify your webpack config, you could try adding the resolve plugin.

Another reason is that for performance reasons for testing we use jsdom to render page on server side within node itself, without bundling the code. Currently we use Node 10 (stable!) which doesn't support .mjs files out of the box.

For node, if you use esm it should be no problem to work with import statements and .mjs files. Have you tried using that to render pages server side? It's very easy to get started with.

Could you for the time being publish both .mjs and .js versions on npm? It would make it far easier to use this library.

This would be a bit tricky for us with our current publishing infrastructure. Right now we publish the file that corresponds to main in package.json to our CDN when we release new versions. For service worker packages, that's a .js file, but for workbox-window we want to publish the .mjs file to our CDN because using modules is the easiest way for modern browsers to consume that file.

For us to change this convention we'd have to rewrite our deployment infrastructure, and that most likely won't be a high priority for us.

[sheerun]

Thank you for quick response and providing a workaround :)

I'll wait for Next.js to release fix, but other developers might not have as actively maintained frameworks available. Sure it would be nice touch to include CommonJS version for previous generation bundlers, but I won't argue on this. But I think using main is something I could argue:

As far as I am aware the convention of package.json fields is as follows:

• main - CJS compatible entry file
• module - ES6 modules entry file
• browser - Browser compatible entry file

So maybe you might consider changing main in your packages to browser to not confuse developers (and bundlers) who expect this entry to be compatible with CommonJS environment.

[sheerun]

You haven't yet released stable v4 so I think there's still time to change this.

[calinoracation]

I'm finding this a bit challenging as well, the recommended solution of using the following code doesn't work well as we use a strict CSP policy preventing using inline script tags.

<script type="module">
// Important: change the filename to workbox-window.prod.mjs before deploying.
import {Workbox} from 'https://storage.googleapis.com/workbox-cdn/releases/4.0.0-beta.1/workbox-window.dev.mjs'

new Workbox('/sw.js').register();
</script>

And we're excluding node_modules from compilation, so it's been a slog to get our babel/webpack combo to figure out how to allow this even outside of SSR.

[philipwalton]

Sure it would be nice touch to include CommonJS version for previous generation bundlers

@sheerun, I don't know of any JS bundlers that don't support .mjs or import/export statements in the source files. The problem you were encountering with NextJS is they were explicitly not including .mjs files in their webpack config. Webpack can handle them just fine, so it's an easy fix for NextJS (and it looks like the fix has already been merged).

As far as I am aware the convention of package.json fields is as follows...

I agree that putting an .mjs file in main is a deviation from standard node conventions.

This is an artifact of our deployment code (which assumes the file in the main field gets uploaded to our CDN) being written before we had a workbox-window package. We should probably decouple these things now (i.e. not use the main field for CDN related data), but that still won't solve your problem unless we also generate a CJS/UMD version.

I could look into adding that, but I'm not sure if we want to block the v4 release on it. I'm not sure realistically how many people are going to need workbox-window to work with require() in node.

@jeffposnick any thoughts on this?

[philipwalton]

I'm finding this a bit challenging as well, the recommended solution of using the following code doesn't work well as we use a strict CSP policy preventing using inline script tags.

@calinoracation you definitely don't need to use an inline <script> tag, that was just supposed to be an example and not a recommendation. You can put the contents of that inline <script> into its own file and load it via <script type="module" src="path/to/sw-loader.mjs">.

And we're excluding node_modules from compilation, so it's been a slog to get our babel/webpack combo to figure out how to allow this even outside of SSR.

Hmmm, this is a trickier issue because we're definitely not going to publish an ES5 version of any Workbox packages. All browsers that support service worker also support ES modules, so it would be quite wasteful to ship ES5 code to those browsers.

I'd recommend trying to compile your SW-related window code separately from your main bundle. You could then load it via <script type="module">, and it should work fine in all browsers that support service worker.

I'd also strongly recommend you try to change your policy on transpiling node_modules. The number of npm packages being published with only ES2015+ source code is growing every day, and by limiting yourself to ES5-only code, your missing out on these libraries, and you're also likely shipping much larger bundles to your users than you need to be.

[philipwalton]

@sheerun after looking at our deployment code, I don't think it'd be much work at all to generate both a native module and a UMD bundle (which I've confirmed works with require() in node), so I'll try to do that before releasing v4.

[sheerun]

@philipwalton I'm reporting that unfortunately https://github.com/zeit/next.js/pull/6253/files doesn't fix my problem because .mjs files of workbox are ignored by this line:

return /node_modules/.exec(path)

[calinoracation]

Thanks for the UMD bundle change, it will help us a ton!

Does this UMD bundle mean that by importing the package will always result in a production build? I'm not sure the implications or if there's a resolve path to get the dev one that we should be using, but thought I'd mention it in case it wasn't intentional.

I'd also strongly recommend you try to change your policy on transpiling node_modules. The number of npm packages being published with only ES2015+ source code is growing every day, and by limiting yourself to ES5-only code, your missing out on these libraries, and you're also likely shipping much larger bundles to your users than you need to be.

Thanks for that recommendation, we may look into it but it's a pretty risky change for us with such a large codebase and already super long build times and wasn't super feasible for us at this time.

Again thanks so much, we're already benefiting from the changes in v4 and are excited for all of these new features to land!

[philipwalton]

I'm reporting that unfortunately https://github.com/zeit/next.js/pull/6253/files doesn't fix my problem because .mjs files of workbox are ignored by this line:

@sheerun If I understand correctly that line will exclude workbox-window from the babel transpilation, but it won't exclude it from your app. Is that your issue, that it's not being transpiled?

[philipwalton]

Does this UMD bundle mean that by importing the package will always result in a production build? I'm not sure the implications or if there's a resolve path to get the dev one that we should be using, but thought I'd mention it in case it wasn't intentional.

@calinoracation yes, if you run require('workbox-window') in node, you'll get the production bundle. If you want to get the development bundle you'd have to do require('workbox-window/build/workbox-window.dev.umd.js');

[ljharb]

@philipwalton any reason not to put the dev bundle in "main" and the production one in "browser", or, to use process.env.NODE_ENV to switch between the two in a single entry point, as is common?

[philipwalton]

@ljharb can you describe the ideal setup you'd like to see?

We use process.env.NODE_ENV internally in our source for dev-only code, but we strip that out at publish time and create two separate prod and dev builds. We do this because process is not a global in browser environments.

If all bundlers support the browser field of package.json (I think they do?), then I'd be fine with setting it in our package.json, but I'm still unclear as to why you'd want a dev version in node. I'm also hesitant because my understanding is webpack will favor the browser field over the module field if both are present.

[ljharb]

hmm, that webpack thing is a fair point.

However - if you had an extensionless browser pointing to a production UMD .js file and also a production .mjs file, and extensionless "main" pointing to a development CJS .js file and also a development .mjs file, would that work?

[philipwalton]

Ok, after discussing this with the rest of the team, here's what we're going to do. Hopefully this will make it as easy as possible to consume workbox-window in a variety of situations while still allowing those that don't need (or want) to ship ES5 code to be able to do so.

In https://github.com/GoogleChrome/workbox/pull/1899, I've updated the build to include a module version transpiled to ES5. That means we should be able to support all of the following situations:

1) Requiring workbox-window in a node:

// Imports a UMD version with ES5 syntax
const {Workbox} = require('workbox-window');

2) Importing workbox-window via webpack or Rollup in a context where your build does not automatically transpile anything in node_modules:

// Imports the module version with ES5 syntax
import {Workbox} from 'workbox-window';

3) Importing workbox-window via webpack or Rollup in a context where you handle transpiling yourself (or you don't need to transpile to ES5):

// Imports the module version with ES2015+ syntax
import {Workbox} from 'workbox-window/Workbox.mjs';

Does this address all the issues raised here?

[sheerun]

I think this is perfect now

The only issue is one I mentioned earlier, Next.JS supports now .mjs but ignores all .mjs modules in node_modules directory, so it'll take UMD bundle instead of MJS what can increase bundle size.

Probably it would be better to un-ignore just .mjs files from node_modules?

[philipwalton]

@sheerun I'm pretty sure Next will take the .mjs version, it just won't transpile it (which is fine since by default it's already transpiled if you just reference workbox-window).

And we'll likely release these changes today as a new release candidate, so you should be able to test and confirm before we release the final v4.

[sheerun]

It's not a problem with workbox but next.js

[sheerun]

I'm pretty sure Next will take the .mjs version, it just won't transpile it

That's exactly what's happening. Webpack takes .mjs and throws it into the bundle along with "exports xxx" expression which doesn't work when bundled among other js files into single file.

[philipwalton]

Webpack should remove the export statements itself, it shouldn't depend on Babel to do that.

I'm bundling workbox-window on a project right now using webpack, and even if I exclude all node_modules from transpilation in babel-loader, workbox-window still gets included in the bundle, and it doesn't contain any export statements.

We can confirm after the next release candidate is released.

[philipwalton]

Workbox version 4.0.0-rc.1 has been released, so please try it out and let me know if anything is still not working.

[sheerun]

It's working fine but I believe it's because webpack takes umd bundle instead of module (module is set as fallback field https://github.com/zeit/next.js/pull/6256)

Anyway, you can close this as it's not workbox issue anymore, thank you :)

[calinoracation]

This is working fantastically for us now, thanks folks!