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

Breaking change from 6.4.2 to 6.5.0 #3033

Closed
vogelsgesang opened this issue Feb 24, 2022 · 5 comments · Fixed by #3036
Closed

Breaking change from 6.4.2 to 6.5.0 #3033

vogelsgesang opened this issue Feb 24, 2022 · 5 comments · Fixed by #3036
Labels
Bug An issue with our existing, production codebase. workbox-webpack-plugin

Comments

@vogelsgesang
Copy link

vogelsgesang commented Feb 24, 2022

Library Affected:
workbox-webpack-plugin

Browser & Platform:
Node v17.5.0

Issue or Feature Request Description:

The relevant webpack/prod.config.ts:

import {merge} from "webpack-merge";
import commonConfig from "./common.config";
import WorkboxPlugin from "workbox-webpack-plugin";

const prodConfig = merge(commonConfig, {
    mode: "production",
    devtool: "source-map",
    plugins: [
        new WorkboxPlugin.GenerateSW({
            sourcemap: false,
            skipWaiting: true,
            clientsClaim: true,
            ignoreURLParametersMatching: [/./],
            include: [/^bundle.js$/, /^index.html$/],
        }),
    ],
});

export default prodConfig;

Worked without problems in 6.4.2, started failing in 6.5.0

[webpack-cli] Failed to load '/Users/avogelsgesang/query-graphs/standalone-app/webpack/prod.config.ts' config
[webpack-cli] TypeError: Cannot read properties of undefined (reading 'GenerateSW')
    at Object.<anonymous> (/Users/avogelsgesang/query-graphs/standalone-app/webpack/prod.config.ts:9:27)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Module.m._compile (/Users/avogelsgesang/query-graphs/node_modules/ts-node/src/index.ts:1459:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1151:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/avogelsgesang/query-graphs/node_modules/ts-node/src/index.ts:1462:12)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at WebpackCLI.tryRequireThenImport (/Users/avogelsgesang/query-graphs/node_modules/webpack-cli/lib/webpack-cli.js:244:16)

Link to failed build on Github Actions: https://github.com/vogelsgesang/query-graphs/runs/5312503087?check_suite_focus=true

vogelsgesang added a commit to vogelsgesang/query-graphs that referenced this issue Feb 24, 2022
@jeffposnick
Copy link
Contributor

Thanks for bringing this to our attention—we'll investigate.

CC: @tropicadri

@jeffposnick jeffposnick added Bug An issue with our existing, production codebase. workbox-webpack-plugin labels Feb 24, 2022
@jeffposnick
Copy link
Contributor

The compatibility with existing code should be fixed in #3036. Switching to

import {GenerateSW} from 'workbox-webpack-plugin';

should also resolve the issue, if you want a fix right now.

(We currently don't have a default export in the workbox-build module, and that's been the case since Workbox v6.2.0, so I'd like to remove it from workbox-webpack-plugin as part of the v7 major semver release.)

@vogelsgesang
Copy link
Author

Thanks for that workaround! I agree that not having a default export is a nicer coding style and I will gladly update my code to use the named import

The documentation for workbox (https://developers.google.com/web/tools/workbox/guides/generate-service-worker/webpack) still recommends

// Inside of webpack.config.js:
const WorkboxPlugin = require('workbox-webpack-plugin');

module.exports = {
  // Other webpack config...

  plugins: [
    // Other plugins...

    new WorkboxPlugin.GenerateSW()
  ]
};

So I guess my code style above is rather common. I admittedly didn't really think about it, but just followed the pattern provided in the docs. I can imagine, many other users of workbox did the same...

If you are planning to remove the default export in the future, you might already want to update your docs today and recommend the future proof named import instead of the to-be-removed default import. That might save a few persons (like myself) an unnecessary breakage when upgrading to v7 in the future

@jeffposnick
Copy link
Contributor

We're also about to migrate our docs over to developer.chrome.com. The new version of the most relevant doc uses the named export, so I think we're good:

https://developer.chrome.com/docs/workbox/modules/workbox-webpack-plugin/

(I'll double-check to make sure there aren't any other lingering references to the default export.)

@jeffposnick
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An issue with our existing, production codebase. workbox-webpack-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants