refactor: avoid splitting vendor chunk by default

[patak-dev]

Fix #6250
Fix #3731 
Close #6318
Close #6347
Close #5585

Related #6289 #5487 #4890 #2672 #1703

Description

See @QiroNT’s discussion thread, and @benmccann proposal to remove the split vendor chunk strategy by default in Vite Core and leave each framework to decide what is best.

We agree that the current strategy isn't the best option for the general case, and it is better to leave the user or framework to choose the best strategy for their app.

This PR removes the current manualChunks strategy, and exposes it both as a function and as a plugin in case users would like to continue using it. Later, Vite Core could provide other strategies for specific cases if we find them general enough to include.

This PR also fixes a build watch issue, as the split vendor chunk cache wasn't being reset in buildStart. @ygj6, thanks for your work on #5585. I ended up with a different API because we don't want to have the cache logic in Core now.

Because of the needed cache reset and the conditionals needed to decide if the manualChunks strategy should be applied, giving the user a single function for this strategy makes it difficult for them to get the same chunking as in Vite 2.7. A splitVendorChunkPlugin plugin is provided that resets the cache properly and applies the exact same logic we had before.

I can add docs for this in case the API is approved, but I don't know how much we want to promote this strategy.

@benmccann @QiroNT @sanyuan0704 @ygj6 let me know if you have any feedback, I think we could include this in 2.8

Additional context

See linked issues and PRs for additional context, there are issues against SvelteKit also listed there.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[benmccann]

Thanks! I have no objection to documenting/promoting the plugin or continuing to improve it. I do think it's better not to have it on by default though like this

[aleclarson]

This class seems unnecessary. The Map object has a clear method.

[patak-dev]

I don't know if we will need to expand the cache later. I wanted to expose the cache for the method, not a particular Map that is an implementation detail and may change

[aleclarson]

See comments

[aleclarson]

Idk what the use case for custom reset is, but we could add an onReset callback instead of using a custom class.

[patak-dev]

The cache used by splitVendorChunk needs to be cleaned up on every buildStart. I don't understand the onReset callback, how would that work?

[aleclarson]

So the plugin would call cache.clear on the Map object and then call onReset callback after that in case there's any custom logic needed. But as I said, I have zero idea why anyone would need custom reset logic here. Perhaps you have an idea? 😄

[aleclarson]

Oh here's a use case. A plugin that wraps the splitVendorChunk plugin may need to be reset its own cache.

function extraChunkPlugin() {
  const anotherCache = new Map()
  return splitVendorChunkPlugin({
    manualChunks() {
      // Insert custom chunking logic.
    },
    onReset() {
      anotherCache.clear()
    }
  })
}

This example needs manualChunks and onReset options to be added to splitVendorChunkPlugin though. Not sure if this is what you had in mind for the reset method though.

[patak-dev]

If the user is already doing a new plugin, I think it is better that they use the function splitVendorChunk and they can reset their caches at the same time as resetting the SplitVendorChunkCache (on buildStart). I don't think we need to have an API for the plugin. The plugin is there so users can add in a single line the old strategy, and we are exposing the low level function because it was requested several times before to compose it with other logic.

[aleclarson]

Okay that's fair, but I'm still confused why the SplitVendorChunkCache class is necessary if you can just call cache.clear on the Map object.

[aleclarson]

Oh right, you said:

I don't know if we will need to expand the cache later. I wanted to expose the cache for the method, not a particular Map that is an implementation detail and may change

That doesn't really make sense to me. If we decide not to use a Map in the future (not sure why we would tbh), then we can handle that then.

But if you really believe the cache class is the right move, I'll defer to your judgment, since this isn't a huge deal or anything. :)

[patak-dev]

It just feels to me that we are leaking an implementation detail if we share the Map. In this case it isn't even something useful like a map of modules, it is an internal cache used by the algorithm. I also don't like having to create a class, but it feels safer here to avoid the need for a breaking change.

[antfu]

I am not so sure about the interface of having a separate plugin for that, as the other parts of Vite are all through configurations. Do you consider this a feat and maybe bring it up to the next meeting?

[patak-dev]

I was also hesitant to add the plugin, but if you see its implementation, if we only provide the low-level manualChunks function it isn't straightforward at all for users to replicate what we currently offer in v2.7. But yes, we need @yyx990803's input here.

@antfu what do you think about removing the function and the plugin from core directly? I (or someone else) can create a package with it so we don't place this burden on core forever?

[antfu]

I would imaging something like

export default defineConfig({
  build: {
    chunksOptions: {
      // the options of the plugin
    }
  }
})

Then we do this internally

if (config.build.chunksOptions) {
  plugins.push(splitVendorChunkPlugin(config.build.chunksOptions))
}

[patak-dev]

I don't know if Vite core is going to end up offering different chunking strategies in the future. We don't have enough information, this looks like a problem better solved by each framework.

But if we do end up having a set of strategies for common cases, that we can document, I agree with you that an API like you propose is a good way to go. I don't think we will have this for v2.8 though. The API could also be informed by these strategies. What about leaving some time to the ecosystem (frameworks and user plugins) to come up with strategies and we then put in core the ones that everybody is using?

I currently see this refactoring as a bug fix more than a feat. Our current strategy is causing problems, so it is better to remove it.

[ygj6]

Thanks, that's acceptable to me,

[spacedawwwg]

Will this allow the extraction of CSS from SSR rendered async components to be included at build time?

Currently only the HTML is pre-rendered, but the CSS feels like a critical element (especially for users with javascript disabled... the component is async for JS loading in my example)

[patak-dev]

See ##6681 for an alternative moving the new plugin and function to a new monorepo package, and out of core.

[QiroNT]

LGTM

tho' should we mention that this plugin overwrites the manualChunks option?

[patak-dev]

tho' should we mention that this plugin overwrites the manualChunks option?

Interesting question. About the way it works, if there are both manualChunks and this plugin it isn't that clear what should win. Maybe the vite.config.js option should still win over the plugin so users are able to change one of the outputs by hand and leave the rest to be set by the plugin?

[patak-dev]

We discussed and decided to move forward with this PR in 2.9 (see alternative #6681 which is now closed).

23440e9 adds support for composing user manualChunks with Vite split vendor chunks. This only is supported for the function form of manualChunks. I tried to support the object form, but it requires access to the module graph and I don't believe that the complexity justifies adding support for it. For the function form, because we are splitting the whole node_modules, adding vue wouldn't have any effect, but users could use composition for splitting their own source code.

My take is that this new plugin remains as a way for people to opt-in to the old chunking strategy but frameworks are the ones that have enough knowledge to offer a good default to users.

[souljorje]

When it's gonna be released? 👀

[patak-dev]

Probably in one or two weeks. But a 2.9-beta.0 is going to be released later today or tomorrow so you can start testing it soon

[jamesthomsondev]

I may have missed it in the comments above, but I wasn't sure exactly how to return back to splitting vendor as a chunk. After a bit of searching, found it in the guide under "Building for Production". Hopefully that's useful for anyone landing on this thread.