Fetch priority #16991

aboktor · 2023-04-14T22:42:10Z

Summary

`🤖 Generated by Copilot at 0619c3e`

This pull request adds a new feature to webpack that allows users and plugins to control the fetch priority of dynamic imports, which can affect the performance and resource management of chunk loading. It introduces a new option dynamicImportFetchPriority in the webpack configuration, and a new property fetchPriority in the ChunkGroup class and the import options object. It also modifies several runtime modules and the LoadScriptRuntimeModule class to support the fetchPriority option. Additionally, it adds a .gitattributes file to enforce consistent line endings for JavaScript files.

Details

Adds webpackFetchPriority to magic comments
Adds dynamicImportFetchPriorityto module.parser.javascript
Consumes that and sets ChunkGroup.options.fetchPriority
Passes ChunkGroup.options.fetchPriority down to ensure chunk and co to set the right priority on the script tag

Currently completely untested but looking for early feedback

`🤖 Generated by Copilot at 0619c3e`

Add a .gitattributes file to enforce LF line endings for JavaScript files (link)
Introduce a new fetchPriority property for ChunkGroup class to represent the priority of fetching a chunk group dynamically (link)
Add a new dynamicImportFetchPriority option to the webpack configuration to specify the global default value for the fetchPriority property (link, link)
Add a new webpackFetchPriority option to the import options object to allow overriding the global default value for the fetchPriority property for a specific dynamic import (link)
Include the fetchPriority option when generating the code for the ensureChunk function in different runtime modules (RuntimeTemplate, EnsureChunkRuntimeModule, JsonpChunkLoadingRuntimeModule) (link, link, link, link)
Set the fetchPriority attribute on the script tag when loading a chunk via loadScript function in LoadScriptRuntimeModule (link, link)

linux-foundation-easycla · 2023-04-14T22:42:13Z

The committers listed above are authorized under a signed CLA.

✅ login: aboktor / name: Andrew Boktor (eb22496, 2d5f67c, 968c3c5)

webpack-bot · 2023-04-14T22:42:16Z

For maintainers only:

This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
This needs to be backported to webpack 4 (issue will be created when merged)

.gitattributes

alexander-akait

Looks good, now we need a test cases

TheLarkInn

This implementation looks great.

We need test cases for at least two/three scenarios:

I can use module.parser.javascript.dynamicImportFetchPriority and it works correctly w/ high/low/auto
I can not use module.parser.javascript.dynamicImportFetchPriority but instead use magic comments /* webpackFetchPriority: "high/low/etc." */ and it works
I can use both features and override at the import() level using webpackFetchPriority: and it works correctly.

Here is example test cases you can leverage/learn from:

https://github.com/aboktor/webpack/blob/d9b5f321aeb39c394252ad0a7ad109d7e78dec48/test/cases/chunks/inline-options/index.js#L1-L31
@alexander-akait Is there anywhere else I'm missing?

declarations/WebpackOptions.d.ts

alexander-akait · 2023-04-15T20:15:17Z

@TheLarkInn Yeah, right, like we do for other import(...) options

TheLarkInn · 2023-04-19T02:16:55Z

@aboktor we merged a few PR's for tomorrow release, and now we have some merge conflicts. Would you mind rebasing, and you can probably move this over to "Ready for review" once you've done so.

aboktor · 2023-04-19T16:19:32Z

The CI is failing on many of the integration tests. Some of those failures I understand (e.g. ConfigTestCases › async-commons-chunk › existing-name › exported tests › should not have duplicate chunks in blocks), and the rest of them I don't. The problem is, I can't repro these failures locally. I tried running yarn cover:integration:a -t async-commons-chunk (tried cover:integration:b as well) and they pass (except for may be open handles)
Ask: How do I repro the CI failures?
@alexander-akait

alexander-akait · 2023-04-19T22:27:32Z

@aboktor yeah, I will look at this soon, we will merge it to the next release, don't worry

alexander-akait · 2023-04-22T16:54:29Z

Notes for me:

implement it for ImportMetaContextDependecyParserPlugin.js
use mapping/function style (becaue ESM import(...) doesn't support it and we can't implement this for ESM)
~~test with externals~~ our externals don't support prefetch/preload, so I will not implement fetchPriority too, but it is a potential improvement, but only for externalsType: "script"
webpackFetchPriority: false
less size of runtime generation
more tests to cover all cases

webpack-bot · 2023-04-22T20:43:17Z

@alexander-akait Thanks for your update.

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

@TheLarkInn Please review the new changes.

TheLarkInn · 2023-04-26T03:41:37Z

@alexander-akait can you fix the merge conflicts and I'd also like @aboktor's

aboktor · 2023-04-26T04:52:08Z

One comment on @alexander-akait's changes: Now the fetch priority is set per chunk at compile time. From what I see in the code, the chunk will pick a random priority (based on execution order of the code in ChunkPrefetchPreloadPlugin.js) from any of the ChunkGroups it belongs to.
Is my understanding correct here?

aboktor · 2023-04-26T04:54:27Z

lib/RuntimeGlobals.js

+/**
+ * function to return webpack's Trusted Types policy
+ * Arguments: () => TrustedTypePolicy
+ */


Inaccurate comment.

alexander-akait · 2023-04-26T10:52:37Z

One comment on @alexander-akait's changes: Now the fetch priority is set per chunk at compile time. From what I see in the code, the chunk will pick a random priority (based on execution order of the code in ChunkPrefetchPreloadPlugin.js) from any of the ChunkGroups it belongs to.
Is my understanding correct here?

~~It is not random, I just move all runtime code to own function and create an object like { "chunkId": "featchPriority" }, so we don't have extra runtime~~ I see what do you mean, we should use the featchPriority property from the first loaded async chunk (i.e. apply order), because another featchPriority doesn't make sense, chunk already loaded

TheLarkInn · 2023-04-26T14:09:42Z

Let's sort out this design for next Wednesdays release. Need to handle rebase conflicts in addition to make sure we are on the same page.

Zlatkovsky · 2023-05-02T17:21:38Z

lib/dependencies/ImportMetaContextDependencyParserPlugin.js

