Skip to content

Commit

Permalink
AFB-777 create eslint to enforce shorthand sorting (#1714)
Browse files Browse the repository at this point in the history
  • Loading branch information
zesmith2 authored Oct 11, 2024
1 parent b4864f9 commit c786a44
Show file tree
Hide file tree
Showing 10 changed files with 433 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-geckos-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@compiled/eslint-plugin': patch
---

Adding eslint rule to enforce shorthand css properties come before longhand
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { noJavaScriptXCSSRule } from './rules/no-js-xcss';
import { noKeyframesTaggedTemplateExpressionRule } from './rules/no-keyframes-tagged-template-expression';
import { noStyledTaggedTemplateExpressionRule } from './rules/no-styled-tagged-template-expression';
import { noSuppressXCSS } from './rules/no-suppress-xcss';
import { shorthandFirst } from './rules/shorthand-property-sorting';

export const rules = {
'jsx-pragma': jsxPragmaRule,
Expand All @@ -27,6 +28,7 @@ export const rules = {
'no-styled-tagged-template-expression': noStyledTaggedTemplateExpressionRule,
'no-suppress-xcss': noSuppressXCSS,
'no-empty-styled-expression': noEmptyStyledExpressionRule,
'shorthand-property-sorting': shorthandFirst,
};

export const configs = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import type { Rule } from 'eslint';

import { createNoTaggedTemplateExpressionRule } from '../../utils';

import { isStyled } from './utils';
import { createNoTaggedTemplateExpressionRule, isStyled } from '../../utils';

export const noStyledTaggedTemplateExpressionRule: Rule.RuleModule = {
meta: {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# `shorthand-property-sorting`

Enforces css property sorting for packages `css`, `cssMap`, `styled` that originates from `@compiled/react`, and `@atlaskit/css`.

Having a longhand (like `fontSize` and `borderTopColor`) before a shorthand (like `font` and `border`) is not valid CSS. While this is not usually a problem, it becomes a problem in Compiled where classes are generated for each CSS declaration and rules are then sorted at build time during stylesheet extraction. As a result, this could cause unwanted side effects when CSS longhands are redudant.

This rule enforces that the order in which the properties appear in a component's source code matches the actual ordering the properties will have at build time and runtime.

## Rule details

👎 Examples of **incorrect** code for this rule:

```js
import { css } from '@compiled/react';

const styles = css({
borderColor: 'red',
border: '1px solid black',
});
```

👍 Examples of **correct** code for this rule:

```js
import { css } from '@compiled/react';

const styles = css({
border: '1px solid black',
borderColor: 'red',
});
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
import { outdent } from 'outdent';

import { typeScriptTester as tester } from '../../../test-utils';
import { shorthandFirst } from '../index';

const packages_calls_and_imports = [
['css', 'css', '@atlaskit/css'],
['css', 'css', '@compiled/react'],
['styled', 'styled.div', '@compiled/react'],
['cssMap', 'cssMap', '@atlaskit/css'],
['cssMap', 'cssMap', '@compiled/react'],
];

const packages_and_calls = [
['css', 'css'],
['styled', 'styled.div'],
['cssMap', 'cssMap'],
];

tester.run('shorthand-property-sorting', shorthandFirst, {
valid: [
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `correct property ordering, (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
margin: '1', // 1
border: '2', // 1
borderColor: '7', // 2
borderBlock: '3', // 3
borderBottom: '6', // 4
borderBlockEnd: '4', // 5
borderBlockStart: '5', // 5
});
export const EmphasisText1 = ({ children }) => <span css={styles}>{children}</span>;
`,
})),
...packages_and_calls.map(([pkg, call]) => ({
name: `incorrect property ordering, (${pkg}: from unregulated package)`,
code: outdent`
import {${pkg}} from 'wrongpackage';
const styles = ${call}({
borderTop: '1px solid #00b8d9',
border: '#00b8d9',
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `properties that don't interact with out of order depths (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderColor: '#00b8d9', // 2
font: '#00b8d9', // 1
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `property not in bucket (${pkg}: '${imp}'`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
transitionDuration: '2', // unknown
transition: 'fast', // 1
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `depth in correct order for properties in the same bucket (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderColor: '#00b8d9', // 2
borderTop: '1px solid #00b8d9', // 4
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
})),
],
invalid: [
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `incorrect property ordering (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderTop: '1px solid #00b8d9',
border: '#00b8d9',
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `incorrect property ordering -> 3 properties (${pkg}: '${imp}'`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderColor: '#00b8d9', // 2
font: '#00b8d9', // 1
border: '#00b8d9', // 1
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `incorrect property ordering -> inline (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({ borderTop: '1px solid #00b8d9', border: '#00b8d9' });
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `depth out of order for properties in the same bucket (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderTop: '1px solid #00b8d9',
borderColor: '#00b8d9',
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `incorrect property ordering -> nested ObjectExpression (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const containerAppearance = {
default: ${call}({
borderBlockEnd: '1px solid #00b8d9',
borderBlock: '#00b8d9',
border: '#00b8d9',
}),
success: ${call}({
border: '#00b8d9',
borderBlock: '#00b8d9',
borderBlockEnd: '1px solid #00b8d9',
}),
inverse: ${call}({
border: '#00b8d9',
borderBlockEnd: '1px solid #00b8d9',
borderBlock: '#00b8d9',
}),
};
`,
errors: [{ messageId: 'shorthand-first' }, { messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `incorrect property ordering -> 6 reordering errors (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
borderTop: '1px solid #00b8d9',
border: '#00b8d9',
borderColor: '#00b8d9',
borderRight: '#00b8d9',
gridTemplate: '1fr 1fr',
gridRow: '1',
borderBlockStart: '10px',
});
export const EmphasisText = ({ children }) => <span css={styles}>{children}</span>;
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `includes pseudo-selectors -> pseudo is out of order (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
'&:hover': {
borderTop: '1px solid #00b8d9', // 4
borderColor: 'red', // 2
border: '1px solid #00b8d9', // 1
},
border: '1px solid #00b8d9', // 1
borderColor: 'red', // 2
borderTop: '1px solid #00b8d9', // 4
})
`,
errors: [{ messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `includes pseudo-selectors -> non-pseduo are out of order (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
'&:hover': {
border: '1px solid #00b8d9', // 1
borderColor: 'red', // 2
borderTop: '1px solid #00b8d9', // 4
},
borderTop: '1px solid #00b8d9', // 4
borderColor: 'red', // 2
border: '1px solid #00b8d9', // 1
})
`,
errors: [{ messageId: 'shorthand-first' }],
})),

/* fixer can't deal with nested fixing in one go. I've split this test into:
pt1 with the first round of fixes,
pt2 carrying on from the output of pt1.
*/
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `includes pseduo-selectors -> pseduo and non-pseduo are out of order pt1 (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({
'&:hover': {
borderTop: '1px solid #00b8d9', // 4
borderColor: 'red', // 2
border: '1px solid #00b8d9', // 1
},
borderTop: '1px solid #00b8d9', // 4
borderColor: 'red', // 2
border: '1px solid #00b8d9', // 1
})
`,
errors: [{ messageId: 'shorthand-first' }, { messageId: 'shorthand-first' }],
})),
...packages_calls_and_imports.map(([pkg, call, imp]) => ({
name: `includes pseduo-selectors -> pseduo and non-pseduo are out of order pt2 (${pkg}: '${imp}')`,
code: outdent`
import {${pkg}} from '${imp}';
const styles = ${call}({ '&:hover': {
borderTop: '1px solid #00b8d9', // 4
borderColor: 'red', // 2
border: '1px solid #00b8d9', // 1
}, border: '1px solid #00b8d9', borderColor: 'red', borderTop: '1px solid #00b8d9' })
`,
errors: [{ messageId: 'shorthand-first' }],
})),
],
});
Loading

0 comments on commit c786a44

Please sign in to comment.