Skip to content

Commit

Permalink
Merge pull request #122 from solidjs-community/fix/show-fragments
Browse files Browse the repository at this point in the history
Check JSXFragment in most places where checking JSXElement, fixes #106.
  • Loading branch information
joshwilsonvu authored Dec 30, 2023
2 parents 85c4f5a + 794a64a commit 16a5209
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 8 deletions.
14 changes: 14 additions & 0 deletions docs/prefer-for.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ let Component = (props) => (
</ol>
);

let Component = (props) => (
<>
{props.data.map((d) => (
<li>{d.text}</li>
))}
</>
);
// after eslint --fix:
let Component = (props) => (
<>
<For each={props.data}>{(d) => <li>{d.text}</li>}</For>
</>
);

let Component = (props) => (
<ol>
{props.data.map((d) => (
Expand Down
14 changes: 14 additions & 0 deletions docs/prefer-show.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ function Component(props) {
);
}

function Component(props) {
return <>{props.cond && <span>Content</span>}</>;
}
// after eslint --fix:
function Component(props) {
return (
<>
<Show when={props.cond}>
<span>Content</span>
</Show>
</>
);
}

function Component(props) {
return <div>{props.cond ? <span>Content</span> : <span>Fallback</span>}</div>;
}
Expand Down
4 changes: 2 additions & 2 deletions src/rules/prefer-for.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TSESTree as T, ESLintUtils, ASTUtils } from "@typescript-eslint/utils";
import { isFunctionNode } from "../utils";
import { isFunctionNode, isJSXElementOrFragment } from "../utils";

const createRule = ESLintUtils.RuleCreator.withoutDocs;
const { getPropertyName } = ASTUtils;
Expand Down Expand Up @@ -55,7 +55,7 @@ export default createRule({
const callOrChain = node.parent?.type === "ChainExpression" ? node.parent : node;
if (
callOrChain.parent?.type === "JSXExpressionContainer" &&
callOrChain.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(callOrChain.parent.parent)
) {
// check for Array.prototype.map in JSX
if (
Expand Down
9 changes: 5 additions & 4 deletions src/rules/prefer-show.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { TSESTree as T, ESLintUtils } from "@typescript-eslint/utils";
import { isJSXElementOrFragment } from "../utils";

const createRule = ESLintUtils.RuleCreator.withoutDocs;

Expand All @@ -25,7 +26,7 @@ export default createRule({
const sourceCode = context.getSourceCode();
const putIntoJSX = (node: T.Node): string => {
const text = sourceCode.getText(node);
return node.type === "JSXElement" || node.type === "JSXFragment" ? text : `{${text}}`;
return isJSXElementOrFragment(node) ? text : `{${text}}`;
};

const logicalExpressionHandler = (node: T.LogicalExpression) => {
Expand All @@ -36,7 +37,7 @@ export default createRule({
fix: (fixer) =>
fixer.replaceText(
node.parent?.type === "JSXExpressionContainer" &&
node.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(node.parent.parent)
? node.parent
: node,
`<Show when={${sourceCode.getText(node.left)}}>${putIntoJSX(node.right)}</Show>`
Expand All @@ -55,7 +56,7 @@ export default createRule({
fix: (fixer) =>
fixer.replaceText(
node.parent?.type === "JSXExpressionContainer" &&
node.parent.parent?.type === "JSXElement"
isJSXElementOrFragment(node.parent.parent)
? node.parent
: node,
`<Show when={${sourceCode.getText(node.test)}} fallback={${sourceCode.getText(
Expand All @@ -68,7 +69,7 @@ export default createRule({

return {
JSXExpressionContainer(node) {
if (node.parent?.type !== "JSXElement") {
if (!isJSXElementOrFragment(node.parent)) {
return;
}
if (node.expression.type === "LogicalExpression") {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/reactivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
isDOMElementName,
ignoreTransparentWrappers,
getFunctionName,
isJSXElementOrFragment,
} from "../utils";

const { findVariable, getFunctionHeadLocation } = ASTUtils;
Expand Down Expand Up @@ -527,7 +528,7 @@ export default createRule<Options, MessageIds>({
if (
// The signal is not being called and is being used as a props.children, where calling
// the signal was the likely intent.
elementOrAttribute?.type === "JSXElement" ||
isJSXElementOrFragment(elementOrAttribute) ||
// We can't say for sure about user components, but we know for a fact that a signal
// should not be passed to a non-event handler DOM element attribute without calling it.
(elementOrAttribute?.type === "JSXAttribute" &&
Expand Down Expand Up @@ -886,7 +887,7 @@ export default createRule<Options, MessageIds>({
// to the DOM. This is semantically a "called function", so it's fine to read reactive
// variables here.
pushTrackedScope(node.expression, "called-function");
} else if (node.parent?.type === "JSXElement" && isFunctionNode(node.expression)) {
} else if (isJSXElementOrFragment(node.parent) && isFunctionNode(node.expression)) {
pushTrackedScope(node.expression, "function"); // functions inline in JSX containers will be tracked
} else {
pushTrackedScope(node.expression, "expression");
Expand Down
5 changes: 5 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ export const isProgramOrFunctionNode = (
node: T.Node | null | undefined
): node is ProgramOrFunctionNode => !!node && PROGRAM_OR_FUNCTION_TYPES.includes(node.type);

export const isJSXElementOrFragment = (
node: T.Node | null | undefined
): node is T.JSXElement | T.JSXFragment =>
node?.type === "JSXElement" || node?.type === "JSXFragment";

export const getFunctionName = (node: FunctionNode): string | null => {
if (
(node.type === "FunctionDeclaration" || node.type === "FunctionExpression") &&
Expand Down
5 changes: 5 additions & 0 deletions test/rules/prefer-for.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export const cases = run("prefer-for", rule, {
errors: [{ messageId: "preferFor" }],
output: `let Component = (props) => <ol><For each={props.data}>{d => <li>{d.text}</li>}</For></ol>;`,
},
{
code: `let Component = (props) => <>{props.data.map(d => <li>{d.text}</li>)}</>;`,
errors: [{ messageId: "preferFor" }],
output: `let Component = (props) => <><For each={props.data}>{d => <li>{d.text}</li>}</For></>;`,
},
{
code: `let Component = (props) => <ol>{props.data.map(d => <li key={d.id}>{d.text}</li>)}</ol>;`,
errors: [{ messageId: "preferFor" }],
Expand Down
11 changes: 11 additions & 0 deletions test/rules/prefer-show.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ export const cases = run("prefer-show", rule, {
return <div><Show when={props.cond}><span>Content</span></Show></div>;
}`,
},
{
code: `
function Component(props) {
return <>{props.cond && <span>Content</span>}</>;
}`,
errors: [{ messageId: "preferShowAnd" }],
output: `
function Component(props) {
return <><Show when={props.cond}><span>Content</span></Show></>;
}`,
},
{
code: `
function Component(props) {
Expand Down

0 comments on commit 16a5209

Please sign in to comment.