+									/** @type {ExpressionNode} */ (prop.value)
+								);
+								if (expr.isBoolean()) {
+									groupOptions.fetchPriority = "auto";


@aboktor , what is the purpose of allowing a boolean at all, when setting it (and regardless of the value) will result in "auto"? I would think just keeping it to the three enums -- "high", "low", "auto" -- would keep it both simpler and more understandable

I believe @alexander-akait added that code. I do see that pattern followed throughout webpack though.

Zlatkovsky · 2023-05-02T17:27:41Z

types.d.ts

@@ -5579,6 +5579,11 @@ declare interface JavascriptParserOptions {
 	 */
 	createRequire?: string | boolean;

+	/**
+	 * Specifies global fetchPriority for dynamic import.


@aboktor , I think it would be helpful to note the default value is "auto" (by which I assume it means that we leave it up to the browser's defaults); and also what a boolean value would mean.

(Though like I mentioned in another comment, I'm finding it hard to understand what true or false would mean in this case at all; do we need them?)

I agree, let's remove true, false requires to disable it, you can have a parent module with webpackFetchPriority: "high" and want to disable it for some deep modules, would say it's fine tuning, we use the same logic for any magic comments, also some libraries can use webpackFetchPriority comment too but you don't want it, so we allow to disable it

webpack-bot added the PR: unreviewed label Apr 14, 2023

aboktor commented Apr 14, 2023

View reviewed changes

.gitattributes Outdated Show resolved Hide resolved

alexander-akait reviewed Apr 15, 2023

View reviewed changes

aboktor force-pushed the fetchPriority branch from ebba962 to c1c41a9 Compare April 15, 2023 18:41

TheLarkInn requested changes Apr 15, 2023

View reviewed changes

declarations/WebpackOptions.d.ts Show resolved Hide resolved

webpack-bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Apr 15, 2023

TheLarkInn assigned TheLarkInn, alexander-akait and aboktor Apr 15, 2023

TheLarkInn mentioned this pull request Apr 15, 2023

Implement CSS Module fetchpriority #16999

Closed

aboktor force-pushed the fetchPriority branch from d9b5f32 to 968c3c5 Compare April 18, 2023 00:20

webpack-bot added PR: review-outdated and removed PR: reviewed-changes-requested labels Apr 18, 2023

aboktor force-pushed the fetchPriority branch from 968c3c5 to ffeba44 Compare April 18, 2023 23:41

webpack-bot added the PR: conflict label Apr 19, 2023

aboktor force-pushed the fetchPriority branch from f2567ef to eb8d11b Compare April 19, 2023 07:54

webpack-bot removed the PR: conflict label Apr 19, 2023

aboktor added 2 commits April 19, 2023 15:09

Adding option to set fetchPriority on script tags

eb2b7c3

Update snaps and stats

45d05dd

aboktor force-pushed the fetchPriority branch from 780b3a6 to 45d05dd Compare April 19, 2023 22:23

refactor: logic

0ad3e57

alexander-akait added 8 commits April 22, 2023 23:49

test: update

7c5af84

test: fix

81925c9

fix: import.meta.webpackContext

f8da6dc

fix: reduce runtime size

f819855

fix: false value

dec034b

test: update

2403946

test: more

1309fcc

test: more

22505dd

alexander-akait marked this pull request as ready for review April 23, 2023 00:36

alexander-akait added 3 commits April 23, 2023 03:40

test: more

71f117a

fix: reduce size more

56b855f

test: update

9a97b2a

aboktor commented Apr 26, 2023

View reviewed changes

Zlatkovsky reviewed May 2, 2023

View reviewed changes

alexander-akait mentioned this pull request May 23, 2023

feat: added fetchPriority #17249

Merged

TheLarkInn closed this in #17249 Jun 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch priority #16991

Fetch priority #16991

aboktor commented Apr 14, 2023 •

edited by alexander-akait

linux-foundation-easycla bot commented Apr 14, 2023 •

edited

webpack-bot commented Apr 14, 2023 •

edited by TheLarkInn

alexander-akait left a comment

TheLarkInn left a comment •

edited by alexander-akait

alexander-akait commented Apr 15, 2023

TheLarkInn commented Apr 19, 2023

aboktor commented Apr 19, 2023 •

edited

alexander-akait commented Apr 19, 2023

alexander-akait commented Apr 22, 2023 •

edited

webpack-bot commented Apr 22, 2023

TheLarkInn commented Apr 26, 2023

aboktor commented Apr 26, 2023

aboktor Apr 26, 2023

alexander-akait commented Apr 26, 2023 •

edited

TheLarkInn commented Apr 26, 2023

Zlatkovsky May 2, 2023 •

edited

aboktor May 2, 2023

Zlatkovsky May 2, 2023

alexander-akait May 5, 2023

Fetch priority #16991

Fetch priority #16991

Conversation

aboktor commented Apr 14, 2023 • edited by alexander-akait

Summary

🤖 Generated by Copilot at 0619c3e

Details

🤖 Generated by Copilot at 0619c3e

linux-foundation-easycla bot commented Apr 14, 2023 • edited

webpack-bot commented Apr 14, 2023 • edited by TheLarkInn

alexander-akait left a comment

Choose a reason for hiding this comment

TheLarkInn left a comment • edited by alexander-akait

Choose a reason for hiding this comment

alexander-akait commented Apr 15, 2023

TheLarkInn commented Apr 19, 2023

aboktor commented Apr 19, 2023 • edited

alexander-akait commented Apr 19, 2023

alexander-akait commented Apr 22, 2023 • edited

webpack-bot commented Apr 22, 2023

TheLarkInn commented Apr 26, 2023

aboktor commented Apr 26, 2023

aboktor Apr 26, 2023

Choose a reason for hiding this comment

alexander-akait commented Apr 26, 2023 • edited

TheLarkInn commented Apr 26, 2023

Zlatkovsky May 2, 2023 • edited

Choose a reason for hiding this comment

aboktor May 2, 2023

Choose a reason for hiding this comment

Zlatkovsky May 2, 2023

Choose a reason for hiding this comment

alexander-akait May 5, 2023

Choose a reason for hiding this comment

aboktor commented Apr 14, 2023 •

edited by alexander-akait

`🤖 Generated by Copilot at 0619c3e`

`🤖 Generated by Copilot at 0619c3e`

linux-foundation-easycla bot commented Apr 14, 2023 •

edited

webpack-bot commented Apr 14, 2023 •

edited by TheLarkInn

TheLarkInn left a comment •

edited by alexander-akait

aboktor commented Apr 19, 2023 •

edited

alexander-akait commented Apr 22, 2023 •

edited

alexander-akait commented Apr 26, 2023 •

edited

Zlatkovsky May 2, 2023 •

edited