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

fix(optimizer): support new Worker and new URL(..., import.meta.url) for pre-bundled dependencies #17837

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

hi-ogawa
Copy link
Collaborator

@hi-ogawa hi-ogawa commented Aug 8, 2024

Description

related #16418, #8427, #11672

I'm exploring new URL and new Worker support for pre-bundled dependencies as a user land vite plugin (and mostly as a standalone esbuild plugin) in https://github.com/hi-ogawa/vite-plugins/tree/main/packages/pre-bundle-new-url. From what I tested on various examples, this seems to cover well known scenarios, so I'm starting with this draft PR to discuss whether this could be moved to core.

The approach is based on 3rd one suggested in #16418 (comment). This intercept onLoad hooks for all js files, so performance impact is one concern (there are other concerns too and I wrote them as TODO in the code).

Some examples I tested are:

I think the main question to ask first is whether we need to rash into supporting this on top of current esbuild. Maybe would it be better to suggest and test user land workaround (either simply optimizeDeps.exclude, which users already see as a warning message, or my plugin) and wait for a bundler level support by esbuild or rolldown?

Also regardless of whether waiting for esbuild or rolldown, it's probably important to guide their implementation so that it'll be directly usable for Vite. I made a bundler comparison table in https://github.com/hi-ogawa/reproductions/tree/main/webpack-new-url-worker-bundle-or-copy and it seems esbuild's future implementation (though it could be stale PR) might not match well with what Vite needs. If anyone has suggestions for what we can do right now, I'm curious to hear.

Copy link

stackblitz bot commented Aug 8, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

const subUrl = new URL('./sub.js', import.meta.url)

export default () => import(subUrl)
export default () => import(/* @vite-ignore */ String('./sub.js'))
Copy link
Collaborator Author

@hi-ogawa hi-ogawa Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is now compatible, so I made it more "incompatible" for the tests added in #16080

@hi-ogawa hi-ogawa marked this pull request as ready for review August 14, 2024 08:16
@hi-ogawa hi-ogawa changed the title feat(optimizer): support new Worker and new URL(..., import.meta.url) for pre-bundled dependencies fix(optimizer): support new Worker and new URL(..., import.meta.url) for pre-bundled dependencies Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant