Adds brotli support for modern javascript #674

prateekbh · 2018-11-27T19:24:48Z

What kind of change does this PR introduce?

Feature

Did you add tests for your changes?
WIP

Summary
This PR does the following

Adds brotli support for precaching first party modern javascript, hence reducing the total cost/ total download amount from network to the end user.
Puts this behind a flag as one needs to add headers to their jS serving infra, before enabling this(see the added console.log)
Switches the underlying sw builder from sw-precache to workbox, for better extendable code.(next PR png -> webP)
Adds the capability to copy sw.js into user land and add custom logic to it.
Fixes [next] Service Worker precaches HTML files, but they aren't used #751

We only add brotli support modern js because we can be sure without a test that browser supporting esm supports brotli.

Does this PR introduce a breaking change?
Yes, the existing preact-cli-sw-precache plugins wont work anymore.

But the users will be able to just copy our sw.js file into their src root and add custom logic to it.

prateekbh · 2018-11-27T19:26:55Z

Note: We can extend this support to css files as well

…w-webpack

developit

Couple of things, some important some not.

developit · 2018-12-05T16:14:48Z

packages/cli/lib/lib/sw.js

+    // 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;


Would this work instead of manually cloning?

response = response.clone(); response.headers.set('content-type', 'application/javascript') return response;

This should – it's what my service workers are doing, tho I just pray that they work

am i doing something wrong here?

my bad - this would work though:

const headers = new Headers(clonedResponse.headers); newHeaders.set('content-type', 'application/javascript'); return new Response(await clonedResponse.text(), { headers })

packages/cli/lib/lib/sw.js

developit · 2018-12-05T16:17:46Z

packages/cli/lib/lib/webpack/webpack-client-config.js

