Skip to content

Commit

Permalink
[Emotion] Add eslint rule for CSS logical properties (#6125)
Browse files Browse the repository at this point in the history
* Add custom Emotion css eslint plugin for non-logical properties

* Convert logicals map to JSON
- so that both script/ JS and src/ TS can use it

* Fix datagrid docs TS JSON imports typing

- throwing as a result of our new `resolveJsonModule: true`

* Convert old static map to use imported logicals map

- importing the map directly with a regex for newlines produces less false positives for non-logical properties

* Remove inset property from map

- it's already a new logical property and doesn't need to be converted

* Re-add catch for text-align logical values

+ add separate message for values vs properties - this might get used more for other future properties

* Initial conversion to using regex exec

- switching to a exec allows us to get the start index of the match, so that we can more accurately return the node location and also use a fixer
- (opinionated) using a single regex vs multiple loops also simplifes logic somewhat, even if the regex itself is annoying
- use regex capture groups for readability

* Add ability to --fix eslint rule

- via the maps we've set up

* Add logic for determining exact css property location
- vs highlighting the entire css`` block
- yay for even more gnarly regex utils

* Convert global HTML height property

* Typography

* EuiScreenReaderOnly

* EuiAccordion

* EuiBeacon

* EuiButton

* EuiCallOut

* EuiComment

* EuiExpression

* EuiImage - use euiBreakpoint mixin

- instead of manual media query

* EuiImage inset

* Loading components

note: loading_logo.styles already had so many static logical properties i just continued using them for in-file consistency

* EuiPageTemplate

* EuiProgress

* EuiToast

* EuiPanel

* Fix misc src-doc usages

* Fix scroll documentation usages + simplify snippets

* [PR feedback] lint *all* template literals, not just css``

- since we have several Emotion utilities that pass back raw ``s instead of css``

* Fix src styles missing logical properties

* Fix over-aggressive src-docs linting by removing template literals

* Fix over-aggressive src-docs linting by disabling rule

* Fix valid src-docs catches

- although debatable how useful they are if we're moving away from Sass

* Add dev documentation on how to disable the lint rule for specific properties
  • Loading branch information
Constance authored Aug 22, 2022
1 parent 0b7a908 commit 4c285cd
Show file tree
Hide file tree
Showing 56 changed files with 549 additions and 311 deletions.
1 change: 1 addition & 0 deletions .eslintplugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ exports.rules = {
'href-with-rel': require('./scripts/eslint-plugin/rel'),
'require-license-header': require('./scripts/eslint-plugin/require_license_header'),
'forward-ref': require('./scripts/eslint-plugin/forward_ref_display_name'),
'css-logical-properties': require('./scripts/eslint-plugin/css_logical_properties'),
};
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module.exports = {
'local/i18n': 'error',
'local/href-with-rel': 'error',
'local/forward-ref': 'error',
'local/css-logical-properties': 'error',
'local/require-license-header': [
'warn',
{
Expand Down
123 changes: 123 additions & 0 deletions scripts/eslint-plugin/css_logical_properties.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
const logicals = require('../../src/global_styling/functions/logicals.json');
const logicalProperties = Object.keys(logicals);

const logicalValues = {
'text-align: left': 'text-align: start',
'text-align: right': 'text-align: end',
// TODO: Consider adding float, clear, & resize as well
// @see https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Logical_Properties
};

const logicalPropertiesRegex = logicalProperties.join('|');
const logicalValuesRegex = Object.keys(logicalValues).join('|');
// @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec
// @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Groups_and_Backreferences
const regex = new RegExp(
`^(?<whitespace>[\\s]*)((?<property>${logicalPropertiesRegex}):)|(?<value>${logicalValuesRegex})`,
'gm'
);

const logicalsFixMap = { ...logicals, ...logicalValues };

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Enforce using CSS logical properties in our Emotion CSS',
},
messages: {
preferLogicalProperty:
'Prefer the CSS logical property for {{ property }} - @see src/global_styling/functions/logicals.ts',
preferLogicalValue:
'Prefer the CSS logical value for {{ property }} - @see src/global_styling/functions/logicals.ts',
},
fixable: 'code',
// NOTE: To disable this lint rule for a single line/property within a css`` block
// your code must use a comment inside a template literal, e.g.:
// css`
// color: red;
// height: 40px; ${/* eslint-disable-line local/css-logical-properties */ ''}
// `
},
create: function (context) {
return {
TemplateLiteral(node) {
const templateContents = node.quasis || [];
templateContents.forEach((quasi) => {
const stringLiteral = quasi?.value?.raw;
if (!stringLiteral) return;

findOccurrences(regex, stringLiteral).forEach(
({ match, lineNumber, column }) => {
const property = match.groups.property || match.groups.value;
const whitespace = match.groups.whitespace?.length || 0;

const lineStart = quasi.loc.start.line + lineNumber;
const columnStart = column + whitespace;

context.report({
loc: {
start: {
line: lineStart,
column: columnStart,
},
end: {
line: lineStart,
column: columnStart + property.length,
},
},
messageId: match.groups.value
? 'preferLogicalValue'
: 'preferLogicalProperty',
data: { property },
fix: function (fixer) {
const literalStart = quasi.range[0] + 1; // Account for backtick
const indexStart = literalStart + match.index + whitespace;

return fixer.replaceTextRange(
[indexStart, indexStart + property.length],
logicalsFixMap[property]
);
},
});
}
);
});
},
};
},
};

/**
* Regex helpers for finding the location of a property
* (vs highlighting the entire css`` node)
*
* credit to https://stackoverflow.com/a/61725880/4294462
*/

const lineNumberByIndex = (index, string) => {
const re = /^[\S\s]/gm;
let line = 0,
match;
let lastRowIndex = 0;
while ((match = re.exec(string))) {
if (match.index > index) break;
lastRowIndex = match.index;
line++;
}
return [Math.max(line - 1, 0), lastRowIndex];
};

const findOccurrences = (needle, haystack) => {
let match;
const result = [];
while ((match = needle.exec(haystack))) {
const pos = lineNumberByIndex(needle.lastIndex, haystack);
result.push({
match,
lineNumber: pos[0],
column: needle.lastIndex - pos[1] - match[0].length,
});
}
return result;
};
99 changes: 99 additions & 0 deletions scripts/eslint-plugin/css_logical_properties.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import rule from './css_logical_properties.js';
const RuleTester = require('eslint').RuleTester;

const ruleTester = new RuleTester({
parser: require.resolve('babel-eslint'),
});

const valid = [
`css\`
inline-size: 50px;
inline-start-end: 10px;
\``,
// Make sure we don't incorrectly catch similar properties that do not have logical equivalents
`\`
line-height: 20px;
border-width: 1px;
scrollbar-width: 30px
\``,
];

const invalid = [
{
code: 'css`height: 50px;`',
output: 'css`block-size: 50px;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: '`max-height: 50px;`',
output: '`max-block-size: 50px;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`width: 50px;`',
output: 'css`inline-size: 50px;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: '`min-width: 50px;`',
output: '`min-inline-size: 50px;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`top: 0;`',
output: 'css`inset-block-start: 0;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`padding-right: 0;`',
output: 'css`padding-inline-end: 0;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`margin-bottom: 0;`',
output: 'css`margin-block-end: 0;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`border-left: 1px solid green;`',
output: 'css`border-inline-start: 1px solid green;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`border-left-color: red;`',
output: 'css`border-inline-start-color: red;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
{
code: 'css`text-align: left;`',
output: 'css`text-align: start;`',
errors: [{ messageId: 'preferLogicalValue' }],
},
{
code: 'css`overflow-y: hidden;`',
output: 'css`overflow-block: hidden;`',
errors: [{ messageId: 'preferLogicalProperty' }],
},
// Test multiple errors
{
code: `css\`
content: 'ok';
text-align: right;
bottom: 50px;
\``,
output: `css\`
content: 'ok';
text-align: end;
inset-block-end: 50px;
\``,
errors: [
{ messageId: 'preferLogicalValue' },
{ messageId: 'preferLogicalProperty' },
],
},
];

ruleTester.run('css_logical_properties', rule, {
valid,
invalid,
});
14 changes: 10 additions & 4 deletions src-docs/src/views/auto_sizer/auto_sizer.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import React from 'react';
import { css } from '@emotion/react';

import { EuiAutoSizer, EuiCode, EuiPanel } from '../../../../src/components';
import {
EuiAutoSizer,
EuiCode,
EuiPanel,
logicalSizeCSS,
} from '../../../../src';

export default () => {
const containerStyles = css`
height: 200px;
width: 100%;
${logicalSizeCSS('100%', '200px')}
`;

const panelStyles = css`
Expand All @@ -21,7 +25,9 @@ export default () => {
<EuiAutoSizer>
{({ height, width }) => (
<EuiPanel css={[panelStyles, { height, width }]}>
<EuiCode>{`height: ${height}, width: ${width}`}</EuiCode>
<EuiCode>
height: {height}, width: {width}
</EuiCode>
</EuiPanel>
)}
</EuiAutoSizer>
Expand Down
6 changes: 3 additions & 3 deletions src-docs/src/views/comment/comment_props.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export default ({ snippet }: { snippet: ReactNode }) => {
<span
css={css`
display: inline-block;
width: ${euiTheme.size.base};
height: ${euiTheme.size.base};
${logicalCSS('width', euiTheme.size.base)}
${logicalCSS('height', euiTheme.size.base)}
background: ${euiTheme.colors.primary};
color: ${euiTheme.colors.emptyShade};
${useEuiFontSize('xs')};
Expand Down Expand Up @@ -69,7 +69,7 @@ export default ({ snippet }: { snippet: ReactNode }) => {
${euiTheme.border.radius.small} 0 0;
padding: ${euiTheme.size.s};
background: ${euiTheme.colors.lightestShade};
border-bottom: ${euiTheme.border.thin};
${logicalCSS('border-bottom', euiTheme.border.thin)}
display: flex;
`}
>
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/datagrid/_snippets.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { Link } from 'react-router-dom';

/* eslint-disable local/css-logical-properties */
export const gridSnippets = {
inMemory: `// Will try to autodectect schemas and do sorting and pagination in memory.
inMemory={{ level: 'sorting' }}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
EuiDataGridCellValueElementProps,
} from '!!prop-loader!../../../../../src/components/datagrid/data_grid_types';

/* eslint-disable local/css-logical-properties */
const gridLeadingColumnsSnippet = `<EuiDataGrid
aria-label="Data grid with trailing columns set"
columns={columns}
Expand Down
11 changes: 3 additions & 8 deletions src-docs/src/views/datagrid/styling/row_height_auto.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import React, {
useMemo,
ReactNode,
} from 'react';
// @ts-ignore not configured to import json
import githubData from '../_row_auto_height_data.json';

import {
Expand Down Expand Up @@ -34,12 +33,7 @@ interface DataShape {
}>;
comments: number;
created_at: string;
body?: string;
}

// convert strings to Date objects
for (let i = 0; i < githubData.length; i++) {
githubData[i].created_at = new Date(githubData[i].created_at);
body: null | string;
}

type DataContextShape =
Expand Down Expand Up @@ -110,7 +104,8 @@ const RenderCellValue: EuiDataGridProps['renderCellValue'] = ({
imageUrl={item.user.avatar_url}
size="s"
/>{' '}
{item.user.login} on {formatDate(item.created_at, 'dobLong')}
{item.user.login} on{' '}
{formatDate(new Date(item.created_at), 'dobLong')}
</span>
</EuiText>

Expand Down
11 changes: 3 additions & 8 deletions src-docs/src/views/datagrid/styling/row_height_fixed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import React, {
useMemo,
ReactNode,
} from 'react';
// @ts-ignore not configured to import json
import githubData from '../_row_auto_height_data.json';

import {
Expand Down Expand Up @@ -34,12 +33,7 @@ interface DataShape {
}>;
comments: number;
created_at: string;
body?: string;
}

// convert strings to Date objects
for (let i = 0; i < githubData.length; i++) {
githubData[i].created_at = new Date(githubData[i].created_at);
body: null | string;
}

type DataContextShape =
Expand Down Expand Up @@ -110,7 +104,8 @@ const RenderCellValue: EuiDataGridProps['renderCellValue'] = ({
imageUrl={item.user.avatar_url}
size="s"
/>{' '}
{item.user.login} on {formatDate(item.created_at, 'dobLong')}
{item.user.login} on{' '}
{formatDate(new Date(item.created_at), 'dobLong')}
</span>
</EuiText>

Expand Down
Loading

0 comments on commit 4c285cd

Please sign in to comment.