Skip to content

refractor service worker #4123

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

Merged
merged 19 commits into from
Dec 3, 2020
Merged

Conversation

chenxsan
Copy link
Member

@chenxsan chenxsan commented Nov 3, 2020

We have webpack.ssg.js to generate those static html pages, but the current service worker is not taking full advantage of them.

For example, when we open view-source:https://webpack.js.org/guides/, we expect dist/guides/index.html to be returned. Instead /app-shell/index.html is returned at the moment.

This pull request refractors the service worker replacing GenerateSW with InjectManifest which gives us more control.

  • add some e2e tests

@vercel
Copy link

vercel bot commented Nov 3, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/webpack-docs/webpack-js-org/jz89vbgza
✅ Preview: https://webpack-js-org-git-bugfix-fix-service-worker.webpack-docs.vercel.app

@chenxsan
Copy link
Member Author

chenxsan commented Nov 4, 2020

Got a weird issue in CI https://travis-ci.org/github/webpack/webpack.js.org/jobs/741364115#L283:

setup compilation HtmlWebpackPluginTypeError: Cannot read property 'Compilation' of undefined
    at compiler.hooks.thisCompilation.tap (/home/travis/build/webpack/webpack.js.org/node_modules/html-webpack-plugin/index.js:210:26)
    at Hook.eval [as call] (eval at create (/home/travis/build/webpack/webpack.js.org/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:64:1)
    at Hook.CALL_DELEGATE [as _call] (/home/travis/build/webpack/webpack.js.org/node_modules/tapable/lib/Hook.js:14:14)
    at Compiler.newCompilation (/home/travis/build/webpack/webpack.js.org/node_modules/webpack/lib/Compiler.js:915:30)
    at hooks.beforeCompile.callAsync.err (/home/travis/build/webpack/webpack.js.org/node_modules/webpack/lib/Compiler.js:957:29)
    at Hook.eval [as callAsync] (eval at create (/home/travis/build/webpack/webpack.js.org/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:40:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/travis/build/webpack/webpack.js.org/node_modules/tapable/lib/Hook.js:18:14)
    at Compiler.compile (/home/travis/build/webpack/webpack.js.org/node_modules/webpack/lib/Compiler.js:952:28)
    at compiler.hooks.watchRun.callAsync.err (/home/travis/build/webpack/webpack.js.org/node_modules/webpack/lib/Watching.js:113:19)
    at Hook.eval [as callAsync] (eval at create (/home/travis/build/webpack/webpack.js.org/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:26:1)

Downgrading html-webpack-plugin fixed the problem.

@@ -57,7 +57,7 @@
"jest": "jest",
"cypress:open": "cypress open",
"cypress:run": "cypress run",
"cypress:ci": "start-server-and-test http-get://localhost:3000 cypress:run"
"cypress:ci": "start-server-and-test build-test http://localhost:4200 cypress:run"
Copy link
Member Author

@chenxsan chenxsan Nov 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're testing against the production site now, I think it makes more sense than testing against development site.

@chenxsan
Copy link
Member Author

chenxsan commented Dec 3, 2020

Oops, we have a problem GoogleChrome/workbox#2493 now:

Uncaught ReferenceError: regeneratorRuntime is not defined

@chenxsan chenxsan merged commit b57379d into webpack:master Dec 3, 2020
@chenxsan chenxsan deleted the bugfix/fix-service-worker branch December 3, 2020 11:21
const { Compilation, sources } = require('webpack');
const getManifestEntriesFromCompilation = require('workbox-webpack-plugin/build/lib/get-manifest-entries-from-compilation');

module.exports = class PrecacheSsgManifestPlugin {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to an external dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants