BodyText linting part 2: no-invalid-bodytext-children#3015
BodyText linting part 2: no-invalid-bodytext-children#3015marcysutton wants to merge 10 commits intomainfrom
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🦋 Changeset detectedLatest commit: 2a2725d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
Size Change: +819 B (+0.66%) Total Size: 124 kB 📦 View Changed
ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (c1a8338) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR3015"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR3015 |
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-juaikzgwqi.chromatic.com/ Chromatic results:
|
marcysutton
changed the title
marcysutton
force-pushed
the
WB-2247-bodytext-lint
branch
5 times, most recently
from
4bc6115 to
f554d3f
Compare
marcysutton
force-pushed
the
WB-2248-bodytext-children
branch
from
4199e63 to
5e7182a
Compare
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
marcysutton
changed the title
|
Ok I'm marking this one for review to see if we want to keep it. There weren't any violations in Here's the integration PR: https://github.com/Khan/frontend/pull/10738 |
|
The navigation tabs playtesting stories failed again even though I updated from |
|
@claude review once |
| --- | ||
| "@khanacademy/eslint-plugin-wonder-blocks": minor | ||
| --- | ||
|
|
||
| Add new Typography lint rule for no-invalid-bodytext-parent to avoid nested paragraphs |
There was a problem hiding this comment.
🔴 This PR adds .changeset/healthy-peas-push.md with a description for the no-invalid-bodytext-parent rule, but that changeset was already consumed by Version Packages PR #3018 and shipped in @khanacademy/eslint-plugin-wonder-blocks@0.3.0. Re-introducing it here will cause a second minor version bump with a duplicate CHANGELOG entry for an already-released feature. Please delete .changeset/healthy-peas-push.md before merging — .changeset/brown-tips-draw.md is the correct changeset for this PR.
Extended reasoning...
What the bug is
.changeset/healthy-peas-push.md is being re-added in this PR with the body "Add new Typography lint rule for no-invalid-bodytext-parent to avoid nested paragraphs". This describes the previous Bodytext lint rule (the parent rule shipped in part 1), not the new no-invalid-bodytext-children rule that this PR introduces.
How it manifests
When this PR merges:
- The changesets bot will pick up both
.changeset/brown-tips-draw.md(the legitimate one for this PR) and.changeset/healthy-peas-push.md(the stale duplicate). - On the next Version Packages run, both changesets will be consumed and folded into one minor bump for
@khanacademy/eslint-plugin-wonder-blocks. - The resulting CHANGELOG entry will list both "Add new no-invalid-bodytext-children rule…" and "Add new Typography lint rule for no-invalid-bodytext-parent to avoid nested paragraphs" — but the second line refers to a feature that already shipped in v0.3.0, so consumers will see a misleading duplicate entry.
Step-by-step proof
git log --diff-filter=A --all -- .changeset/healthy-peas-push.mdshows the file was originally created in commit8868c54on the WB-2247 branch (the no-invalid-bodytext-parent rule).- PR Introduce lint rule: no-invalid-bodytext-parent #2997 (commit
a421ae1) merged that branch tomain, bringinghealthy-peas-push.mdalong. - Version Packages PR Version Packages #3018 (commit
e580a78) consumed and deleted.changeset/healthy-peas-push.mdwhile publishing@khanacademy/eslint-plugin-wonder-blocks@0.3.0.packages/eslint-plugin-wonder-blocks/CHANGELOG.mdline 7 records: "a421ae1: Add new Typography lint rule for no-invalid-bodytext-parent to avoid nested paragraphs" — byte-identical to the body of the changeset being re-added here. git show origin/main:.changeset/healthy-peas-push.mdfails because the file no longer exists onmain. The PR diff shows it being added from/dev/null, confirming it's a fresh re-introduction.
Why existing code doesn't prevent it
Changesets has no awareness of which descriptions have already shipped — it processes whatever .md files are present in .changeset/ at release time. The changeset-bot comment on this PR ("includes changesets to release 1 package" — Minor) reflects only that there is at least one minor changeset; it doesn't catch that one of them is stale.
Likely cause
This PR was opened on 2026-04-16, before #2997 merged. The branch likely picked up 8868c54 either directly or via rebase, and when the Version Packages PR later removed the file from main, the file remained on this feature branch.
How to fix
Simply delete .changeset/healthy-peas-push.md from this branch before merging. The legitimate changeset .changeset/brown-tips-draw.md already correctly describes the new no-invalid-bodytext-children rule and should be retained.
| | Rule | Enabled in `recommended`| Enabled in `strict` | | ||
| |------|-------------------------|---------------------| | ||
| | [`no-custom-tab-role`](docs/no-custom-tab-role.md)| |✅| | ||
| | [`no-invalid-bodytext-children`](docs/no-invalid-bodytext-children.md)|✅| | |
There was a problem hiding this comment.
🔴 The 'recommended' status of no-invalid-bodytext-children is inconsistent across three places: the README rules table (README.md:74) marks it as ✅ in recommended and blank in strict, the rule's meta declares recommended: true (no-invalid-bodytext-children.ts:32), but src/configs/recommended.ts has rules: {} and the rule is only added to src/configs/strict.ts. So users extending plugin:@khanacademy/wonder-blocks/recommended will silently NOT get the rule, contrary to what the README documents. Easiest fix: swap the README ✅ to the strict column to match the actual config and follow the convention used by the other two rows in the table.
Extended reasoning...
What's wrong
There are three sources of truth for whether no-invalid-bodytext-children is part of the recommended preset, and they don't agree:
- README rules table (
packages/eslint-plugin-wonder-blocks/README.md:74):✅ in| [`no-invalid-bodytext-children`](docs/no-invalid-bodytext-children.md)|✅| |recommended, blank instrict. - Rule meta (
src/rules/no-invalid-bodytext-children.ts:32):recommended: true. - Actual configs:
src/configs/recommended.ts→rules: {}(empty)src/configs/strict.ts:8→'@khanacademy/wonder-blocks/no-invalid-bodytext-children': 'error'
So the rule is only enabled when consumers extend strict, but the README markets the recommended preset as the simpler entry point and tells readers this rule is in it.
Step-by-step proof a consumer is misled
- A consumer reads the README and decides on the lighter
recommendedpreset:// .eslintrc.js module.exports = { extends: ['plugin:@khanacademy/wonder-blocks/recommended'] };
- They look at the rules table at line 74 and see
no-invalid-bodytext-children✅ underrecommended. They reasonably conclude the rule will fire on their code. - They write
<BodyText><div>x</div></BodyText>. - ESLint runs against the merged config from
src/configs/recommended.ts, which hasrules: {}. Nothing fires. No warning, no error. - The same code under
plugin:@khanacademy/wonder-blocks/strictwould correctly produce thedivChilderror.
Why the existing code doesn't catch this
There's no automated check that the README table matches the configs, and the rule meta's recommended: true is used independently of the config exports. The other two rows in the table (no-custom-tab-role, no-invalid-bodytext-parent) follow the strict-only convention and happen to match the configs — so this row is the odd one out.
Addressing the refutation
A refutation noted that no-custom-tab-role.ts:19 also has recommended: true in meta despite being strict-only, suggesting this is an existing convention and the new rule simply follows it. That's a fair observation about meta.docs.recommended specifically — that field is somewhat decoupled from the actual config exports in this plugin already, so changing it isn't strictly required. However, it does not explain or justify the README discrepancy: no-custom-tab-role's row in the README table is correctly marked ✅ in strict and blank in recommended, matching its placement. The new rule's row inverts the columns relative to its actual placement, which is the clear, user-facing bug.
Impact
Documentation-only, but consequential: the README is what consumers use to choose between presets, and it currently misrepresents which preset enables this rule. Anyone following the docs to opt into the lighter recommended preset will silently miss this lint coverage.
How to fix
Pick one (the first is the smallest change and matches existing convention):
- Option A (recommended): Swap the checkmarks in README.md:74 to
| | ✅ |so the row matchesstrict-only placement, like the other two rows. Optionally also fliprecommended: true→recommended: falsein no-invalid-bodytext-children.ts:32 to matchno-invalid-bodytext-parent.ts:101. - Option B: Add
'@khanacademy/wonder-blocks/no-invalid-bodytext-children': 'error'tosrc/configs/recommended.tsso the configs match the README + meta. (This is a bigger behavior change for consumers ofrecommended.)
marcysutton
force-pushed
the
WB-2248-bodytext-children
branch
from
5e7182a to
8253206
Compare
…rule for no-invalid-bodytext-children to avoid invalid HTML nesting
marcysutton
force-pushed
the
WB-2248-bodytext-children
branch
from
8253206 to
2a2725d
Compare
I haven't reviewed the PR yet but I tried out the example in the demo project! I think it would be valuable to have still, so future instances can be prevented! One thing I noticed is that there are cases where it would get lint errors for both rules: 
Would it make sense to only report one of those rules in those cases? I also found it interesting that the error squiggly would be on the child element in some cases! I was expecting that the parent element would be the one with the lint rule violation because it's the element with invalid children elements. 
I'm also curious if there are any violations in the legacy apps. If not, we can add this rule to |
3 participants
Summary:
This new lint rule will flag invalid children within BodyText:
viewChild— always warns whenViewis a direct child ofBodyText, regardless oftagpropdivChild— warns when<div>is a direct child andBodyTextrenders as<p>(notagortag="p")paragraphChild— warns when<p>or a default<BodyText>(renders as<p>) is a direct childblockChild— warns on other block-level HTML elements (section,ul,h1–h6, etc.) and WB heading components whenBodyTextrenders as<p>tooManyChildren— warns when direct JSX element children exceed a configurable threshold (default: 5)BodyTexthas an inline or block-container tag prop setIssue: WB-2248
Test plan: