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

Adds importScriptsViaChunks option to webpack plugin #2131

Merged
merged 1 commit into from
Jul 19, 2019

Conversation

jeffposnick
Copy link
Contributor

R: @philipwalton

Fixes #1752

This new option in workbox-webpack-plugin's GenerateSW mode allows you to use the name of chunks from your webpack compilation as the source for importScripts(). It also takes steps to exclude whatever's imported via importScripts() from the precache manifest, since the service worker maintains its own cache of imported scripts.

It's important that developers use this with versioned filenames (i.e. including a hash), since the current service worker implementations in browsers won't check for updates to imported scripts unless their URL changes.

In the course of writing tests for this, I also fixed some of the validation logic around calls to importScripts, making it so that we can check to make sure that the Workbox runtime is imported without having to hardcode the exact filename. This should lead to less test case churn resulting from small changes in the Workbox runtime code leading to new hashed filenames.

@workbox-pr-bot
Copy link
Collaborator

PR-Bot Size Plugin

Changed File Sizes

File Before After Change GZipped
packages/workbox-webpack-plugin/build/generate-sw.js 3.13 KB 3.71 KB +18% 1.40 KB ☠️

New Files

No new files have been added.

All File Sizes

View Table
File Before After Change GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js 3.86 KB 3.86 KB 0% 1.59 KB
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js 1.92 KB 1.92 KB 0% 959 B
packages/workbox-build/build/_types.js 41 B 41 B 0% 61 B
packages/workbox-build/build/index.js 3.30 KB 3.30 KB 0% 1.31 KB
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js 594 B 594 B 0% 354 B
packages/workbox-cli/build/app.js 4.15 KB 4.15 KB 0% 1.64 KB
packages/workbox-cli/build/bin.js 940 B 940 B 0% 502 B
packages/workbox-core/build/workbox-core.prod.js 5.94 KB 5.94 KB 0% 2.46 KB
packages/workbox-expiration/build/workbox-expiration.prod.js 2.94 KB 2.94 KB 0% 1.27 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js 1.95 KB 1.95 KB 0% 913 B
packages/workbox-navigation-preload/build/workbox-navigation-preload.prod.js 660 B 660 B 0% 324 B
packages/workbox-precaching/build/workbox-precaching.prod.js 4.25 KB 4.25 KB 0% 1.70 KB
packages/workbox-range-requests/build/workbox-range-requests.prod.js 1.57 KB 1.57 KB 0% 797 B
packages/workbox-routing/build/workbox-routing.prod.js 3.41 KB 3.41 KB 0% 1.47 KB
packages/workbox-strategies/build/workbox-strategies.prod.js 4.10 KB 4.10 KB 0% 1.13 KB
packages/workbox-streams/build/workbox-streams.prod.js 1.46 KB 1.46 KB 0% 720 B
packages/workbox-sw/build/workbox-sw.js 1.34 KB 1.34 KB 0% 747 B
packages/workbox-webpack-plugin/build/generate-sw.js 3.13 KB 3.71 KB +18% 1.40 KB ☠️
packages/workbox-webpack-plugin/build/index.js 349 B 349 B 0% 255 B
packages/workbox-webpack-plugin/build/inject-manifest.js 4.67 KB 4.67 KB 0% 1.59 KB
packages/workbox-window/build/workbox-window.dev.umd.js 31.88 KB 31.88 KB 0% 8.10 KB
packages/workbox-window/build/workbox-window.prod.umd.js 4.46 KB 4.46 KB 0% 1.84 KB

Workbox Aggregate Size Plugin

3.49KB gzip'ed (23% of limit)
7.8KB uncompressed

@jeffposnick jeffposnick merged commit 3f465a4 into master Jul 19, 2019
@jeffposnick jeffposnick deleted the webpack-import-chunks branch July 19, 2019 17:42
@madmoizo
Copy link

The current order importScripts > importScriptsViaChunk fits my use case but a single importScripts property with chunk support could be more generic.

{
  importScripts: ['./prechunk-script.js', 'chunk', './postchunk-script.js']
}

@jeffposnick
Copy link
Contributor Author

I am not a fan of the overloaded nature of passing in chunk names mixed in with script names, and looking at how sw-precache-webpack-plugin implemented it, it's also overly complex:

  • importScripts: [Array<String|Object>]
    • When importScripts array item is a String:
      • Converts to object format { filename: '/my-script.js'}
    • When importScripts array item is an Object:
      • Looks for chunkName property.
      • Looks for filename property.
      • If a chunkName is specified, it will override the accompanied value for filename.

I guess I'm hoping that folks who really need a specific order will be able to use either importScripts or importScriptsViaChunks, rather than both? We'll see how well that works out based on alpha feedback.

@madmoizo
Copy link

madmoizo commented Jul 29, 2019

One last thing, I used InjectManifest until now but I see importScriptsViaChunk has been added to GenerateSW only so I tried it in v4:
image
The boilerplate (yellow) generated by GenerateSW is already a part of my custom imported chunk, so is it possible to disable this boilerplate in v5? If not, could you add importScriptsViaChunk to InjectManifest?

@jeffposnick
Copy link
Contributor Author

If you're using InjectManifest in v5, then your swSrc should go through a full compilation. Can you try explicitly importing (using a syntax that webpack natively supports) the code from the desired chunk inside of your swSrc file, and allow webpack to inline that chunk's code, rather than relying on importScriptsViaChunks?

@madmoizo
Copy link

madmoizo commented Jul 29, 2019

@jeffposnick I guess I can if swSrc can be a lodash template and if there is a way to access the list of chunks, like in html-webpack-plugin but I didn't find any details about this in the doc, could you help?

Edit: It seems I misunderstand, you mean I have to duplicate the webpack config to have a compilation of my sw chunk directly in InjectManifest processing ?
webpackCompilationPlugins is just for the plugins property of the webpack config? If so, it won't be enough.

@madmoizo
Copy link

madmoizo commented Jul 29, 2019

tested in v5.

webpack config

new WorkboxPlugin.InjectManifest({
  swSrc: `${rootPath}/src/sw.js`,
  swDest: 'sw.js',
  include: [
    /\.html$/,
    /\.js$/,
    /\.css$/,
    /\.woff2$/,
    /\.jpg$/,
    /\.png$/
  ]
})

sw.js

importScripts('/env.js')

import './sw/main.js' // entry point

importScripts('/env.js') is automatically put after import by webpack so script evaluation fails (it contains global vars used in the main script)
image
If you have an idea...

Edit: the idea is

import ./importScripts.js
import ./entryPoint.js

importScripts.js

importScripts('./env.js')

@jeffposnick
Copy link
Contributor Author

(The comments of a closed PR isn't a great place to discuss this—can you open a new issue if you're still running into problems?)

My suggestion was that instead of using importScripts('abc.js') in your swSrc file, you can use import ./abc.js (or some other syntax that webpack supports for pulling module code).

In v5, swSrc should end up being compiled by webpack just like any other source file, so the import ./abc.js should also end up being compiled. You may need to confirm a specific set of loaders to get exactly the behavior you want during that compilation, but that's a problem that always exists for webpack.

@madmoizo
Copy link

madmoizo commented Aug 5, 2019

@jeffposnick Thanks for the reply, I added Edit note in my previous comment with the solution. Everything is good 👍

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 this pull request may close these issues.

workbox-webpack-plugin: accept chunk names in importScripts
4 participants