Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default sortShorthand to true (in extraction) to reduce configuration required. #1740

Merged
merged 2 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading