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

feat: optimize custom extensions #6801

Merged
merged 8 commits into from
Mar 3, 2022

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Feb 7, 2022

Description

Add optimizeDeps.extensions: string[] to pre-bundle custom extensions. A respective esbuild plugin is required to handle that extension. e.g. ['.svelte', '.svelte.md'].

Additional context

A corresponding branch in vite-plugin-svelte is created that's based on this PR, under the flag experimental.prebundleSvelteLibraries. You can try it out as well in vite-plugin-svelte's big-component-library playground, after linking Vite locally.

I've also tested this on a production app and it's working nicely so far. Pre-bundling time is up slightly, but browser load time is noticably faster.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

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.

@bluwy bluwy marked this pull request as draft February 7, 2022 17:30
@bluwy bluwy added p2-nice-to-have Not breaking anything but nice to have (priority) YAO Yet another option... labels Feb 7, 2022
@bluwy bluwy marked this pull request as ready for review February 10, 2022 07:38
@patak-dev
Copy link
Member

This looks good, I'll add it to be discussed in the next team meeting.

I've also tested this on a production app and it's working nicely so far. Pre-bundling time is up slightly, but browser load time is noticably faster.

The extra pre-bundling time should be less noticeable if we get to merge #6758, and make the dep optimization processing non-blocking.

@patak-dev
Copy link
Member

@bluwy we talked about this PR in the last team meeting. This is a feature that we want to support in Vite. Not only it will benefit svelte, but as you said it could be used by Vue and other frameworks to do the same.

We would like to explore an alternative approach though, to be able to avoid forking the pipeline and require dual implementations of each plugin (svelte, vue, etc). The ideal would be that the same Vite/Rollup plugins are used for deps, as we currently do when we exclude them from pre-bundling. Evan and Anthony proposed to see if something like unplugin could work here, wrapping the normal plugins to make them compatible (at least for what is supported) with esbuild. So for users, this will be as transparent as possible, and we also are more abstracted from esbuild in the case at some point we decide to switch to a different tool.

@bluwy
Copy link
Member Author

bluwy commented Feb 15, 2022

Thanks for looking into this! Yeah the PR now is definitely not the long-term solution, so an alternative approach is certainly welcomed. I've thought about some unified approaches too, but ultimately it comes down to intricacies of handling CSS, prebundling invalidation, and special prebundling logic that would need opts.prebundling etc (besides opts.ssr). If we want to go even further, opts.scanning can be added for the scanning phase to generalize it. Overall, I think it'll be a lot of work to get it right, and quite possibly a v3 thing.

I still hope that this PR can be considered though, at least under the experimental status as it's mostly additive. I can find some time to look into the above ideas too, but I'm hoping the Svelte ecosystem can get an improved prebundling experience while we're at it.

@patak-dev
Copy link
Member

Lets talk about it again in the next team meeting. Marking it as experimental sounds like an option we should consider

@bluwy
Copy link
Member Author

bluwy commented Feb 25, 2022

From the last team meeting, we've agreed to move forward with this. The optimizeDeps.extensions config would be kept experimental, while we figure out a long term solution to integrate Vite plugins in the esbuild pre-bundling flow (#4921). Plugin authors can experiment with this option as well at the meantime.

@bluwy bluwy added this to the 2.9 milestone Feb 25, 2022
@patak-dev
Copy link
Member

Started a discussion to gather feedback so we can move this feature out of experimental:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority) YAO Yet another option...
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants