Skip to content

Commit f554d3f

Browse files
committed
[WB-2247-bodytext-lint] PR feedback
1 parent 923f6c9 commit f554d3f

12 files changed

Lines changed: 351 additions & 63 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import {Meta, Markdown} from "@storybook/addon-docs/blocks";
2+
import lintRuleDocs from "../../../packages/eslint-plugin-wonder-blocks/docs/no-invalid-bodytext-parent.md?raw";
3+
4+
<Meta
5+
title="Tools / eslint-plugin-wonder-blocks / Rules / no-invalid-bodytext-parent"
6+
/>
7+
8+
<Markdown>{lintRuleDocs}</Markdown>

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@
108108
"@types/react-window": "^1.8.8",
109109
"@typescript-eslint/eslint-plugin": "8.17.0",
110110
"@typescript-eslint/parser": "8.17.0",
111-
"@typescript-eslint/rule-tester": "^8.58.0",
112111
"@vitejs/plugin-react-swc": "^3.9.0",
113112
"@vitest/browser": "^4.0.15",
114113
"@vitest/browser-playwright": "^4.0.15",

packages/eslint-plugin-wonder-blocks/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,4 @@ The following shows what rules are enabled in each config:
7171
| Rule | Enabled in `recommended`| Enabled in `strict` |
7272
|------|-------------------------|---------------------|
7373
| [`no-custom-tab-role`](docs/no-custom-tab-role.md)| ||
74+
| [`no-invalid-bodytext-parent`](docs/no-invalid-bodytext-parent.md)| ||

