Assets: Fix the order of the chunk files

[arthur791004]

Related to pbxlJb-6v0-p2

Proposed Changes

• Fix the chunk files are not loaded in the correct order

Here is the example of the “Add new domain” button

Before	After

Why are these changes being made?

• To resolve the inconsistent CSS loading order

Testing Instructions

• Go through any pages on Calypso in different orders and ensure the styles are always the same

Here is an example of the “Add new domain” button on the domains page

• Go to the domains page as follows
  • Go to /sites and then click the Domains from the sidebar
  • Go to /domains directly
• Check the styles of the “Add new domain” is applied correctly

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• Have you written new tests for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-123847

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-123847&env=jetpack

Automattic for Agencies live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-123847&env=a8c-for-agencies

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[arthur791004]

I assume the named chunks always have the chunks property derived from the stats.namedChunkGroups above. But it seems to be safer to keep the old implementation.

[arthur791004]

@Automattic/team-calypso Does this change make sense? It would be helpful to get your review 🙂

[jsnajdr]

I don't think this is going to work. You change the order of the CSS modules to fix the domain button, but what if this reorder breaks another CSS which used to be in a "correct" order and now it is not?

The right solution is to change the CSS specificity so that it doesn't depend on the stylesheet order. The .button.split-button__main class is the SplitButton's general style, and has specificity 2. The .options-domain-button .split-button__main class is a special style just for the AddDomainButton component, but it also has specificity 2. It should have a higher specificity. The class name should be .options-domain-button .button.split-button__main, with specificity 3. The specificities should increase as you go from the most general style to the most specialized one.

By the way, is the order of these two classes really breaking something? One of the classes defines only the -webkit-font-smoothing property, that shouldn't conflict with any other rule.

[tyxla]

I second what @jsnajdr explained. The load order depends on the load order of the corresponding stylesheets, so it can vary based on the user's current session. To not have to rely on the load order, proper specificity should be used. That's one of the downsides of the current CSS system. Unfortunately, it hasn't improved over the last years, and we've had similar issues before (see p4TIVU-9vD-p2 as one example).

[arthur791004]

@jsnajdr Thanks for the review! I replied pbxlJb-6v0-p2#comment-3995

By the way, is the order of these two classes really breaking something? One of the classes defines only the -webkit-font-smoothing property, that shouldn't conflict with any other rule.

This one is just an example 😅 And I guess there are still lots of places that have the same issue but I didn't remember suddenly. For example, I remembered there are the same issue on the reader page and I changed the CSS specificity and rewrite the conflicts into a single file to resolve the issue.

[arthur791004]

@jsnajdr @tyxla Do you think it's good to move this forward? If not, I'll close this one 🙂

[jsnajdr]

Do you think it's good to move this forward?

Yes, this should definitely get merged. I'm just proposing to simplify the code a bit, removing the bits that are no longer used.

[jsnajdr]

The new code is good, but to keep the code readable we should remove the old code. The assets writer plugin doesn't need to output siblings any more, they are not used. And getFilesForChunk should work only with chunk.chunks. getFilesForChunk is called only for chunks that are the main chunks of a chunk group, and the chunk.chunks should always be there. If it's not, it's a reason to warn about an unknown chunk group and return empty assets.

[arthur791004]

Is bin/loader-stats.js still in use?

[jsnajdr]

I think you can safely delete it, it was relevant many years ago. Its job is now done by the iscalypsofastyet.com server and the GitHub comments that bot is posting.

But even if you wanted to keep it should be easy to update it to use chunk.chunks instead of chunk.siblings. The getChunkAndSiblings just gets the list of chunks in a chunk group, a task we are already familiar with.

[matticbot]

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

• notifications
• wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/webpack-chunk-files-order on your sandbox.

[jsnajdr]

Looks good, thanks for figuring out this subtle fix

[jsnajdr]

Better wording:

cannot find the chunks of the chunk group xxx

In the Calypso codebase we often don't correctly distinguish between a "chunk" and a "chunk group". For historical reasons, because we've been using webpack for a long time and "chunk groups" were added only in version 4 I think.

[arthur791004]

Ok! Updated, thank you!