Fix shorthand sorting buggy

dddlr commented 1 week ago

What is this change?

Fix the shorthand property sorting being buggy when not using runtime mode (e.g. when using stylesheet extraction).

Why are we making this change?

@kylorhall-atlassian and I noticed that sortShorthand wasn't actually sorting the properties as it should.

It turns out that a lot of the time in the sorting algorithm, the two properties being compared are

not related to each other (e.g. padding is related to paddingLeft, but padding is not related to borderColor), and
one of the properties is a longhand property that doesn't exist in shorthandBuckets

As a result, there are cases where the stylesheet would not be sorted at all, simply because the wrong nodes were being compared and so the sortNodes function returns 0 every time.

How are we making this change?

Fix and simplify the sortNodes function:

Remove the logic of checking whether one property is a shorthand of another. It is safe to assume that this logic evaluates to false most of the time anyway.
In aShorthandBucket and bShorthandBucket, set the default value to Infinity. This way, if one property is a shorthand property and the other property is not a shorthand property, we guarantee that the shorthand property will come in front. For example, margin will come before paddingRight (even though they're not related).

PR checklist

Don't delete me!

I have...

[x] Updated or added applicable tests
[x] Verify the changes to existing tests are correct
[x] Updated the documentation in website/
[x] Verify that this PR fixes the lack of shorthand sorting in Jira
[x] Address this PR comment from Kylor: https://github.com/atlassian-labs/compiled/pull/1730#discussion_r1809966320
[x] Add changeset

changeset-bot[bot] commented 1 week ago

🦋 Changeset detected

Latest commit: 8b6edba726e3733b36a0c286bd74f0eb31539f50

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

| Name | Type | | ------------------------------------ | ----- | | @compiled/css | Minor | | @compiled/babel-plugin-strip-runtime | Patch | | @compiled/babel-plugin | Patch | | @compiled/parcel-optimizer | Patch | | @compiled/webpack-loader | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 1 week ago

Deploy Preview for compiled-css-in-js canceled.

Name	Link
Latest commit	8b6edba726e3733b36a0c286bd74f0eb31539f50
Latest deploy log	https://app.netlify.com/sites/compiled-css-in-js/deploys/67183ef7ea580e000836a4d3

atlassian-labs / compiled