Skip to content

Commit

Permalink
Default sortShorthand to true (in extraction) to reduce configurati…
Browse files Browse the repository at this point in the history
…on required. (#1740)

* Default `sortShorthand` to true to reduce configuration required.

This matches the config we have internally at Atlassian and our recommended guidance and also matches the runtime sorting/bucketing (which has no configurable opt-out).

* Fixup parcel snapshots (expected sorting changes
  • Loading branch information
kylorhall-atlassian authored Nov 6, 2024
1 parent 18d2529 commit f63b99d
Show file tree
Hide file tree
Showing 17 changed files with 114 additions and 121 deletions.
12 changes: 12 additions & 0 deletions .changeset/plenty-feet-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@compiled/babel-plugin-strip-runtime': minor
'@compiled/parcel-optimizer': minor
'@compiled/webpack-loader': minor
'@compiled/css': minor
---

Possibly BREAKING: Default `sortShorthand` to be enabled during stylesheet extraction to match the config we have internally at Atlassian and our recommendation.

You can opt-out from this change by setting `sortShorthand: false` in several places, refer to https://compiledcssinjs.com/docs/shorthand and package-specific documentation.

This is only a breaking change if you expect `margin:0` to override `margin-top:8px` for example, which in other CSS-in-JS libraries may actually work, but in Compiled it's not guaranteed to work, so we forcibly sort it to guarantee the order in which these styles are applied.
2 changes: 1 addition & 1 deletion packages/babel-plugin-strip-runtime/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface PluginOptions {
/**
* Whether to sort shorthand and longhand properties,
* eg. `margin` before `margin-top` for enforced determinism.
* Defaults to `false`.
* Defaults to `true`.
*/
sortShorthand?: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,7 @@ describe('css map advanced functionality (at rules, selectors object)', () => {
'._14jq32ev color{color:pink}',
'._1wsc13q2 fontSize{background-color:blue}',
'._1wyb12am{font-size:50px}',

'const styles={success:"_syazjafr _1wyb12am _14jq32ev _1wsc13q2"}',
'const styles={success:"_syazjafr _1wyb12am _1wsc13q2 _14jq32ev"}',
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('Keyframes', () => {
'._1wybgktf{font-size:20px}',
'._2rko1l7b{border-radius:3px}',
'._y44v1bcx{animation:kfwl3rt}',
'{ax(["_1wybgktf _2rko1l7b _y44v1bcx", __cmplp.className])}',
'{ax(["_2rko1l7b _y44v1bcx _1wybgktf", __cmplp.className])}',
]);
});

Expand All @@ -45,7 +45,7 @@ describe('Keyframes', () => {
'._y44v178k{animation:kvif0b9}',
'._1wybgktf{font-size:20px}',
'._2rko1l7b{border-radius:3px}',
'{ax(["_y44v178k _1wybgktf _2rko1l7b", __cmplp.className])}',
'{ax(["_y44v178k _2rko1l7b _1wybgktf", __cmplp.className])}',
]);
});

Expand All @@ -69,7 +69,7 @@ describe('Keyframes', () => {
expect(actual).toIncludeMultiple([
'._syaz5scu{color:red}',
'._y44v1bcx{animation:kfwl3rt}',
'{ax(["_syaz5scu _y44v1bcx", __cmplp.className])}',
'{ax(["_y44v1bcx _syaz5scu", __cmplp.className])}',
]);
});

Expand Down
117 changes: 49 additions & 68 deletions packages/babel-plugin/src/styled/__tests__/behaviour.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ describe('styled component behaviour', () => {
]);
`);

expect(actual).toInclude('{font-size:12px}');
expect(actual).toInclude('{color:blue}');
expect(actual).toInclude('{font-weight:500}');
expect(actual).toIncludeMultiple(['{font-size:12px}', '{color:blue}', '{font-weight:500}']);
});

it('should not destructure valid html attributes from props', () => {
Expand Down Expand Up @@ -256,8 +254,10 @@ describe('styled component behaviour', () => {
});
`);

expect(actual).toInclude('{color:var(--_63bh2t)}');
expect(actual).toInclude('"--_63bh2t":ix((()=>{return __cmplp.color;})())');
expect(actual).toIncludeMultiple([
'{color:var(--_63bh2t)}',
'"--_63bh2t":ix((()=>{return __cmplp.color;})())',
]);
});

