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

Omit __vitePreload if dependency array is empty #13952

Open
4 tasks done
kwangure opened this issue Jul 26, 2023 · 6 comments
Open
4 tasks done

Omit __vitePreload if dependency array is empty #13952

kwangure opened this issue Jul 26, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@kwangure
Copy link

Description

Vite provides modulePreload.resolveDependencies to give fine-grained control over module preloading. However, when I exclude module dependencies, it still imports the preloader. This is a feature request not to render the preloader at all if the dependency list is empty.

This has the additional benefit that if no preloading happens, I don't have to pay the price for the preloading script.

I need this for a Vite plugin generating content scripts for browser extensions. Content scripts are non-modules and thus don't allow the import func from 'file' syntax. Additionally, since files are local, the waterfall without preloading is negligible.

Suggested solution

Given:

modulePreload: {
	resolveDependencies: (url, deps, { importer }) => {
    	return [];
    }
}

Instead of:

import { __vitePreload } from "./preload-helper.js";
__vitePreload(() => import("path/to/file.js")), true ? [] : void 0);  // note empty preload list

output:

import("path/to/file.js"));

Alternative

I considered creating a plugin, but Vite runs built-in "enforce": "post" plugins last after user "post" plugins...so I can't override the import within the transform hook.

Additional context

This is essentially a follow up of #5991 and #9938. This is a common pattern (#8023 (comment)) in browser extensions.

Validations

@jaswrks
Copy link

jaswrks commented Nov 30, 2023

➕ 💯 for this. Incredibly frustrating to set modulePreload: false and then find that every dynamic import is still wrapped with __vitePreload() calls. It is doubly frustrating when you realize it’s basically dead code added to your bundle by the build tool that is supposed to be helping you optimize things. To make it worse, if you have dynamic imports and CSS dependencies, those get injected as pseudo-preloads, of some sort, which is essentially Vite not honoring the modulePreload: false option at all, really.

@yzy415
Copy link

yzy415 commented Mar 28, 2024

any updates regarding this issue? It's frustrating to have this polyfill in web extensions.

@bluwy
Copy link
Member

bluwy commented Apr 10, 2024

Vite currently can't omit the __vitePreload because the dependency array can only be retrieved during the generateBundle phase (after Rollup has chunked everything). So Vite can only preemptively guess that a dynamic import might need preloading and inject it before Rollup chunks things.

While Vite could maybe walk back and remove the __vitePreload text altogether (using some form of regex), that also has caveats too like extra work needed to re-ensure sourcemaps are correct. I think leaving it alone is still fine as __vitePreload only has a small overhead for empty deps.

If the idea is to have no preloading at all then modulePreload: false would be the option. However as noted above, we can't completely avoid preloading if CSS deps is involved, otherwise your application styles will be broken. To prevent that, I think disabling build.cssCodeSplit should work.

So altogether, I'm not sure if there's anything else we can do here.

@nyanrus
Copy link

nyanrus commented Apr 10, 2024

Currently to use vite on WebExtension, the public dir can be useful.
the way is importing vite-processed source in public dir source.
and register the public dir source to the content_script.
because the source in public dir be just copied to dist and don't bundled by vite.

@kwangure
Copy link
Author

Yes, the overhead of the empty array is negligible. The key issue though as mentioned in the original post is that there are some environments where specific files 1) need dynamic imports 2) cannot be modules. (e.g. browser extension content-scripts).

I can hack around to remove the __vitePreload using a regex and magic-string during BUILD in later rollup hooks. The challenge is that in DEV mode transform is the last hook before Vite sends files to the browser and it runs its code to add the __vitePreload in a post hook that runs after my plugin. There's simply no opportunity to undo that __vitePreload in a user Vite plugin in DEV mode because of plugin ordering.

How do you feel about "enforce": "post-post" for user plugins? 🙈

PS: In writing this it occurred to me that I might be able remove the __vitePreload by adding a service-worker that overwrites the module, though I'm not sure about the viability of that. Either way, I understand that it's challenging given the constraints of your integration with Rollup, but it still feels like something that could/should somehow be resolved in Vite rather than hacked around in user land.

@bluwy
Copy link
Member

bluwy commented Apr 11, 2024

some environments where specific files 1) need dynamic imports 2) cannot be modules.

I don't quite understand this. Dynamic imports only work for modules, if it can't be modules, it's conflicting the idea.

The challenge is that in DEV mode transform is the last hook before Vite sends files to the browser and it runs its code to add the __vitePreload in a post hook that runs after my plugin.

You can have a transform hook like this:

{
  transform: {
    order: 'post'
    handler() {}
  }
}

The order (supported by Rollup) re-arranges the hooks after enforce re-arranges the plugins.

it still feels like something that could/should somehow be resolved in Vite rather than hacked around in user land.

I think this depends on Vite's target audience focus, which is mainly browser apps, SSR, backend integrations, etc. Developing web extensions isn't part of the original goal so requiring workarounds seems fair to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants