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

Preloading modules with magic comments does not result in modulepreload injection #16838

Closed
sinemacula-ben opened this issue Mar 17, 2023 · 13 comments · Fixed by #17143
Closed
Assignees

Comments

@sinemacula-ben
Copy link

Bug report

Webpack magic comments do not inject the correct HTML syntax for preloading modules.

What is the current behavior?

Preloading a module results in the following HTML injection:

<link rel="preload" as="script" href="http://example.com/js/123.456789.chunk.mjs">

If the current behavior is a bug, please provide the steps to reproduce.
I believe this is a bug, but I am aware that it could be to do with my configuration...

My configuration is large so I will only include the bit that I think is relevant:

target: [ 'web', 'es6' ],
output: {
    filename: 'js/[id].[contenthash].mjs',
    chunkFilename: 'js/[id].[contenthash].chunk.mjs'
}

I also enabled experiments.outputModule and added module: true to the output, but I got errors with this, so reverted.

And in one of my files, I have the following line:

component: () => import(/* webpackChunkName: "about", webpackPreload: true */ '../views/About.vue')

What is the expected behavior?
I would expect the injected HTML to look like this instead:

<link rel="modulepreload" href="http://example.com/js/123.456789.chunk.js">

I may be mistaken here as obviously /* webpackPrefetch: true */ would not work with a module, so perhaps this is intentional, and module preloading is not meant to be supported. If this is the case, then I apologise - I did search the documentation and Google from top to bottom before posting here...

Other relevant information:
webpack version: 5.76.1
Node.js version: 19.7
Operating System: WSL 2 (Linux on Windows)
Additional tools:

@alexander-akait
Copy link
Member

What plugin do you use for HTML generation, I don't think webpack can solve it here

@knockkk
Copy link

knockkk commented Mar 20, 2023

import(/* webpackPreload: true */ 'ChartingLibrary');

When a page which uses the ChartComponent is requested, the charting-library-chunk is also requested via <link rel="preload">

Will the description of Preload in the document cause misunderstandings, making readers think that Webpack will do the injection job?

@sinemacula-ben
Copy link
Author

Hi @alexander-akait - I am using the HTML Webpack Plugin, however, I handle the injection manually using an HTML .ejs template as opposed to using inject: true.

Upon further inspection, I think I have misunderstood the documentation here. From what I now understand, the magic comments will provide Webpack context for how to inject the the script tags into the DOM upon evaluation, NOT compilation. The scripts inject upon compilation are the 'entry/initial' scripts, and then when the JS is evaluated, it will load any additional modules needed as and when required. For example, navigating to a new route, the JS will inject a new script tag into the DOM to load the module(s) necessary to run the route. The purpose of the magic comments is to advise Webpack whether to preload/prefetch these modules at entry point based on the likelihood they will be needed. Given modules cannot be 'prefetched', they can only be preloaded using modulepreload, the magic comments do not really apply to them...

So, with that in mind, I think this is more of a feature request than a bug report. This is how I would expect it to work:

See the following:

When a compilation is set to target: ['es6'], we handle the magic comments slightly differently:

This:

import(/* webpackPreload: true */ 'ChartingLibrary');

Will yield and injection of the following into the DOM:

<link rel="modulepreload" href="http://example.com/js/123.456789.chunk.js">

However, this:

import(/* webpackPrefetch: true */ 'ChartingLibrary');

Will either be ignored, or will yield the same as the above (I can't decide) - this will make this functionality backward compatible if people upgrade to ES6.

@alexander-akait
Copy link
Member

alexander-akait commented Mar 27, 2023

But we don't generate HTML here, all this needs to be handled and generated in html-webpack-plugin, prefetch and preload work as expected only for on-demand-loaded chunks.

Prefetch/preload doesn’t work in the entrypoint for me. What’s the problem?
The webpack runtime only takes care of prefetch/preload for on-demand-loaded chunks. When prefetch/preload is used in import()s in a entry chunk the html generation is resposible for adding the tags to the HTML. The stats json data provides information in entrypoints[].childAssets.

Ref: https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c

Example:

index.js

async function test() {
  await import(/* webpackPrefetch: true */ './module1.js');
}

setTimeout(() => {
  test();
}, 1000)

module1.js

console.log("1");

async function test() {
  await import(/* webpackPreload: true */ './module2.js');
}

setTimeout(() => {
  test();
}, 1000)

module2.js

console.log("2");

But I think what you are trying to preload in initial chunk, so it just doesn't make sense

@alexander-akait
Copy link
Member

alexander-akait commented Mar 27, 2023

Oh, I see a bug, when you have, (my bad, don't fully undestand the issue, sorry):

experiments: {
  outputModule: true
}

We generate:

<link type="module" charset="utf-8" rel="preload" as="script" href="/dist/src_module2_js.bundle.js">

But we need to generate:

<link rel="modulepreload" href="http://example.com/js/123.456789.chunk.js">

@alexander-akait
Copy link
Member

Do you want to send a fix, it should be easy https://github.com/webpack/webpack/blob/main/lib/web/JsonpChunkLoadingRuntimeModule.js#L253

@alexander-akait
Copy link
Member

@snitin315 There is a good issue, we just need to fix:

<link rel="preload" as="script" href="http://example.com/js/123.456789.chunk.mjs">

to

<link rel="modulepreload" href="http://example.com/js/123.456789.chunk.js">

logic is here https://github.com/webpack/webpack/blob/main/lib/web/JsonpChunkLoadingRuntimeModule.js#L253

@snitin315
Copy link
Member

@alexander-akait yes, can you assign this issue to me? Otherwise, I'll forget.

@alexander-akait
Copy link
Member

Done 👍

@snitin315
Copy link
Member

@alexander-akait We need this only when output.module: true right? Also, how do I add a test case for this?

@alexander-akait
Copy link
Member

@snitin315 Yeah, I think we already have test case for this, can you check?

@snitin315
Copy link
Member

@alexander-akait We have for CJS

// Test preload of chunk1-b
link = document.head._children[2];
expect(link._type).toBe("link");
expect(link.rel).toBe("preload");
expect(link.as).toBe("script");
expect(link.href).toBe("https://example.com/public/path/chunk1-b.js");
expect(link.charset).toBe("utf-8");
expect(link.getAttribute("nonce")).toBe("nonce");
expect(link.crossOrigin).toBe("anonymous");
// Run the script

@alexander-akait
Copy link
Member

@alexander-akait Yeah, just copy this test and set experiments.outputModule and check that we have right attributes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Shipped
Development

Successfully merging a pull request may close this issue.

4 participants