it('should transform an arrow function with a body into an IIFE by preventing passing down invalid html attributes to the node', () => {
Expand All @@ -269,9 +269,11 @@ describe('styled component behaviour', () => {
});
`);

expect(actual).toInclude('{font-size:var(--_1eiw442)}');
expect(actual).toInclude('const{textSize,...__cmpldp}=__cmplp;');
expect(actual).toInclude('"--_1eiw442":ix((()=>{return __cmplp.textSize;})())');
expect(actual).toIncludeMultiple([
'{font-size:var(--_1eiw442)}',
'const{textSize,...__cmpldp}=__cmplp;',
'"--_1eiw442":ix((()=>{return __cmplp.textSize;})())',
]);
});

it('should move suffix and prefix of a dynamic arrow function with a body into an IIFE', () => {
Expand All @@ -283,8 +285,10 @@ describe('styled component behaviour', () => {
});
`);

expect(actual).toInclude('{content:var(--_63bh2t)}');
expect(actual).toInclude('"--_63bh2t":ix((()=>{return __cmplp.color;})(),"\\"","\\"")');
expect(actual).toIncludeMultiple([
'{content:var(--_63bh2t)}',
'"--_63bh2t":ix((()=>{return __cmplp.color;})(),"\\"","\\"")',
]);
});

it('should collect args as styles', () => {
Expand Down Expand Up @@ -531,7 +535,7 @@ describe('styled component behaviour', () => {
'._ca0qftgi{padding-top:8px}',
'._19itlf8h{border:2px solid blue}',
'._1wyb1ul9{font-size:30px}',
'ax(["_1wyb1ul9 _19itlf8h _ca0qftgi _u5f3ftgi _n3tdftgi _19bvftgi",__cmplp.isPrimary?"_syaz13q2":"_syaz5scu",__cmplp.isDone?"_1hms1911":"_1hmsglyw",__cmplp.isClamped?"_1yyj11wp":"_1yyjkb7n",__cmplp.className])',
'ax(["_19itlf8h _ca0qftgi _u5f3ftgi _n3tdftgi _19bvftgi _1wyb1ul9",__cmplp.isPrimary?"_syaz13q2":"_syaz5scu",__cmplp.isDone?"_1hms1911":"_1hmsglyw",__cmplp.isClamped?"_1yyj11wp":"_1yyjkb7n",__cmplp.className])',
]);
});

Expand Down Expand Up @@ -624,11 +628,8 @@ describe('styled component behaviour', () => {
'._syaz5scu{color:red}',
'._syaz13q2{color:blue}',
'._1wyb1ul9{font-size:30px}',
`ax([\"_1wyb1ul9\",__cmplp.isPrimary?\"_syaz13q2\":\"_syaz5scu\",__cmplp.isPrimary?\"_19it1nsd\":\"_19it107e\",__cmplp.className]`,
]);

expect(actual).toInclude(
`ax([\"_1wyb1ul9\",__cmplp.isPrimary?\"_syaz13q2\":\"_syaz5scu\",__cmplp.isPrimary?\"_19it1nsd\":\"_19it107e\",__cmplp.className]`
);
});

it('should apply conditional CSS with nested ternary operators', () => {
Expand All @@ -653,7 +654,7 @@ describe('styled component behaviour', () => {
'._syaz5scu{color:red}',
'._syaz13q2{color:blue}',
'._syaz11x8{color:black}',
`ax([\"_1wyb1ul9 _19itlf8h _ca0qftgi _u5f3ftgi _n3tdftgi _19bvftgi\",__cmplp.isPrimary?__cmplp.isDisabled?\"_syaz11x8\":\"_syaz13q2\":\"_syaz5scu\",__cmplp.className])`,
`ax([\"_19itlf8h _ca0qftgi _u5f3ftgi _n3tdftgi _19bvftgi _1wyb1ul9\",__cmplp.isPrimary?__cmplp.isDisabled?\"_syaz11x8\":\"_syaz13q2\":\"_syaz5scu\",__cmplp.className])`,
]);
});

Expand All @@ -674,11 +675,8 @@ describe('styled component behaviour', () => {
'._19it7fe6{border:3px solid yellow}',
'._bfhk1x77{background-color:white}',
'._syaz5scu{color:red}',
'className={ax(["_bfhk1x77 _19it7fe6 _syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}',
]);

expect(actual).toInclude(
'className={ax(["_syaz5scu _bfhk1x77 _19it7fe6",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}'
);
});

it('should apply conditional CSS with template literal and nested ternary operators', () => {
Expand Down Expand Up @@ -733,11 +731,8 @@ describe('styled component behaviour', () => {
'._k48p8n31{font-weight:bold}',
'._syaz13q2{color:blue}',
'._syaz5scu{color:red}',
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.isBolded&&"_k48p8n31",__cmplp.className])}',
]);

expect(actual).toInclude(
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.isBolded&&"_k48p8n31",__cmplp.className])}'
);
});

