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

Opt-in / opt-out of module preloading #5991

Closed
4 tasks done
AlexandreBonaventure opened this issue Dec 7, 2021 · 5 comments · Fixed by #9938
Closed
4 tasks done

Opt-in / opt-out of module preloading #5991

AlexandreBonaventure opened this issue Dec 7, 2021 · 5 comments · Fixed by #9938

Comments

@AlexandreBonaventure
Copy link
Contributor

Clear and concise description of the problem

Hey there!
Recently came across a problem where we need to load some mocks or test files conditionally, like this:

Screen Shot 2021-12-07 at 11 40 41 AM

This is all well and good but vite is injecting modulepreload links for those. Generally speaking this is a good default behaviour, however in this case I would really like to opt-out of preloading because:

  1. it loads unecessary js files
  2. these files are meant to run code outside of any closure. so code execute as soon as module is preloaded

I know this issue (#5120) is asking for a somewhat similar request but I think the 2 proposal are valid and different:
The above ticket needs to disable the feature altogether, whereas this ticket is more about having the ability to filter which files can be preloaded.

One can argue that the import should be stripped anyways
and that if (USE_INTEROP_MOCK) should be if (import.meta.USE_INTEROP_MOCK) but we actually sometimes need this code in production to enable debug mode capabilities (like a feature-flag)

Suggested solution

Couple of ideas:

  1. Mark async depdencies as modulepreload #2835 (comment)
    This suggest using an api like import.meta.preload() to opt-in for preloading

  2. Support comment directive like import('/interop/mock/index' /*@vite-preload-ignore*/);

  3. Maybe a config ? I am not a big fan of this one...
    like build.preloadExclude: glob

Alternative

No response

Additional context

No response

Validations

@AlonMiz
Copy link

AlonMiz commented Jan 31, 2022

currently, i suffer from this default config.
Part of our bundle is still using angular.js, which is not imported by default unless the user lands on an angular route.
angular and jquery are being imported with "modulepreload" but the angular is not initialized correctly as the order of import is crucial, angular waits for window.jQuery to be defined, and in that case, it's not. so angular is wrongly initialized.
currently, there is no other way to confront this. and I've tried several methods.
we have no other choice but to somehow disable the default appending of all bundles (even by greping the link tags and removing them)
this is a cool feature that's increasing performance by default, but that's blocking us from migrating to vite in prod for a large project.

@jpvincent
Copy link

Hi
I opened the other ticket (#5120) about disabling module preload. I understand the proposal here is to be more precise than just a switch, but I cant add a use-case for that.

However I can do a follow-up after we stopped using modulepreload altogether in production, 1 week ago.
Please find a comparison test here : https://www.webpagetest.org/video/compare.php?tests=220401_BiDcHQ_46P%2C220329_BiDcNP_ANH&highlightCLS=1&highlightLCP=1&thumbSize=200&ival=200&end=visual
Visually, it gives this :
image

What went better :

  • we display the page much sooner (FCP)
  • we display the main image sooner (LCP)

It comes at a cost : the whole JS application executes later. You can guess it from the screenshot above by taking a look at the player button in the footer.

That was confirmed with figures coming from our real users after one week into production : here are the values for the First Contentful Paint going down on some pages we monitor, as seen from the Chrome Report User eXperience and reported from our chrome users to Google Search.
image

So, if you are not using SSR correctly, not developing the progressive enhancement way or are doing a real SPA that fully relies on JS, the default modulepreload is a good choice.
If your site is visually OK during the loading phase and can wait a few second for JS to execute, like ours, disabling modulepreload, be it fully or on a per-file basis is better for performance.

@spacedawwwg
Copy link

spacedawwwg commented Jun 30, 2022

For anyone interested, until this is resolved within Vite itself. This is how I strip the module preloads (and image preloads) from my HTML AFTER a Vite build has run.

NOTE: I am using vite-plugin-ssr, though the below is a good starter.

/* package.json */
{
  scripts: {
    "build": "yarn build:vite && yarn build:post-build",
    "build:vite": 'vite build --ssrManifest && vite build --ssr',
    "build:post-build": 'node -r ts-node/register scripts/postBuild'
  }
}
/* scripts/postBuild.ts */

import consola from 'consola';
import fs from 'fs-extra';
import glob from 'glob-promise';
import path from 'path';

// relative to package root
const root = path.resolve(__dirname, '../../');
const dist = path.resolve(root, './dist/client');

export function removePreloads(html: string) {
  return html.replace(
    /<link .?rel="(preload|modulepreload)" .*?href="(?<href>.+?\.(woff|svg|png|jpg|webp|gif|js))".*?>/g,
    ''
  );;
}

export async function transformHtml() {
  try {
    const files = await glob('**/*.html', { root: dist });

    await Promise.all(
      files.map(async (file: string) => {
        try {
          let html = await fs.readFile(file, 'utf-8');

          html = removePreloads(html);

          await fs.outputFile(file, html);
        } catch (error) {
          consola.error(error);
        }
      })
    );

    consola.success(`${files.length} HTML files transformed successfully.`);
  } catch (error) {
    consola.error(error);
  }
}

transformHtml();

@AlonMiz
Copy link

AlonMiz commented Jul 2, 2022

cool, would be nice if that's going to be a config for each chunk or a global config for all files.
instead of hacking the files after the build is done.

@patak-dev
Copy link
Member

@AlonMiz @jpvincent @spacedawwwg @AlexandreBonaventure would you check if your use cases would be covered by the feature as implemented in:

@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants