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

Eslint plugin: prefer-box-no-classname rule w/ autofix + merged into prefer-box-lonely-ref #1629

Merged
merged 6 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 8 additions & 7 deletions docs/src/Eslint Plugin.doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,24 @@ card(
`}
/>
<MainSection.Subsection
title="gestalt/prefer-box"
title="gestalt/prefer-box-inline-style"
description={`
Prevent using \`<div>\` inline styling for attributes that are already implemented in Box.

[Learn more about Box](/Box).
`}
/>
<MainSection.Subsection
title="gestalt/prefer-box-lonely-ref"
description={`
Prevent \`<div>\` tags used to only contain a \`ref\` attribute.
title="gestalt/prefer-box-no-disallowed"
description={`Prevent \`<div>\` tags that don't contain disallowed attributes: className and onClick. Use Gestalt Box, instead. Other attributes are disallowed as well so this Eslint rule doesn't conflict with [eslint-plugin-jsx-a11y](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y).

Instead, use \`ref\` props supported in Box or other elements such as Button or TextField.
[Read more about Box](/Box).

With AUTOFIX!
It also prevents \`<div>\` tags used to only contain a \`ref\` attribute. \`ref\` is supported in Box and other elements such as Button or TextField.

[Read more about the ref prop in Box](/Box#Using-as-a-ref).

[Read more about the ref prop in Box](/Box#Using-as-a-ref).
With AUTOFIX!
`}
/>
<MainSection.Subsection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { Box as RenamedBox } from 'gestalt';
export default function TestElement() {
const ref = useRef(null);
return (
<Box>
<RenamedBox>
<div ref={ref}>
<Box />
<RenamedBox />
</div>
</Box>
</RenamedBox>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { Box as RenamedBox } from 'gestalt';
export default function TestElement() {
const ref = useRef(null);
return (
<Box>
<Box ref={ref}>
<Box />
</Box>
</Box>
<RenamedBox>
<RenamedBox ref={ref}>
<RenamedBox />
</RenamedBox>
</RenamedBox>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export default function TestElement() {
const ref = useRef(null);
return (
<div ref={ref}>
<div />
<button />
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export default function TestElement() {
const ref = useRef(null);
return (
<Box ref={ref}>
<div />
<button />
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Box } from 'gestalt';
export default function TestElement() {
return (
<Box>
<div onBlur={() => {}} style={{ marginTop: 200 }}>
<Box />
</div>
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Box } from 'gestalt';
export default function TestElement() {
return (
<Box>
<Box onBlur={() => {}} dangerouslySetInlineStyle={{ __style: { marginTop: 200 } }}>
<Box />
</Box>
</Box>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { Box } from 'gestalt';
export default function TestElement() {
return (
<Box>
<Box ref={undefined} />
<div ref={undefined} className={undefined} />
<div onBlur={() => {}}>
<Box />
</div>
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Box } from 'gestalt';
export default function TestElement() {
return (
<Box>
<Box onBlur={() => {}}>
<Box />
</Box>
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Box as RenamedBox } from 'gestalt';
export default function TestElement() {
return (
<RenamedBox>
<div onBlur={() => {}}>
<RenamedBox />
</div>
</RenamedBox>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Box as RenamedBox } from 'gestalt';
export default function TestElement() {
return (
<RenamedBox>
<RenamedBox onBlur={() => {}}>
<RenamedBox />
</RenamedBox>
</RenamedBox>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Text, Flex } from 'gestalt';
export default function TestElement() {
return (
<div onBlur={() => {}}>
<Text>Test</Text>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Box, Flex, Text } from 'gestalt';
export default function TestElement() {
return (
<Box onBlur={() => {}}>
<Text>Test</Text>
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function TestElement() {
return (
<div onBlur={() => {}}>
<button />
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Box } from 'gestalt';
export default function TestElement() {
return (
<Box onBlur={() => {}}>
<button />
</Box>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function TestElement() {
return <div onBlur={() => {}} />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { Box } from 'gestalt';
export default function TestElement() {
return <Box onBlur={() => {}} />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Box } from 'gestalt';
export default function TestElement() {
const props = { onBlur: () => {} };
return (
<Box>
<Box ref={undefined} />
<div ref={undefined} className={undefined} />
<div ref={undefined} onClick={() => {}} />
<div ref={undefined} tabIndex={-1} />
<div ref={undefined} role="button" />
<div ref={undefined} onMouseOver={() => {}} />
<div ref={undefined} accessKey="test" />
<div ref={undefined} autoFocus />
<div ref={undefined} {...props} />
</Box>
);
}
45 changes: 33 additions & 12 deletions packages/eslint-plugin-gestalt/src/eslintASTFixers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// @flow strict
import { getNamedImportsComponents, getTextNodeFromSourceCode } from './eslintASTHelpers.js';
import {
getLocalComponentImportName,
getNamedImportsComponents,
getTextNodeFromSourceCode,
} from './eslintASTHelpers.js';

// $FlowFixMe[unclear-type]
type GenericNode = {| [string]: any |};
Expand Down Expand Up @@ -56,6 +60,7 @@ type RenameTagFixerType = ({|
context: GenericNode,
elementNode: GenericNode,
fixer: GenericNode,
gestaltImportNode: GenericNode,
newComponentName: string,
tagName: string,
|}) => $ReadOnlyArray<GenericNode>;
Expand All @@ -70,43 +75,53 @@ export const renameTagFixer: RenameTagFixerType = ({
context,
elementNode,
fixer,
gestaltImportNode,
newComponentName,
tagName,
}) => {
return [elementNode.openingElement, elementNode.closingElement]
.map((node) => {
// $FlowFixMe[incompatible-type] Flow is not detecting the method filter(Boolean)
if (!node) return false;
const namedImportsComponents =
getNamedImportsComponents({
importNode: gestaltImportNode,
}) ?? [];

const componentNameMatch = namedImportsComponents.find(
(item) => item[0] === newComponentName,
);

return fixer.replaceText(
node,
getTextNodeFromSourceCode({ context, elementNode: node }).replace(
tagName,
newComponentName,
),
const replacedText = getTextNodeFromSourceCode({ context, elementNode: node }).replace(
tagName,
(componentNameMatch && componentNameMatch[1]) ?? newComponentName,
);
return fixer.replaceText(node, replacedText);
})
.filter(Boolean);
};

type RenameTagWithPropsFixerType = ({|
additionalPropsString: string,
context: GenericNode,
elementNode: GenericNode,
fixer: GenericNode,
gestaltImportNode: GenericNode,
modifiedPropsString: string,
newComponentName: string,
tagName: string,
propsToRemove?: $ReadOnlyArray<string>,
|}) => $ReadOnlyArray<GenericNode>;

/** This function is a more complex version of renameTagFixer. It has the same tag replacement functionality, but it also rebuild the props in the opening tag to include a new prop. The new prop must be formatted as a string, p.e. `as="article"`
Examples 1:
"\<div\>\<\/div\>" if tagName="div", newComponentName="Box", and completePropsString=`as="article"` returns "\<Box as="article"\>\<\/Box\>"
"\<div\>\<\/div\>" if tagName="div", newComponentName="Box", and modifiedPropsString=`as="article"` returns "\<Box as="article"\>\<\/Box\>"
*/
export const renameTagWithPropsFixer: RenameTagWithPropsFixerType = ({
additionalPropsString,
context,
elementNode,
gestaltImportNode,
fixer,
modifiedPropsString,
newComponentName,
tagName,
}) => {
Expand All @@ -115,16 +130,22 @@ export const renameTagWithPropsFixer: RenameTagWithPropsFixerType = ({
// $FlowFixMe[incompatible-type] Flow is not detecting the method filter(Boolean)
if (!node) return false;

const completeOpeningNode = `<${newComponentName} ${additionalPropsString}${
const finalNewComponentName = getLocalComponentImportName({
importNode: gestaltImportNode,
componentName: newComponentName,
});

const completeOpeningNode = `<${finalNewComponentName} ${modifiedPropsString}${
elementNode.closingElement ? '' : ' /'
}>`;

return index === 0
? fixer.replaceText(node, completeOpeningNode)
: fixer.replaceText(
node,
getTextNodeFromSourceCode({ context, elementNode: node }).replace(
tagName,
newComponentName,
finalNewComponentName,
),
);
})
Expand Down