it('should not allow a logical statement with a conditional right-hand side', () => {
Expand Down Expand Up @@ -838,11 +833,8 @@ describe('styled component behaviour', () => {
'._19it7fe6{border:3px solid yellow}',
'._bfhk1x77{background-color:white}',
'._syaz5scu{color:red}',
'{ax(["_bfhk1x77 _19it7fe6 _syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}',
]);

expect(actual).toInclude(
'{ax(["_syaz5scu _bfhk1x77 _19it7fe6",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}'
);
});

it('should apply unconditional after a conditional css rule with template literal', () => {
Expand All @@ -862,11 +854,8 @@ describe('styled component behaviour', () => {
'._bfhk1x77{background-color:white}',
'._syaz5scu{color:red}',
'._19it7fe6{border:3px solid yellow}',
'{ax(["_19it7fe6 _bfhk1x77 _syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}',
]);

expect(actual).toInclude(
'{ax(["_19it7fe6 _syaz5scu _bfhk1x77",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}'
);
});

it('should apply unconditional CSS with props', () => {
Expand All @@ -881,9 +870,8 @@ describe('styled component behaviour', () => {
expect(actual).toIncludeMultiple([
'const _="._syaz1q2z{color:var(--_1r7cl4y)}"',
'"--_1r7cl4y":ix(__cmplp.primary)',
'className={ax(["_syaz1q2z",__cmplp.className])}',
]);

expect(actual).toInclude('className={ax(["_syaz1q2z",__cmplp.className])}');
});

it('should apply unconditional CSS with and without props', () => {
Expand All @@ -900,9 +888,8 @@ describe('styled component behaviour', () => {
'._syaz1q2z{color:var(--_1r7cl4y)}',
'._bfhk5scu{background-color:red}',
'--_1r7cl4y":ix(__cmplp.primary)}',
'className={ax(["_bfhk5scu _syaz1q2z",__cmplp.className])}',
]);

expect(actual).toInclude('className={ax(["_bfhk5scu _syaz1q2z",__cmplp.className])}');
});

it('should apply conditional CSS with object styles', () => {
Expand All @@ -915,11 +902,11 @@ describe('styled component behaviour', () => {
);
`);

expect(actual).toIncludeMultiple(['._syaz13q2{color:blue}', '._syaz5scu{color:red}']);

expect(actual).toInclude(
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}'
);
expect(actual).toIncludeMultiple([
'._syaz13q2{color:blue}',
'._syaz5scu{color:red}',
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}',
]);
});

it('should apply conditional CSS with object styles and multiple props lines', () => {
Expand All @@ -937,11 +924,8 @@ describe('styled component behaviour', () => {
'._k48p8n31{font-weight:bold}',
'._syaz13q2{color:blue}',
'._syaz5scu{color:red}',
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.isBolded&&"_k48p8n31",__cmplp.className])}',
]);

expect(actual).toInclude(
'className={ax(["_syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.isBolded&&"_k48p8n31",__cmplp.className])}'
);
});

it('should apply unconditional before and after a conditional css rule with object styles', () => {
Expand All @@ -955,15 +939,12 @@ describe('styled component behaviour', () => {
);
`);

expect.toIncludeMultiple([
expect(actual).toIncludeMultiple([
'._syaz13q2{color:blue}',
'._19it97hw{border:1px solid black}',
'._syaz5scu{color:red}',
'{ax(["_19it97hw _syaz5scu",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}',
]);

expect(actual).toInclude(
'{ax(["_syaz5scu _19it97hw",__cmplp.isPrimary&&"_syaz13q2",__cmplp.className])}'
);
});

it('should apply conditional CSS with object styles regardless declaration order', () => {
Expand All @@ -976,11 +957,12 @@ describe('styled component behaviour', () => {
);
`);

expect(actual).toIncludeMultiple(['._syaz5scu{color:red}', '._syaz13q2{color:blue}']);
expect(actual).toIncludeMultiple([
'._syaz5scu{color:red}',
'._syaz13q2{color:blue}',

expect(actual).toInclude(
'className={ax(["_syaz13q2",__cmplp.isPrimary&&"_syaz5scu",__cmplp.className])}'
);
'className={ax(["_syaz13q2",__cmplp.isPrimary&&"_syaz5scu",__cmplp.className])}',
]);
});

it('should apply multi conditional logical expression', () => {
Expand All @@ -993,11 +975,11 @@ describe('styled component behaviour', () => {
);
`);

expect(actual).toIncludeMultiple(['._syaz13q2{color:blue}', '._syaz5scu{color:red}']);

expect(actual).toInclude(
'{ax(["_syaz5scu",(__cmplp.isPrimary||__cmplp.isMaybe)&&"_syaz13q2",__cmplp.className])}'
);
expect(actual).toIncludeMultiple([
'._syaz13q2{color:blue}',
'._syaz5scu{color:red}',
'{ax(["_syaz5scu",(__cmplp.isPrimary||__cmplp.isMaybe)&&"_syaz13q2",__cmplp.className])}',
]);
});

it('should apply multi conditional logical expression with different props lines and syntax styles', () => {
Expand Down Expand Up @@ -1030,11 +1012,11 @@ describe('styled component behaviour', () => {
);
`);

expect(actual).toIncludeMultiple(['._syaz13q2{color:blue}', '._syaz5scu{color:red}']);

expect(actual).toInclude(
'{ax(["_syaz5scu",__cmplp.isPrimary&&(__cmplp.isBolded||__cmplp.isFoo)&&"_syaz13q2",__cmplp.className])}'
);
expect(actual).toIncludeMultiple([
'._syaz13q2{color:blue}',
'._syaz5scu{color:red}',
'{ax(["_syaz5scu",__cmplp.isPrimary&&(__cmplp.isBolded||__cmplp.isFoo)&&"_syaz13q2",__cmplp.className])}',
]);
});

it('should apply conditional CSS with ternary and boolean in the same line', () => {
Expand Down Expand Up @@ -1070,9 +1052,8 @@ describe('styled component behaviour', () => {
expect(actual).toIncludeMultiple([
'._bfhk1x77{background-color:white}',
'._syazruxl{color:orange}',
'className={ax(["_bfhk1x77 _syazruxl",__cmplp.className])}',
]);

expect(actual).toInclude('className={ax(["_syazruxl _bfhk1x77",__cmplp.className])}');
});

it('should only add falsy condition when truthy condition has no value', () => {
Expand All @@ -1087,7 +1068,7 @@ describe('styled component behaviour', () => {
expect(actual).toIncludeMultiple([
'._syazbf54{color:green}',
'._bfhk11x8{background-color:black}',
'className={ax(["",!__cmplp.isPrimary&&"_syazbf54 _bfhk11x8",__cmplp.className])}',
'className={ax(["",!__cmplp.isPrimary&&"_bfhk11x8 _syazbf54",__cmplp.className])}',
]);
});

Expand All @@ -1103,7 +1084,7 @@ describe('styled component behaviour', () => {
expect(actual).toIncludeMultiple([
'._syazbf54{color:green}',
'._bfhk11x8{background-color:black}',
'className={ax(["",__cmplp.isPrimary&&"_syazbf54 _bfhk11x8",__cmplp.className])}',
'className={ax(["",__cmplp.isPrimary&&"_bfhk11x8 _syazbf54",__cmplp.className])}',
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ describe('styled tagged template expression', () => {
const color = 'red';
const color2 = 'blue';
const ListItem = styled.div\`
background: linear-gradient(
\${\`var(--my-variable, \${color})\`},
Expand All @@ -889,7 +889,7 @@ describe('styled tagged template expression', () => {
const color = 'red';
const interpolation = \`1px solid \${\`var(--my-variable, \${color})\`}\`;
const ListItem = styled.div\`
border: \${interpolation};
\`;
Expand Down Expand Up @@ -1062,7 +1062,7 @@ describe('styled tagged template expression', () => {
'._1wybgktf{font-size:20px}',
'._2rko1l7b{border-radius:3px}',
'._syaz1qjj{color:var(--_pvyxdf)}',
'{ax(["_1wybgktf _2rko1l7b _syaz1qjj", __cmplp.className])}',
'{ax(["_2rko1l7b _1wybgktf _syaz1qjj", __cmplp.className])}',
]);
});

Expand All @@ -1084,7 +1084,7 @@ describe('styled tagged template expression', () => {
'._syaz1qjj{color:var(--_pvyxdf)}',
'._1wybgktf{font-size:20px}',
'._2rko1l7b{border-radius:3px}',
'{ax(["_syaz1qjj _1wybgktf _2rko1l7b", __cmplp.className])}',
'{ax(["_2rko1l7b _syaz1qjj _1wybgktf", __cmplp.className])}',
]);
});
});
4 changes: 2 additions & 2 deletions packages/css/src/__tests__/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,10 @@ describe('#css-transform', () => {
);

expect(actual.join('\n')).toMatchInlineSnapshot(`
"._18u0idpf{margin-left:0}
"._1h6d1r31{border-color:currentColor}
._18u0idpf{margin-left:0}
._1sb21e8g{content:"hello"}
._syaz15td{color:#639}
._1h6d1r31{border-color:currentColor}
._bfhk1r31{background-color:currentColor}
._5wra1r31{border-left-color:currentColor}"
`);
Expand Down
Loading

0 comments on commit f63b99d

Please sign in to comment.