Prefetch and preload from entry chunk. by MLoughry · Pull Request #7444 · webpack/webpack

MLoughry · 2018-05-29T22:56:46Z

Following up on @sokra's work in #7056, this change addresses #7084 to have webpack prefetch and preload designated chunks from the entry chunk. It adds a function (with its name configurable by output.entryPrefetchFunction) that can be called by consuming code to trigger the prefetch/preload.

Add tests

What kind of change does this PR introduce?

I would call it a bugfix; @sokra may disagree.

Did you add tests for your changes?

I have not yet added tests; I will work on them, but wanted to get feedback on the design first.

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

Any documentation (which I could not locate in this repro) that references webpackPreload or webpackPrefetch should no longer add the caveat about not working from the entry chunk.

Additionally, with my second commit, need to document the output.entryPrefetchFunction option.

@sokra

Following up on @sokra's work in webpack#7056, this change addresses webpack#7084 to have webpack prefetch and preload designated chunks from the entry chunk.

jsf-clabot · 2018-05-29T22:56:48Z

All committers have signed the CLA.

webpack-bot · 2018-05-29T22:56:50Z

For maintainers only:

This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

MLoughry · 2018-05-29T23:17:38Z

I'm having some trouble adding tests, since the code executes during bootstrap. The tests in #7056 work because the FakeDocument is installed before the associated code that adds prefetch links executes.

Rather than calling the prefetch function on its own during bootstrap, this change exposes the function as a function that can be called from the compiled code.

MLoughry · 2018-05-30T00:04:12Z

Even changing the code to not automatically call the prefetch itself, I can't access the function from the prefetch-preload tests, since it clears out the window object before the test.

As far as I can find, none of the existing tests test anything like this (a publicly exposed function), so I don't have good examples to base my tests off of.

sokra · 2018-05-30T08:28:00Z

Ok I see what you want to do. Let's solve this a bit different. I'm not happy with exposing an extra function, but I would be ok to enable triggering prefetch on bootstrap. preload has a different meaning and triggering it on bootstrap would change the semantics (it only makes sense as <link> tag).

We can add this behavior in a back-compat way without extra option.

verify that adding prefetch <link> twice doesn't cause two requests
- if that doesn't work: read existing <link> tags on bootstrap to fill up chunkPrefetchMap
move prefetching code from requireEnsure into webpackJsonp
- This will also trigger prefetching for initial chunks (without runtime)
- Relevant
after the startup code trigger the prefetching of chunks for the entry chunk.
- This will trigger prefetching for the entry chunk
getChildIdsByOrdersMap only takes async chunks. You can add an option to include initial chunks too.
Take a not of this line. prefetching is only triggered for the last chunk of a ChunkGroup.
- This avoid over-fetching for shared chunks. It also means that a runtimeChunk: "single" will not trigger any prefetching.

Why is this backward-compatible?

Using it in the existing way with <link> tags added for prefetch will continue to work.
When not adding <link> tags we now added the feature to trigger prefetching. This was also the users intend as she/he added webpackPrefetch hints.
bundle size doesn't increase significantly.

This reverts commit 7458526.

This reverts commit 482b4b3.

…y prefetch" This reverts commit b642403.

This reverts commit eefacf3.

MLoughry · 2018-05-30T22:54:08Z

@sokra I updated the change to match what (I interpreted) you suggested. I even added a small optimization to split the chunkPrefetchMap among all the chunks, rather than loading it all upfront in the entry chunk.

However, I'm still running into test issues, because the bootstrap code is called before the prefetch-preload, hash-length, or commons-chunk-plugin test cases can install their FakeDocument instances.

sokra · 2018-06-01T08:16:47Z

I even added a small optimization to split the chunkPrefetchMap among all the chunks, rather than loading it all upfront in the entry chunk.

Good idea.

However, I'm still running into test issues, because the bootstrap code is called before the prefetch-preload, hash-length, or commons-chunk-plugin test cases can install their FakeDocument instances.

