Hi @maiieul, thank you for bringing this to our attention and suggesting a fix. I think you are right in so far as this would be possibly a breaking change for some users. I agree, though, that for nearly all situations where a user supplies a function for manualChunks, your approach is more correct.
The reason it works like this is that originally, we only had the object form API where you basically designate some "entry points" for your manual chunks. For most use cases, your change would make the object form of manualChunks unusable.
In the "function" form, however, your behavior is much closer to expectations I think, i.e. you manually take a few modules out of chunk assignment but let Rollup handle the rest.

Here would be a suggestion how to move forward:

• We make the behavior I outlined the way the next major version of Rollup will work, i.e. when an object is used, we merge dependencies into the chunks, otherwise we do not. One way to do that is to start here

  rollup/src/Bundle.ts

  Lines 170 to 174 in 298609e

  	const manualChunkAliasByEntry =
  	typeof manualChunks === 'object'
  	? await this.addManualChunks(manualChunks)
  	: this.assignManualChunks(manualChunks);
  	const snippets = getGenerateCodeSnippets(this.outputOptions);

  where we specifically know which form was used. We could for instance store this in a flag that we pass to getChunkAssignments to control which version is used.
• To allow a gradual transition, we add a temporary new option e.g. onlyExplicitManualChunksAssignments that we remove again in the next major version.
• We also need to adapt the documentation for manualChunks and add some basic documentation for the new option.

Let me know if you would be willing to perform these changes and if you need any additional help.

Hi @lukastaegert, thanks for the prompt reply!

I must say I had not thoroughly checked the snapshot results and the object form was broken indeed 😄.

Let me know if you would be willing to perform these changes and if you need any additional help.

Happy to give it a go! In fact, I already started diving into it 😁

We make the behavior I outlined the way the next major version of Rollup will work, i.e. when an object is used, we merge dependencies into the chunks, otherwise we do not.

I dug a little deeper into the object vs function form and found out that there are quite a few discrepencies between the two. To me as a consumer of Rollup, I would find it more intuitive if both behaved similarly. If we go down the path of merging dependencies into the chunks only when the object form is used, the discrepencies become even bigger, because now we have a "merge deps" behavior with the object form, and a "let Rollup handle the rest" with the function form.

I explored a way to make both APIs consistent, by converting the object form to a function and always use the new function form code path of assignManualChunks for both.

The idea is that when the developer passes

    manualChunks: {
      vendor: ['lodash-es', 'clsx'],
    },

Rollup then converts it to

    manualChunks: (id) => {
      if (id.includes('lodash-es')) return 'vendor';
      if (id.includes('clsx')) return 'vendor';
    },

and then always uses the assignManualChunks logic, without the addStaticDependenciesToManualChunk merge logic.

Doing this, both forms work exactly the same way, which means:

• No surprises to the developer when switching between one form to the other.
• The code to get the manualChunkAliasByEntry and later on getChunkDefinitionsFromManualChunks becomes quite a bit simpler.
• The object form no longer automatically merges dynamic deps or handle "non-inlcuded" manualChunks and other edge-cases. It is up to the developer to precisely tell Rollup how to group the manualChunks together.
• The two forms have the same algorithmic efficiency. I don't know if the object form was supposed to be faster than the function form. I have a feeling that removing the recursion in addStaticDependenciesToManualChunk should be faster but I might be wrong. I didn't run any benchmark on this. If the object form was faster, would you see the normalization of both forms as a problem? In my experience with Qwik apps, manualChunks in the function form is fast and never the bottleneck, even with a lot of conditional logic.

I went ahead and implemented the logic for you to see. I added a comment to each test (either on _config.js or _expected/es/...), explaining the new behavior and why the new output makes sense to me.

(I sent you a message on Discord in case you would like to go over it in a call)

Looking forward to hear from you. Once we are aligned on the code changes I will proceed with implementing the new onlyExplicitManualChunksAssignments option to be removed in the v5 release and updating the docs.

I agree with your idea and I actually like it. In this spirit, the array form could even be extended to support both strings and regular expressions, making it truly powerful.

With regard to the test changes:
Many tests that use manual chunks specifically rely on certain modules to be merged. So by default, I would rather change the manualChunk config in those tests so that the same merges take place than to change the expected generated output.

As for the way forward, progress is unfortunately slow with Rollup and I cannot promise a timely new major release. If you want the feature quickly, how about we add a flag e.g. "onlyExplicitManualChunks" or something that switches to the new algorithm until we do the full switch in the next major version? I would be fine with updating the tests already to specifically set the flag and just keep one legacy test for the old logic.

Really nice, thanks a lot. Only some minor comments, then we can release this!