-				}),
-			);
-		}
+	if (config['sw']) {


Any reason this isn't config.sw?

nups, fixed

developit · 2018-12-05T16:20:15Z

packages/cli/lib/lib/webpack/webpack-sw-config.js

+      sw: swSrc,
+    },
+    target: 'webworker',
+		output: {


spacing (https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig)

done.. i guess?

developit · 2018-12-05T16:23:34Z

packages/cli/lib/lib/webpack/webpack-sw-config.js

+      filename: '[name]-esm.js',
+      beforeStartExecution: (plugins) => {
+        plugins.forEach(plugin => {
+          if (plugin.constructor.name === 'DefinePlugin' && plugin.definitions) {


What's the reason for only mutating values in-place here - does this fix DefinePlugin definitions leaking into the parent compiler?

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'; } });

good point, i can chk that

works, fixed

developit · 2018-12-05T16:32:15Z

packages/cli/lib/lib/webpack/webpack-sw-config.js

+
+  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'));


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.')); } }

developit · 2018-12-05T16:33:32Z

packages/cli/lib/lib/webpack/webpack-client-config.js

-	);
-};
+	)];
+	if (env.sw) {


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.

How is the child compiler affected by the its parent compilation mode?

…into sw-webpack

prateekbh · 2018-12-19T18:44:59Z

@developit I re-did this with a child compiler, please review

# Conflicts: # packages/cli/lib/lib/entry.js # packages/cli/lib/lib/webpack/webpack-client-config.js # packages/cli/package.json

prateekbh · 2019-03-07T18:25:47Z

packages/cli/lib/lib/webpack/sw-plugin.js

+					sw: swSrc,
+				};
+				childCompiler.options.target = 'webworker';
+				childCompiler.options.output = JSON.parse(


@developit @ForsakenHarmony do we have any dep already which can help me do this deepcopy?

nvm fixed it

ForsakenHarmony · 2019-04-04T15:26:40Z

packages/cli/lib/commands/build.js

@@ -22,6 +23,15 @@ module.exports = async function(src, argv) {
 		);
 	}

+	if (argv.brotli) {
+		console.log(


guess this should use the util log fn

ForsakenHarmony · 2019-04-04T15:27:11Z

packages/cli/lib/lib/webpack/sw-plugin.js

+					)
+				);
+			} else {
+				console.log(


same thing here?

ForsakenHarmony

Other than that looks good

developit

its alive!

prateekbh · 2019-04-04T15:56:39Z

Will fix these
Merge this
Release RC
Write tests

… sw-webpack

prateekbh · 2019-04-05T07:24:57Z

Fixed comments!

prateekbh added 6 commits November 20, 2018 10:28

WIP adding sw as a webpack dependency

0507885

Merge branch 'babel-bug-fix' into sw-webpack

c5d5246

adding sw-config

5a37b9f

Brotli support for modern javascript

65a507c

adding brotli plugin to webpack

02498fe

fixing conditional plugins

375c545

prateekbh added 7 commits November 27, 2018 16:24

lint fixes

9c34521

fixing error

e9e1cdc

bug-fix

8b84b4d

WIP fix watch mode

cc29725

Merge branch 'next' of https://github.com/developit/preact-cli into s…

1500698

…w-webpack

fix watch mode

86f0f78

add the capability to build sw from user land

3d1ac3a

prateekbh requested review from lukeed, ForsakenHarmony and developit December 5, 2018 02:29

prateekbh changed the title ~~WIP: Adds brotli support for modern javascript~~ Adds brotli support for modern javascript Dec 5, 2018

developit requested changes Dec 5, 2018

View reviewed changes

prateekbh added 7 commits December 9, 2018 20:04

fixing comments

1369f2c

fixing spacing

3135b3b

fixing in place mutation

c1fd332

no multi compilers

8aec475

Update run-webpack.js

8bc9521

removing unwanted files

dcc8f41

Merge branch 'sw-webpack' of https://github.com/developit/preact-cli …

434a813

…into sw-webpack

prateekbh added 2 commits December 21, 2018 11:38

Update sw-plugin.js

9ed7049

Update sw-plugin.js

fb28138

prateekbh mentioned this pull request Jan 11, 2019

adding swSrc as webpack asset capability GoogleChrome/workbox#1763

Merged

ForsakenHarmony and others added 3 commits January 26, 2019 01:45

Merge branch 'next' into sw-webpack

81bb193

# Conflicts: # packages/cli/lib/lib/entry.js # packages/cli/lib/lib/webpack/webpack-client-config.js # packages/cli/package.json

making changes for workbox v4

dc64998

fixes for regexp

9a04494

prateekbh mentioned this pull request Feb 3, 2019

Enhance service worker #669

Closed

prateekbh mentioned this pull request Feb 13, 2019

Add support for webp images #730

Merged

prateekbh added 8 commits February 19, 2019 23:27

addressing comments

95d3ccb

precacing only index.html

400fdb2

Merge branch 'next' into sw-webpack

7f21e24

Update run-webpack.js

fdf81b4

Update run-webpack.js

37d9ec7

fixing kluer dep

9054e9b

fixing package.json

126cd49

fixing unhashed bundle bug

c95b6dc

prateekbh mentioned this pull request Mar 7, 2019

[next] Service Worker precaches HTML files, but they aren't used #751

Closed

prateekbh commented Mar 7, 2019

View reviewed changes

no json parse

4302de4

ForsakenHarmony changed the base branch from next to master March 19, 2019 19:01

ForsakenHarmony reviewed Apr 4, 2019

View reviewed changes

packages/cli/lib/lib/webpack/sw-plugin.js Outdated

)

);

} else {

console.log(

Copy link

Member

ForsakenHarmony Apr 4, 2019

Choose a reason for hiding this comment

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

same thing here?

ForsakenHarmony approved these changes Apr 4, 2019

View reviewed changes

ForsakenHarmony requested a review from developit April 4, 2019 15:30

developit approved these changes Apr 4, 2019

View reviewed changes

prateekbh added 2 commits April 5, 2019 00:20

fixing comments

8935426

Merge branch 'master' of https://github.com/developit/preact-cli into…

b12603b

… sw-webpack

prateekbh merged commit efdb448 into master Apr 5, 2019

prateekbh deleted the sw-webpack branch April 5, 2019 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds brotli support for modern javascript #674

Adds brotli support for modern javascript #674

prateekbh commented Nov 27, 2018 •

edited

Loading

prateekbh commented Nov 27, 2018

developit left a comment

developit Dec 5, 2018

lukeed Dec 5, 2018

prateekbh Dec 10, 2018

developit Feb 13, 2019 •

edited

Loading

prateekbh Feb 20, 2019

developit Dec 5, 2018

prateekbh Dec 10, 2018

developit Dec 5, 2018

prateekbh Dec 10, 2018

developit Dec 5, 2018

prateekbh Dec 10, 2018

prateekbh Dec 10, 2018

developit Dec 5, 2018

prateekbh Dec 10, 2018

developit Dec 5, 2018

prateekbh Dec 10, 2018

prateekbh commented Dec 19, 2018

prateekbh Mar 7, 2019 •

edited

Loading

prateekbh Mar 7, 2019

ForsakenHarmony Apr 4, 2019

ForsakenHarmony Apr 4, 2019

ForsakenHarmony left a comment

developit left a comment

prateekbh commented Apr 4, 2019

prateekbh commented Apr 5, 2019

Adds brotli support for modern javascript #674

Adds brotli support for modern javascript #674

Conversation

prateekbh commented Nov 27, 2018 • edited Loading

prateekbh commented Nov 27, 2018

developit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

developit Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prateekbh commented Dec 19, 2018

prateekbh Mar 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ForsakenHarmony left a comment

Choose a reason for hiding this comment

developit left a comment

Choose a reason for hiding this comment

prateekbh commented Apr 4, 2019

prateekbh commented Apr 5, 2019

prateekbh commented Nov 27, 2018 •

edited

Loading

developit Feb 13, 2019 •

edited

Loading

prateekbh Mar 7, 2019 •

edited

Loading