EXAMPLE: Sort shorthand vs. longhand declarations — breaking change behind config

[kylorhall-atlassian]

What is this change?

Adds sorting of shorthand vs. longhand to ensure longhand deterministically will override shorthand.

⚠️ This could result in regressions as this will no longer work, so it's behind config, but this may mean this change should never go through (if we added border to this sorting):

const styles = css({ borderTop: '1px solid blue' });
const noBorderStyles = css({ border: 'none' });

export default ({ noBorder }) => <div
  css={[styles, noBorder && noBorderStyles]}
/>

Why are we making this change?

This is because quite commonly this code may be applied in either direction meaning sometimes font-weight: bold is applied and sometimes it's not, depending on the order that the stylesheet was generated in.

const styles = css({ font: '12px / 24px' });
const boldStyles = css({ fontWeight: 'bold' });

export default ({ isBold }) => <div
  css={[styles, isBold && boldStyles]}
/>

Example in a Storybook:

How are we making this change?

Where we already sort atomic styles, we now sort shorthand vs. longhand in that same area.

---

PR checklist

• [ ] Validate this works consistently as it's unclear if this sortAtomicStyleSheet iterates over all declarations or just one by one…
• [x] Added an applicable test suite
• [ ] Expand the list of shorthand properties, right now this is just outline and font, there are many more!
• [ ] Investigate if there's further tests to update
• [ ] Update the documentation in website/ with this new config

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 4ed15ebec15830eb3b67069a9f8f54986f211594

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

This PR includes changesets to release 19 packages

Name	Type
@compiled/babel-plugin-strip-runtime	Minor
@compiled/parcel-optimizer	Minor
@compiled/webpack-loader	Minor
@compiled/react	Minor
@compiled/utils	Minor
@compiled/ssr-app	Minor
@compiled/css	Minor
@compiled/webpack-app	Patch
@compiled/parcel-transformer	Patch
@compiled/parcel-config	Patch
@compiled/parcel-optimizer-test-app	Patch
@compiled/parcel-transformer-test-app	Patch
@compiled/parcel-transformer-test-compress-class-name-app	Patch
@compiled/parcel-transformer-test-custom-resolve-app	Patch
@compiled/parcel-transformer-test-custom-resolver-app	Patch
@compiled/parcel-transformer-test-extract-app	Patch
@compiled/babel-plugin	Minor
@compiled/codemods	Patch
@compiled/eslint-plugin	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]]

✅ Deploy Preview for compiled-css-in-js ready!

Name	Link
🔨 Latest commit	4ed15ebec15830eb3b67069a9f8f54986f211594
🔍 Latest deploy log	https://app.netlify.com/sites/compiled-css-in-js/deploys/66f604772c06d90008203475
😎 Deploy Preview	https://deploy-preview-1700--compiled-css-in-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[dddlr]

More todos

• [x] Add changesets
• [x] Ensure sorting in runtime mode (extraction turned off) is adequately tested
• [x] Ensure sorting border* properties is deterministic when stylesheet extraction is turned on
• [x] ~Benchmark whether adding 14 extra buckets at runtime makes anything slower~

[dddlr]

docs deploy preview

https://deploy-preview-1700--compiled-css-in-js.netlify.app/docs/shorthand

based on my best understanding of how it works, feel free to make any corrections

[JakeLane]

Can we add linting for this area?

This change makes the behaviour deterministic but it's still confusing to developers. Given the example in the description:

const styles = css({ font: '12px / 24px' });
const boldStyles = css({ fontWeight: 'bold' });

export default ({ isBold }) => <div
  css={[styles, isBold && boldStyles]}
/>

The inverse could happen:

const styles = css({ font: '12px / 24px' });
const boldStyles = css({ fontWeight: 'bold' });

export default ({ isFont }) => <div
  css={[boldStyles, isFont && styles]}
/>

which would have deterministic behaviour opposite to what the developer intended.

Generally, we should make the linting sort the code with autofixers (and warn when not possible) so that the source represents the production code.

[dddlr]

Can we add linting for this area?

This change makes the behaviour deterministic but it's still confusing to developers. Given the example in the description:

const styles = css({ font: '12px / 24px' });
const boldStyles = css({ fontWeight: 'bold' });

export default ({ isBold }) => <div
  css={[styles, isBold && boldStyles]}
/>

The inverse could happen:

const styles = css({ font: '12px / 24px' });
const boldStyles = css({ fontWeight: 'bold' });

export default ({ isFont }) => <div
  css={[boldStyles, isFont && styles]}
/>

which would have deterministic behaviour opposite to what the developer intended.

Generally, we should make the linting sort the code with autofixers (and warn when not possible) so that the source represents the production code.

yeah sure

@zesmith2 is working on sorting shorthand and longhand properties within the same css/styled/etc function call, but we can add checking arrays as a follow-up task :)