We could move this into the ConfigTestCases.test.js file.

sokra · 2018-06-01T08:08:26Z

+						`webpackJsonpCallback([${JSON.stringify(
+							chunk.ids
+						)}, {}, null, ${JSON.stringify(
+							chunk.getChildIdsByOrders().prefetch


Check of chunk.getChildIdsByOrders().prefetch is not empty

sokra · 2018-06-01T08:08:48Z

 				if (entries.length > 0) {
 					source.add(`,${JSON.stringify(entries)}`);
+				} else {
+					source.add(`,null`);


This could be omitted when chunk.getChildIdsByOrders().prefetch is empty.

I don't want to increase chunk size if prefetching isn't used.

Also use 0 instead of null, that's shorter. (a bit)

sokra · 2018-06-01T08:10:50Z

 							"var chunkIds = data[0];",
 							"var moreModules = data[1];",
 							withDefer ? "var executeModules = data[2];" : "",
+							"var prefetchChunks = data[3] || []",


Similar to withDefer check if we need this code.

sokra · 2018-06-01T08:13:06Z

 				return source;
 			}
 		);
+		mainTemplate.hooks.beforeStartup.tap(


Trigger prefetching after the startup code runs. This would omit prefetching when import() is used in initialization code.

Where? There's no afterStartup hook, and startup includes a return statement that prevents later code from executing.

sokra · 2018-06-01T08:14:28Z

                      Asset      Size  Chunks             Chunk Names
    9ac13fb7087e9ff1b93e.js  1.05 KiB       0  [emitted]  
-    f2e891598128a57b072c.js    11 KiB       1  [emitted]  
+    f2e891598128a57b072c.js  11.6 KiB       1  [emitted]  


bundle size of most of these test cases shouldn't increase, since prefetching isn't used. We don't want to users to pay for not using this feature.

Following most of the other comments, I was able to eliminate most of the differences. However, to not emit blank lines when not using prefetching, I followed a different pattern than the rest of the code:

"var moreModules = data[1];", withDefer ? "var executeModules = data[2];" : "", ...(withPrefetch ? ["var prefetchChunks = data[3] || []"] : []), '// add "moreModules" to the modules object,',

sokra · 2018-06-01T08:17:17Z

PR looks good

This reverts commit 23ab98d.

- [x] Don't emit prefetch code if not needed - [x] Only affect StatsTestCase dealing with prefetching - [x] Fix existing prefetch test case - [ ] Trigger prefetch after startup code runs - [ ] Add new test(s)

webpack-bot · 2018-06-01T17:51:59Z

@MLoughry Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

MLoughry · 2018-06-01T17:56:07Z

Trying to add webpackPrefetch: true to the existing import in the prefetch-preload test case still results in the issue where the document isn't found. I tried moving the beforeEach/afterEach instalaation of the FakeDocument to ConfigTestsCases, but that just caused the various test cases to be unable to find document. So, I still don't have new test cases.

I was also unclear on your statement about:

Trigger prefetching after the startup code runs. This would omit prefetching when import() is used in initialization code.

as I replied to above.

Lastly, I'm headed out on vacation for the next week, so I may not be able to pick this back up until the week after next.

minor style issues add test case

This makes more sense as startup could take a while and we can use the time to fetch some resources

webpack-bot · 2018-06-04T17:03:06Z

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

sokra · 2018-06-04T18:07:32Z

Thanks

webpack-bot · 2018-06-04T18:07:33Z

I've created an issue to document this in webpack/webpack.js.org.

theKashey · 2019-02-21T05:35:19Z

Could this feature be used, or better not, as long as it still undocumented?

Prefetch and preload from entry chunk.

eefacf3

Following up on @sokra's work in webpack#7056, this change addresses webpack#7084 to have webpack prefetch and preload designated chunks from the entry chunk.

webpack-bot added the PR: unreviewed label May 29, 2018

Add output.entryPrefetchFunction option, and don't immediately prefetch

b642403

Rather than calling the prefetch function on its own during bootstrap, this change exposes the function as a function that can be called from the compiled code.

webpack-bot added the PR: CI-not-ok label May 30, 2018

MLoughry added 2 commits May 29, 2018 17:11

Add tthe new option to the validation schema

482b4b3

Run prettier

7458526

MLoughry added 7 commits May 30, 2018 10:17

Revert "Run prettier"

d26617a

This reverts commit 7458526.

Revert "Add tthe new option to the validation schema"

6d85e8d

This reverts commit 482b4b3.

Revert "Add output.entryPrefetchFunction option, and don't immediatel…

8636670

…y prefetch" This reverts commit b642403.

Revert "Prefetch and preload from entry chunk."

e483682

This reverts commit eefacf3.

Initial rework per sokra's suggestion

685cbcd

Split the map among the chunks

8a995da

Update snapshots

23ab98d

sokra requested changes Jun 1, 2018

View reviewed changes

webpack-bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Jun 1, 2018

MLoughry added 2 commits June 1, 2018 09:35

Revert "Update snapshots"

88d09b0

This reverts commit 23ab98d.

Address most of Sokra's comments

0d21ce1

- [x] Don't emit prefetch code if not needed - [x] Only affect StatsTestCase dealing with prefetching - [x] Fix existing prefetch test case - [ ] Trigger prefetch after startup code runs - [ ] Add new test(s)

webpack-bot added PR: review-outdated and removed PR: reviewed-changes-requested labels Jun 1, 2018

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Jun 1, 2018

sokra added 2 commits June 4, 2018 17:49

move fake document to ConfigTestCases

db668b7

minor style issues add test case

Merge branch 'master' into prefetch-from-entry

dc0e1ec

webpack-bot added PR: CI-not-ok and removed PR: CI-ok labels Jun 4, 2018

revert afterStartup and trigger prefetch before startup

c77ec39

This makes more sense as startup could take a while and we can use the time to fetch some resources

sokra force-pushed the prefetch-from-entry branch from 85d5228 to c77ec39 Compare June 4, 2018 16:10

fix test case

c65d164

webpack-bot added PR: CI-ok and removed PR: CI-not-ok labels Jun 4, 2018

sokra approved these changes Jun 4, 2018

View reviewed changes

webpack-bot added PR: reviewed-approved and removed PR: review-outdated labels Jun 4, 2018

sokra merged commit bc6b5b0 into webpack:master Jun 4, 2018

webpack-bot mentioned this pull request Jun 4, 2018

Document webpack change: Prefetch and preload from entry chunk. webpack/webpack.js.org#2217

Closed

1 task

mwanago mentioned this pull request Aug 20, 2018

webpackPreload not working at all #7920

Closed

EugeneHlushko mentioned this pull request Mar 16, 2019

webpackPreload not working at all webpack/webpack.js.org#2457

Closed

Uh oh!

Conversation

MLoughry commented May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsf-clabot commented May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webpack-bot commented May 29, 2018

Uh oh!

MLoughry commented May 29, 2018

Uh oh!

MLoughry commented May 30, 2018

Uh oh!

sokra commented May 30, 2018

Uh oh!

MLoughry commented May 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sokra commented Jun 1, 2018

Uh oh!

sokra Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

sokra Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

sokra Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

sokra Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

MLoughry Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

sokra Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

MLoughry Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

sokra commented Jun 1, 2018

Uh oh!

webpack-bot commented Jun 1, 2018

Uh oh!

MLoughry commented Jun 1, 2018

Uh oh!

webpack-bot commented Jun 4, 2018

Uh oh!

sokra commented Jun 4, 2018

Uh oh!

webpack-bot commented Jun 4, 2018

Uh oh!

theKashey commented Feb 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MLoughry commented May 29, 2018 •

edited

Loading

jsf-clabot commented May 29, 2018 •

edited

Loading

MLoughry commented May 30, 2018 •

edited

Loading