packages/eslint-plugin-wonder-blocks/demo/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
"@khanacademy/wonder-blocks-button": "workspace:*",
1717
"@khanacademy/wonder-blocks-clickable": "workspace:*",
1818
"@khanacademy/wonder-blocks-core": "workspace:*",
19+
"@khanacademy/wonder-blocks-form": "workspace:*",
1920
"@khanacademy/wonder-blocks-link": "workspace:*",
21+
"@khanacademy/wonder-blocks-typography": "workspace:*",
2022
"@types/react": "^18.2.6",
2123
"react": "^18.2.0"
2224
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* This file demonstrates the wonder-blocks ESLint rule:
3+
* `@khanacademy/wonder-blocks/no-invalid-bodytext-parent`
4+
* Run `pnpm lint` in this directory to see the errors.
5+
*/
6+
7+
import * as React from "react";
8+
9+
import {Checkbox, Choice} from "@khanacademy/wonder-blocks-form";
10+
import {
11+
Heading,
12+
HeadingLarge,
13+
BodyText,
14+
} from "@khanacademy/wonder-blocks-typography";
15+
import {addStyle} from "@khanacademy/wonder-blocks-core";
16+
17+
const StyledButton = addStyle("button");
18+
const StyledP = addStyle("p");
19+
const StyledLabel = addStyle("label");
20+
const StyledH1 = addStyle("h1");
21+
22+
// ✅ Valid: BodyText with an inline tag is safe anywhere
23+
export function ValidExamples() {
24+
return (
25+
<>
26+
{/* Inline tag inside a heading */}
27+
<h1>
28+
Title <BodyText tag="sup">1</BodyText>
29+
</h1>
30+
31+
{/* Inline tag inside a button */}
32+
<button>
33+
<BodyText tag="span">Click me</BodyText>
34+
</button>
35+
36+
{/* Inline tag inside a form component prop */}
37+
<Choice label={<BodyText tag="span">Option A</BodyText>} value="" />
38+
39+
{/* Standalone BodyText in a block container */}
40+
<div>
41+
<BodyText>Paragraph text</BodyText>
42+
</div>
43+
</>
44+
);
45+
}
46+
47+
// ❌ Invalid: BodyText (renders as <p>) inside WB form component props
48+
export function InvalidFormComponents() {
49+
return (
50+
<>
51+
<Choice value="" label={<BodyText>Option A</BodyText>} />
52+
<Choice value="" label="" description={<BodyText>More details</BodyText>} />
53+
<Checkbox
54+
checked={false}
55+
onChange={() => {}}
56+
label={<BodyText>Check me</BodyText>}
57+
/>
58+
</>
59+
);
60+
}
61+
62+
// ❌ Invalid: BodyText inside button elements
63+
export function InvalidButtonElements() {
64+
return (
65+
<>
66+
<button>
67+
<BodyText>Click me</BodyText>
68+
</button>
69+
<StyledButton>
70+
<BodyText>Click me</BodyText>
71+
</StyledButton>
72+
</>
73+
);
74+
}
75+
76+
// ❌ Invalid: BodyText inside paragraph elements
77+
export function InvalidParagraphElements() {
78+
return (
79+
<>
80+
<p>
81+
<BodyText>Nested paragraph</BodyText>
82+
</p>
83+
<StyledP>
84+
<BodyText>Nested paragraph</BodyText>
85+
</StyledP>
86+
</>
87+
);
88+
}
89+
90+
// ❌ Invalid: BodyText inside label elements
91+
export function InvalidLabelElements() {
92+
return (
93+
<>
94+
<label>
95+
<BodyText>Label text</BodyText>
96+
</label>
97+
<StyledLabel>
98+
<BodyText>Label text</BodyText>
99+
</StyledLabel>
100+
</>
101+
);
102+
}
103+
104+
// ❌ Invalid: BodyText inside heading elements
105+
export function InvalidHeadingElements() {
106+
return (
107+
<>
108+
<Heading>
109+
<BodyText>Sub text</BodyText>
110+
</Heading>
111+
<HeadingLarge>
112+
<BodyText>Sub text</BodyText>
113+
</HeadingLarge>
114+
<h1>
115+
<BodyText>Sub text</BodyText>
116+
</h1>
117+
<StyledH1>
118+
<BodyText>Sub text</BodyText>
119+
</StyledH1>
120+
</>
121+
);
122+
}
123+
124+
// ❌ Invalid: BodyText nested inside another BodyText (both render as <p>)
125+
export function InvalidNestedBodyText() {
126+
return (
127+
<BodyText>
128+
Outer <BodyText>inner</BodyText>
129+
</BodyText>
130+
);
131+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# no-invalid-bodytext-parent
2+
3+
Disallow `BodyText` with a block-level tag inside elements that cannot contain block-level content.
4+
5+
## Rule Details
6+
7+
`BodyText` renders as a `<p>` element by default. HTML does not allow block-level
8+
elements like `<p>` inside inline contexts such as `<button>`, `<label>`, `<p>`,
9+
or heading elements. Placing `BodyText` inside these elements produces invalid
10+
markup that browsers must silently repair, which can cause layout or accessibility
11+
issues.
12+
13+
Add `tag="span"` (or another inline tag) to `BodyText` to make it render as an
14+
inline element that is valid in these contexts.
15+
16+
This rule provides an auto-fix that adds `tag="span"` when the fix is
17+
unambiguous. Dynamic `tag` expressions (e.g. `tag={isLabel ? "span" : "p"}`) are
18+
not auto-fixed to avoid silently corrupting conditional logic.
19+
20+
Examples of **incorrect** code:
21+
22+
```tsx
23+
/* BodyText inside a <button> */
24+
<button>
25+
<BodyText>Click me</BodyText>
26+
</button>
27+
28+
/* BodyText inside a WB Button component */
29+
<Button onClick={handleClick}>
30+
<BodyText>Click me</BodyText>
31+
</Button>
32+
33+
/* BodyText inside a <label> */
34+
<label>
35+
<BodyText>Email address</BodyText>
36+
<input type="email" />
37+
</label>
38+
39+
/* BodyText inside a WB form component */
40+
<Choice label={<BodyText>Option A</BodyText>} value="a" />
41+
<LabeledField label={<BodyText>Email</BodyText>} field={<input />} />
42+
<RadioGroup description={<BodyText>Choose one</BodyText>} />
43+
<CheckboxGroup description={<BodyText>Select all that apply</BodyText>} />
44+
45+
/* BodyText nested inside another BodyText (both render as <p>) */
46+
<BodyText>
47+
Outer text <BodyText>inner text</BodyText>
48+
</BodyText>
49+
50+
/* BodyText inside a heading */
51+
<h2>
52+
Section <BodyText>subtitle</BodyText>
53+
</h2>
54+
<StyledH2>
55+
Section <BodyText>subtitle</BodyText>
56+
</StyledH2>
57+
<Heading>
58+
Section <BodyText>subtitle</BodyText>
59+
</Heading>
60+
```
61+
62+
Examples of **correct** code:
63+
64+
```tsx
65+
/* Inline tag makes BodyText valid inside a button */
66+
<button>
67+
<BodyText tag="span">Click me</BodyText>
68+
</button>
69+
70+
/* Inline tag makes BodyText valid inside a WB Button */
71+
<Button onClick={handleClick}>
72+
<BodyText tag="span">Click me</BodyText>
73+
</Button>
74+
75+
/* Inline tag makes BodyText valid inside a label */
76+
<label>
77+
<BodyText tag="span">Email address</BodyText>
78+
<input type="email" />
79+
</label>
80+
81+
/* Inline tag makes BodyText valid inside a WB form component */
82+
<Choice label={<BodyText tag="span">Option A</BodyText>} value="a" />
83+
84+
/* Outer BodyText uses a block-container tag, allowing block children */
85+
<BodyText tag="div">
86+
Outer text <BodyText>inner text</BodyText>
87+
</BodyText>
88+
89+
/* Inline tag makes BodyText valid inside a heading */
90+
<h2>
91+
Section <BodyText tag="sup">note</BodyText>
92+
</h2>
93+
94+
/* BodyText is fine at the top level or inside block containers */
95+
<BodyText>Standalone paragraph</BodyText>
96+
<div>
97+
<BodyText>Inside a div</BodyText>
98+
</div>
99+
```
100+
101+
## When Not To Use It
102+
103+
Disable this rule only if you are intentionally rendering invalid HTML and have a
104+
specific reason to do so. In most cases the auto-fix resolves the issue without
105+
any manual effort.

packages/eslint-plugin-wonder-blocks/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@
2222
"prepublishOnly": "../../utils/publish/package-pre-publish-check.sh"
2323
},
2424
"dependencies": {
25-
"@typescript-eslint/utils": "^8.58.0"
25+
"@typescript-eslint/utils": "^8.59.0"
2626
},
2727
"peerDependencies": {
2828
"eslint": "^8.57.0"
2929
},
3030
"devDependencies": {
3131
"@khanacademy/wb-dev-build-settings": "workspace:*",
32-
"@typescript-eslint/typescript-estree": "^8.58.2",
32+
"@typescript-eslint/typescript-estree": "^8.59.0",
33+
"@typescript-eslint/rule-tester": "^8.59.0",
3334
"eslint": "^8.57.0"
3435
}
3536
}

