-
-
Notifications
You must be signed in to change notification settings - Fork 375
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 brotli support for modern javascript #674
Changes from 13 commits
0507885
c5d5246
5a37b9f
65a507c
02498fe
375c545
9c34521
e9e1cdc
8b84b4d
cc29725
1500698
86f0f78
3d1ac3a
1369f2c
3135b3b
c1fd332
8aec475
8bc9521
dcc8f41
434a813
9ed7049
fb28138
81bb193
dc64998
9a04494
95d3ccb
400fdb2
7f21e24
fdf81b4
37d9ec7
9054e9b
126cd49
c95b6dc
4302de4
8935426
b12603b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
self.__precacheManifest = [].concat(self.__precacheManifest || []); | ||
|
||
/* global workbox */ | ||
workbox.precaching.suppressWarnings(); | ||
/** We are sure brotli is enabled for browsers supporting script type=module | ||
* so we do brotli support only for them. | ||
* We can do brolti support for other browsers but there is no good way of | ||
* feature detect the same at the time of pre-caching. | ||
*/ | ||
if (process.env.ENABLE_BROTLI && process.env.ES_BUILD) { | ||
// Alter the precache manifest to precache brotli files instead of gzip files. | ||
self.__precacheManifest = self.__precacheManifest.map(asset => { | ||
if (/.*.js$/.test(asset.url)) { | ||
asset.url = asset.url.replace(/.esm.js$/, ".esm.js.br"); | ||
} | ||
return asset; | ||
}); | ||
|
||
class BrotliRedirectPlugin { | ||
// Before saving the response in cache, we need to treat the headers. | ||
async cacheWillUpdate({request, response}) { | ||
if (/.js.br$/.test(request.url)) { | ||
const headers = response.headers; | ||
const newHeaders = {}; | ||
for (let key of headers.keys()) { | ||
newHeaders[key] = headers.get(key); | ||
} | ||
// make sure the content type is compatible | ||
newHeaders["content-type"] = "application/javascript"; | ||
return new Response((await response.text()), { | ||
headers: newHeaders, | ||
}); | ||
} | ||
return response; | ||
} | ||
} | ||
workbox.precaching.addPlugins(new BrotliRedirectPlugin()); | ||
} | ||
|
||
const precacheOptions = {}; | ||
if (process.env.ENABLE_BROTLI) { | ||
developit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
precacheOptions['urlManipulation'] = ({url}) => { | ||
if (/.esm.js$/.test(url.href)) { | ||
url.href = url.href + ".br"; | ||
} | ||
return [url]; | ||
}; | ||
} | ||
|
||
workbox.precaching.precacheAndRoute(self.__precacheManifest, precacheOptions); | ||
|
||
workbox.routing.registerNavigationRoute("index.html", { | ||
whitelist: [/^(?!\/__).*/], | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ const { existsSync } = require('fs'); | |
const merge = require('webpack-merge'); | ||
const { filter } = require('minimatch'); | ||
const CopyWebpackPlugin = require('copy-webpack-plugin'); | ||
const SWPrecacheWebpackPlugin = require('sw-precache-webpack-plugin'); | ||
const TerserPlugin = require('terser-webpack-plugin'); | ||
const OptimizeCssAssetsPlugin = require('optimize-css-assets-webpack-plugin'); | ||
const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); | ||
|
@@ -13,7 +12,10 @@ const RenderHTMLPlugin = require('./render-html-plugin'); | |
const PushManifestPlugin = require('./push-manifest'); | ||
const baseConfig = require('./webpack-base-config'); | ||
const BabelEsmPlugin = require('babel-esm-plugin'); | ||
const { InjectManifest } = require('workbox-webpack-plugin'); | ||
const BrotliPlugin = require('brotli-webpack-plugin'); | ||
const { normalizePath } = require('../../util'); | ||
const swWebPackConfig = require('./webpack-sw-config'); | ||
|
||
const cleanFilename = name => name.replace(/(^\/(routes|components\/(routes|async))\/|(\/index)?\.js$)/g, ''); | ||
|
||
|
@@ -114,7 +116,7 @@ function isProd(config) { | |
'process.env.ADD_SW': config.sw, | ||
'process.env.ES_BUILD': false, | ||
'process.env.ESM': config.esm, | ||
}) | ||
}), | ||
], | ||
|
||
optimization: { | ||
|
@@ -146,26 +148,6 @@ function isProd(config) { | |
}, | ||
}; | ||
|
||
if (config.sw) { | ||
prodConfig.plugins.push( | ||
new SWPrecacheWebpackPlugin({ | ||
filename: 'sw.js', | ||
navigateFallback: 'index.html', | ||
navigateFallbackWhitelist: [/^(?!\/__).*/], | ||
minify: true, | ||
stripPrefix: config.cwd, | ||
staticFileGlobsIgnorePatterns: [ | ||
/\.esm\.js$/, | ||
/polyfills(\..*)?\.js$/, | ||
/\.map$/, | ||
/push-manifest\.json$/, | ||
/.DS_Store/, | ||
/\.git/ | ||
] | ||
}), | ||
); | ||
} | ||
|
||
if (config.esm) { | ||
prodConfig.plugins.push( | ||
new BabelEsmPlugin({ | ||
|
@@ -193,26 +175,23 @@ function isProd(config) { | |
}, | ||
}), | ||
); | ||
config['sw'] && prodConfig.plugins.push( | ||
new InjectManifest({ | ||
swSrc: resolve(config.dest, 'sw-esm.js'), | ||
include: [/\.html$/, /\.esm.js$/, /\.css$/, /\.(png|jpg)$/], | ||
precacheManifestFilename: 'precache-manifest.[manifestHash].esm.js' | ||
}), | ||
); | ||
} | ||
|
||
if (config.sw) { | ||
prodConfig.plugins.push( | ||
new SWPrecacheWebpackPlugin({ | ||
filename: 'sw-esm.js', | ||
navigateFallback: 'index.html', | ||
navigateFallbackWhitelist: [/^(?!\/__).*/], | ||
minify: true, | ||
stripPrefix: config.cwd, | ||
staticFileGlobsIgnorePatterns: [ | ||
/(\.[\w]{5}\.js)/, | ||
/polyfills(\..*)?\.js$/, | ||
/\.map$/, | ||
/push-manifest\.json$/, | ||
/.DS_Store/, | ||
/\.git/ | ||
] | ||
}), | ||
); | ||
} | ||
if (config['sw']) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason this isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nups, fixed |
||
prodConfig.plugins.push( | ||
new InjectManifest({ | ||
swSrc: resolve(config.dest, 'sw.js'), | ||
include: [/\.html$/, /\.js$/, /\.css$/, /\.(png|jpg)$/], | ||
exclude: [/\.esm\.js$/] | ||
}) | ||
); | ||
} | ||
|
||
if (config['inline-css']) { | ||
|
@@ -229,6 +208,14 @@ function isProd(config) { | |
); | ||
} | ||
|
||
if (config.brotli) { | ||
prodConfig.plugins.push( | ||
new BrotliPlugin({ | ||
test: /\.esm\.js$/, | ||
}) | ||
); | ||
} | ||
|
||
return prodConfig; | ||
} | ||
|
||
|
@@ -273,9 +260,13 @@ function isDev(config) { | |
} | ||
|
||
module.exports = function (env) { | ||
return merge( | ||
const config = [merge( | ||
baseConfig(env), | ||
clientConfig(env), | ||
(env.isProd ? isProd : isDev)(env) | ||
); | ||
}; | ||
)]; | ||
if (env.sw) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the Array return here done with the intention of moving to a MultiCompiler? This seems like it'll break in future Workbox versions that use child compilation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is the child compiler affected by the its parent compilation mode? |
||
config.unshift(swWebPackConfig(env)); | ||
} | ||
return config; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
const webpack = require('webpack'); | ||
const merge = require('webpack-merge'); | ||
const baseConfig = require('./webpack-base-config'); | ||
const BabelEsmPlugin = require('babel-esm-plugin'); | ||
const fs = require('fs'); | ||
const {yellow} = require('chalk'); | ||
const { resolve } = require('path'); | ||
|
||
function swConfig(config) { | ||
const { dest, src, brotli, esm } = config; | ||
|
||
const plugins = [ | ||
new webpack.DefinePlugin({ | ||
'process.env.ES_BUILD': false, | ||
'process.env.ENABLE_BROTLI': brotli, | ||
'process.env.NODE_ENV': 'production', | ||
}), | ||
]; | ||
|
||
esm && plugins.push( | ||
new BabelEsmPlugin({ | ||
filename: '[name]-esm.js', | ||
beforeStartExecution: (plugins) => { | ||
plugins.forEach(plugin => { | ||
if (plugin.constructor.name === 'DefinePlugin' && plugin.definitions) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for only mutating values in-place here - does this fix Otherwise this could just be plugins.forEach(plugin => {
if (plugin.constructor.name === 'DefinePlugin') {
if (!plugin.definitions) throw Error('ESM Error: DefinePlugin found without definitions.');
plugin.definitions['process.env.ES_BUILD'] = 'true';
}
}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, i can chk that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. works, fixed |
||
for (const definition in plugin.definitions) { | ||
if (definition === 'process.env.ES_BUILD') { | ||
plugin.definitions[definition] = true; | ||
} | ||
} | ||
} else if (plugin.constructor.name === 'DefinePlugin' && !plugin.definitions) { | ||
throw new Error('WebpackDefinePlugin found but not `process.env.ES_BUILD`.'); | ||
} | ||
}); | ||
} | ||
}) | ||
); | ||
|
||
let swSrc = resolve(__dirname, './../sw.js'); | ||
if (fs.existsSync(resolve(`${src}/sw.js`))) { | ||
console.log(yellow('⚛️ Info - CLI found sw.js in source root folder, using that to compile the final service worker instead')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: can we make this a different color like blue? yellow seems like a warning, this is informational. Also maybe shorten the message since it's a heads-up that will be shown every build? Or only show the message when things change (first build or sw.js added/removed): // only warn on first build, OR on rebuild if sw.js is added/removed:
let swSrc = resolve(__dirname, './../sw.js');
const exists = fs.existsSync(resolve(`${src}/sw.js`));
if (exists !== global.hasSeenSw) {
global.hasSeenSw = exists;
if (exists) {
console.log(blue('⚛️ Detected custom sw.js: compiling instead of default Service Worker.'));
}
else {
console.log(blue('⚛️ No custom sw.js detected: compiling default Service Worker.'));
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
swSrc = resolve(`${src}/sw.js`); | ||
} | ||
|
||
return { | ||
context: src, | ||
entry: { | ||
sw: swSrc, | ||
}, | ||
target: 'webworker', | ||
output: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done.. i guess? |
||
path: dest, | ||
publicPath: '/', | ||
filename: '[name].js', | ||
}, | ||
plugins, | ||
}; | ||
} | ||
|
||
module.exports = function (env) { | ||
return merge(baseConfig(env), swConfig(env)); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this work instead of manually cloning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should – it's what my service workers are doing, tho I just pray that they work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am i doing something wrong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad - this would work though:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed