[PP-1] Allow setting aggregation groups for js files in library definitions

I'm spending some time with Drupal\Core\Asset so I can provide an informed opinion on this. Even if it's just a "looks good!", it'll be an informed "looks good!". Although I'm still familiarizing myself with everything Asset, a few things come to mind:

1. There's a default value that can be overridden by a config setting. This should have test coverage, perhaps something analogous to \Drupal\Tests\Core\Asset\CssCollectionGrouperUnitTest::testGrouper? Clearly there's some missing coverage as the the MR changes functionality but all tests continue to pass
2. The aggregation itself should be tested too. Looks like that may be a reasonably simple addition to \Drupal\KernelTests\Core\Asset\AttachedAssetsTest::testAggregation? Aside from basic coverage, this seems additionally important as there are other chunks of code dealing with JS Assets that may be assuming JS does not have configurable groups. (\Drupal\Core\Asset\AssetResolver::getJsAssets seems like it may do this, but tests would prove/disprove it). Such a test would essentially confirm it will work for CKEditor 5 so we don't have to open additional followups while working on #3224261: [PP-2] Front-end performance: concatenate CKEditor 5 JS assets for every unique configuration
3. I don't have a strong opinion on whether or not this should be considered, but it merits mention: Before group was un-API'd, it was understood that its value informed the loading weight. This re-introduction of group is only for aggregation purposes, so perhaps the config property could be named something different likeaggregation_group? Even though the value ultimately gets assigned to group in the stored config, the outcome is different from what group previously did.

+1 to the remarks in #4.

I think this basically means that \Drupal\Core\Asset\AssetCollectionGrouperInterface::group()'s implementation freedom becomes more restricted — but the existing interface and API does not specify very strictly (or very clearly…) the existing responsibilities and freedoms either. (Mea culpa!)

An even better approach? Perhaps too abstract and too much magic?

More generally, I am not sure this change is necessary. We could also choose to instead make the existing grouping logic smarter. For example, if a particular asset library (dependency) subtree is tightly related, then we could choose to put that in a group of its own. See https://en.wikipedia.org/wiki/Component_(graph_theory). That is tricky though: where does one draw the boundaries?
So … I think I agree that this change is probably the simpler (and "less magical") choice.

But I still wonder if there's a better approach here. In the end, this declaring of some group X on a bunch of asset libraries is essentially the same as what I just described in the previous paragraph: marking the assets in a particular asset library subtree — i.e. those with strong connections to each other and weak (or usually no) connections to other asset libraries.

I wonder if we should instead declare the starting point (i.e. the root) of a particular asset subtree as deserving its own group/aggregate? 🤔 But that too leaves the "where does one draw the boundary?" question 😬 And again would involve some level of magic.

Stepping stones towards a better approach?

To get more concrete instead of describing these abstract ideal states: I wonder what the output would be for a script that analyzes the entire list of Drupal core's asset libraries and lists all asset libraries that: A) are leaves (have no dependencies), B) have >1 dependents. For this list, sort them by most dependents to least, and show the dependents count.

For ckeditor5/ckeditor5 (or when it's in core: core/ckeditor5), it would be listed as one of the topmost of these, and would have dozens of dependents.

I think this information might lead us to see a way forward to a better approach?

And the relative order of the groups instead of going with weights, we let the developer declare the order they want the aggregates in, like the css layers?

The order should really be determined by library dependencies.
Which would mean we need #1945262: Replace custom weights with dependencies in library declarations; introduce "before" and "after" for conditional ordering first.

There would then still be a possibility of someone declaring groups that are incompatible with dependencies:

library 1 - group 1

library 2 - depends on library 2, group 2

library 3 - depends on library 2, group 1

For this case, there's a couple of options:
1. Validate it (during discovery?) and throw an exception/log an error. Ignore the group. I think this would be good if we can do it.

2. Create three aggregates (messy):
Aggregate A - group 1
Aggregate B - group 2
Aggregate C - group 1

1. Validate it (during discovery?) and throw an exception/log an error. Ignore the group. I think this would be good if we can do it.

+1

If I'm wrong change it back, but I think this should be postponed on #1945262: Replace custom weights with dependencies in library declarations; introduce "before" and "after" for conditional ordering.

Ohhhhh, long time no see, #1945262: Replace custom weights with dependencies in library declarations; introduce "before" and "after" for conditional ordering, but that literally came up earlier today in a meeting with the CKEditor 5 team, as a more elegant solution to a tricky plugin loading problem that @lauriii had been battling! :D

Adding the issue this is postponed on to the issue title because I couldn't figure it out until I read my own comment...