packages/eslint-plugin-wonder-blocks/src/configs/strict.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@ export default {
55
rules: {
66
...recommended.rules,
77
"@khanacademy/wonder-blocks/no-custom-tab-role": "error",
8+
"@khanacademy/wonder-blocks/no-invalid-bodytext-parent": "error",
89
},
910
};

packages/eslint-plugin-wonder-blocks/src/rules/__tests__/no-invalid-bodytext-parent.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ ruleTester.run(ruleName, rule, {
104104
{code: `<h1><BodyText tag="sup">Sup</BodyText></h1>`},
105105
{code: `<h2><BodyText tag="span">Sub</BodyText></h2>`},
106106

107+
// BodyText with an inline tag inside StyledH* elements
108+
{code: `<StyledH1><BodyText tag="sup">Sup</BodyText></StyledH1>`},
109+
{code: `<StyledH2><BodyText tag="span">Sub</BodyText></StyledH2>`},
110+
107111
// BodyText with other inline tag variants
108112
{code: `<button><BodyText tag="code">Code</BodyText></button>`},
109113
{code: `<label><BodyText tag="strong">Bold</BodyText></label>`},
@@ -317,6 +321,23 @@ ruleTester.run(ruleName, rule, {
317321
output: `<h6><BodyText tag="span">Sub text</BodyText></h6>`,
318322
},
319323

324+
// --- StyledH* elements ---
325+
{
326+
code: `<StyledH1><BodyText>Sub text</BodyText></StyledH1>`,
327+
errors: [{messageId: "nestedInHeading"}],
328+
output: `<StyledH1><BodyText tag="span">Sub text</BodyText></StyledH1>`,
329+
},
330+
{
331+
code: `<StyledH2><BodyText>Sub text</BodyText></StyledH2>`,
332+
errors: [{messageId: "nestedInHeading"}],
333+
output: `<StyledH2><BodyText tag="span">Sub text</BodyText></StyledH2>`,
334+
},
335+
{
336+
code: `<StyledH6><BodyText>Sub text</BodyText></StyledH6>`,
337+
errors: [{messageId: "nestedInHeading"}],
338+
output: `<StyledH6><BodyText tag="span">Sub text</BodyText></StyledH6>`,
339+
},
340+
320341
// --- BodyText with a non-inline tag is still invalid in restricted
321342
// parents; the fix replaces the existing tag value. ---
322343
{

packages/eslint-plugin-wonder-blocks/src/rules/jsx-utils.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,14 @@ export const BLOCK_CONTAINER_TAGS = new Set([
5454
* Wonder Blocks form components that internally render a <label> or similar
5555
* inline-context element around their content.
5656
*/
57-
export const WB_FORM_COMPONENTS = new Set(["Choice", "Checkbox", "Radio"]);
57+
export const WB_FORM_COMPONENTS = new Set([
58+
"Choice",
59+
"Checkbox",
60+
"Radio",
61+
"LabeledField",
62+
"RadioGroup",
63+
"CheckboxGroup",
64+
]);
5865

5966
/**
6067
* Wonder Blocks Button component names. Buttons are inline contexts and
@@ -63,7 +70,7 @@ export const WB_FORM_COMPONENTS = new Set(["Choice", "Checkbox", "Radio"]);
6370
export const WB_BUTTON_COMPONENTS = new Set(["Button", "ActivityButton"]);
6471

6572
/**
66-
* HTML heading element names.
73+
* HTML heading element names and their styled-component equivalents.
6774
*/
6875
export const HTML_HEADING_ELEMENTS = new Set([
6976
"h1",
@@ -72,6 +79,12 @@ export const HTML_HEADING_ELEMENTS = new Set([
7279
"h4",
7380
"h5",
7481
"h6",
82+
"StyledH1",
83+
"StyledH2",
84+
"StyledH3",
85+
"StyledH4",
86+
"StyledH5",
87+
"StyledH6",
7588
]);
7689

7790
/**
@@ -98,7 +111,7 @@ export function getAttributeStringValue(
98111
(a): a is TSESTree.JSXAttribute =>
99112
a.type === "JSXAttribute" &&
100113
a.name.type === "JSXIdentifier" &&
101-
(a.name as TSESTree.JSXIdentifier).name === attributeName,
114+
a.name.name === attributeName,
102115
);
103116

104117
if (!attr?.value) {
@@ -113,7 +126,7 @@ export function getAttributeStringValue(
113126
attr.value.type === "JSXExpressionContainer" &&
114127
attr.value.expression.type === "Literal"
115128
) {
116-
return String((attr.value.expression as TSESTree.Literal).value);
129+
return String(attr.value.expression.value);
117130
}
118131

119132
return null;

0 commit comments

Comments
 (0)