GoogleChrome / workbox

📦 Workbox: JavaScript libraries for Progressive Web Apps
https://developers.google.com/web/tools/workbox/
MIT License
12.36k stars 817 forks source link

swAssetString.match is not a function when building for production #2723

Closed 0x62 closed 3 years ago

0x62 commented 3 years ago

Library Affected: workbox-webpack-plugin - v6.0.2

Browser & Platform: Build step

Issue or Feature Request Description: Building in production throws error when using the Webpack Plugin

swAssetString.match is not a function

Webpack config:

f (process.env.NODE_ENV === 'production') {
  // Service worker generator, only on production
  plugins.push(new WorkboxWebpackPlugin.InjectManifest({
    swSrc: './src/service-worker.js',
    swDest: 'service-worker.js',
  }));
}

src/service-worker.js:

/* eslint-disable no-restricted-globals,no-underscore-dangle */
import { precacheAndRoute } from 'workbox-precaching';
import { registerRoute } from 'workbox-routing';
import { NetworkFirst } from 'workbox-strategies';
import { ExpirationPlugin } from 'workbox-expiration';

// Precache and route all static assets
precacheAndRoute(self.__WB_MANIFEST);

// Cache image CDN requests
registerRoute(
  /https:\/\/img(-dev)?\.mydomain\.com\/.+/,
  new NetworkFirst({
    cacheName: 'img',
    plugins: [
      new ExpirationPlugin({
        maxAgeSeconds: 60 * 60 * 24 * 28, // 28 days
      }),
    ],
  }),
);
jeffposnick commented 3 years ago

Thanks for letting us know. The relevant code is:

https://github.com/GoogleChrome/workbox/blob/2df5ce58bd74abd8da4ec90e427f0e4431a0cff2/packages/workbox-webpack-plugin/src/inject-manifest.js#L301-L302

At the point in time that that code runs, a child compilation should have already taken place that creates the swDest asset, so I'm not sure what the root cause is at the moment.

0x62 commented 3 years ago

webpack: 4.44.2

I'm using the default Webpack config bundled with Vue, injecting a couple plugins.

vue.config.js:

const MomentLocalesPlugin = require('moment-locales-webpack-plugin');
const SentryWebpackPlugin = require('@sentry/webpack-plugin');
const PreloadWebpackPlugin = require('@vue/preload-webpack-plugin');
const { default: ImageminPlugin } = require('imagemin-webpack-plugin');

const plugins = [
  // Add preload tags to index oage
  new PreloadWebpackPlugin({
    rel: 'preload',
    include: 'asyncChunks',
    as(entry) {
      if (/\.css$/.test(entry)) return 'style';
      if (/\.(woff|woff2|eot|ttf)$/.test(entry)) return 'font';
      return 'script';
    },
  }),
  new MomentLocalesPlugin(),
  new ImageminPlugin({
    disable: process.env.NODE_ENV !== 'production',
  }),
];

// if (process.env.NODE_ENV === 'production') {
//   // Service worker generator, only on production
//   plugins.push(new WorkboxWebpackPlugin.InjectManifest({
//     swSrc: './src/service-worker.js',
//     swDest: 'service-worker.js',
//   }));
// }

// Only upload sources when building on CD
if (process.env.NETLIFY && process.env.SENTRY_AUTH_TOKEN) {
  plugins.push(new SentryWebpackPlugin({
    include: './dist/js',
    release: process.env.COMMIT_REF,
    ignoreFile: '.sentrycliignore',
    ignore: [
      'node_modules',
      'vue.config.js',
      'apollo.config.js',
      'babel.config.js',
    ],
    configFile: 'sentry.properties',
  }));
}

module.exports = {
  lintOnSave: true,
  transpileDependencies: ['vuetify', 'vue-clamp', 'resize-detector'],
  pluginOptions: {
    apollo: {
      lintGQL: false,
    },
  },
  configureWebpack: {
    plugins,
  },
  devServer: {
    host: 'localhost',
  },
};
jeffposnick commented 3 years ago

Hello!

Sorry for the delay in getting back to you.

I'm able to reproduce the issue following your configuration, but only if I include the ImageminPlugin in your build configuration. If I omit ImageminPlugin then the compilation succeeds as expected.

The underlying error seems to occur because ImageminPlugin processes the service worker asset created by InjectManifest in the child compilation, even though it's obviously not an image. By the time ImageminPlugin has finished, the swAsset.source.source() ends up returning a Uint8Array containing the corresponding bytes, not a string representation of the service worker as expected. That's why .match() is not a function.

While it's possible to code around this with something like:

let swAssetString = swAsset.source.source();
if (typeof swAssetString !== 'string' && 'buffer' in swAssetString) {
  swAssetString = Buffer.from(swAssetString.buffer).toString();
}

I think the cleanest approach is to configure ImageminPlugin so that it does not attempt to process the swDest asset. This is the only time I've ever seen this sort of interaction due to another plugin, so I would rather not modify InjectManifest to account for unexpected behavior triggered by something else.

It might also be worthwhile exploring why, by default, the ImageminPlugin would make changes to a .js asset created by another plugin in a child compilation, if you feel like pursuing that with the plugin's maintainer.

Given all that, I'm going to close this issue, but we can reopen if it looks like this is something we really do need to work around.