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.

Sign up for GitHub

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

Jump to bottom

feat: add configuration option to disable automatic "modulepreloads" #7766

Closed
wants to merge 1 commit into from
Closed

feat: add configuration option to disable automatic "modulepreloads" #7766

wants to merge 1 commit into from

Conversation

mariohamann
Copy link

@mariohamann mariohamann commented Apr 16, 2022 •
edited

Description

When being not in lib-mode a script is injected in the main js-file which automatically adds chunked files to the html <head> and links them with <link rel="modulepreload"> or <link rel="preload">.

The following line is responsible for the injection:

vite/packages/vite/src/node/plugins/importAnalysisBuild.ts

Line 94 in 9a93233

const preloadCode = `const scriptRel = ${scriptRel};const seen = {};const base = '${preloadBaseMarker}';export const ${preloadMethod} = ${preload.toString()}`

In most cases this behaviour is fine, but can lead to problems in combination with glob importing:

  1. I'm creating a Web Components library which is used from other domains. I use glob importing to import the components (and maybe will add some lazy loading). Nevertheless the script adds lots of <link rel="modulepreload">s to the recipients <head> – which leads to overhead in the main.js I don't need and want (about 2-3kb uncompressed) AND failing fetches as the links link relatively to the base of the url.
  2. When creating a static website within the same repo, using the same Web Component library, this leads to doubled fetches in Safari and FireFox.

The only possibility (which apparently is being used!) is ugly RegEx/replacement to remove the problematic and in that case unneded code after build time.

This PR introduces a build option to completely disable the injection of the script.

The following issues could benefit from this PR:

Additional context

I didn't add a test – it would be great, if someone with more experience in Vite-development could help out with that! ❤️

What is the purpose of this pull request?

  • Bug fix
  • New Feature

(I'm not sure whether this is more a bug fix or a feature. 😄)

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@seivan
Copy link

seivan commented May 8, 2022

Have the same issue, glad to see this here.
Default opt into preloads is a bad idea since it makes the assumptions that the preloaded modules are isolated.

What happens if you preload react and <some_thing that depends on react>
where the latter finishes and executes before?

Something like

if (!react.exports.useState) {
    throw new Error("mobx-react-lite requires React with Hooks support");
}

@distantnative
Copy link

Would be great to see this merged - currently there seems to be no way to prevent vite inserting modulepreload tags that feature a wrong relative path (due to CDN setup). And that's rather bad that there is no way to fix this for us.

@hieudo-dev
Copy link

@mariohamann I was checking out some issues related to this PR and I suspect the current changes might not address this issue that was talked about here #3133 (comment). As of now i think staticly imported css will only be loaded by module preloading.

@seivan
Copy link

seivan commented May 12, 2022

Another issue is that <link rel="modulepreload"> isn't supported by all browsers, so you still gotta write code to "preload" lazy components. Could be anything from filling up a form triggering a lazy load to hover over the signing button.

@patak-dev patak-dev added p2-to-be-discussed Enhancement under consideration (priority) and removed enhancement: pending triage labels May 12, 2022
@patak-dev patak-dev mentioned this pull request May 12, 2022
Merged
4 tasks
@sapphi-red sapphi-red mentioned this pull request Sep 9, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
p2-to-be-discussed Enhancement under consideration (priority)
Projects
Team Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mariohamann @seivan @distantnative @hieudo-dev @patak-dev @sapphi-red