From 3cc1fe0906c36bfb36ce4396a5f1f57820bcb877 Mon Sep 17 00:00:00 2001 From: Diogo Esteves Date: Fri, 30 Oct 2026 17:48:42 +0000 Subject: [PATCH 1/3] fix(lint): unknown family parts, React Aria composition hints, and a stopped ESLint loader - An unknown part of a family (`DialogContent` where Dialog is the content, as on React Aria) gets the family's component and its parts, not the sibling spelled closest (`DialogFooter`). A misspelled part (`DialogTitel`) is still taken for that part, with the rename. - `asChild` on a trigger without asChild or render says it takes the element that opens it as a child. On React Aria's function `render`, asChild, as, component and href no longer get Base UI's render element advice. - The ESLint loader worker answers every waiting call from its exit handler, so a worker that stops (process.exit in a config, an uncaught error) fails the lint at once instead of after 120 s. Roadmap item OS2-59. --- .changeset/message-polish.md | 9 +++ src/eslint/worker.ts | 34 +++++++-- src/lint/rules/components.ts | 84 ++++++++++++++++++---- test/component-guesses.test.ts | 119 ++++++++++++++++++++++++++++++++ test/eslint-worker-exit.test.ts | 63 +++++++++++++++++ 5 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 .changeset/message-polish.md create mode 100644 test/component-guesses.test.ts create mode 100644 test/eslint-worker-exit.test.ts diff --git a/.changeset/message-polish.md b/.changeset/message-polish.md new file mode 100644 index 00000000..3844eb6a --- /dev/null +++ b/.changeset/message-polish.md @@ -0,0 +1,9 @@ +--- +'onsystem': patch +--- + +An unknown component that looks like a part of a family the design system has, but is like none of its parts, is no longer taken for the part spelled closest. `DialogContent` in a design system on React Aria, whose `Dialog` is the content, got "Did you mean ?"; it now gets "Did you mean , or one of its parts: DialogTrigger, DialogHeader, DialogFooter, DialogTitle?", with no rename. A misspelled part (`DialogTitel`) is still taken for that part. + +`asChild` on a React Aria trigger (``) now says that the trigger takes the element that opens it as a child. On a React Aria component whose `render` is a function, `asChild`, `as`, `component` and `href` no longer get Base UI's advice (`render={}`), which React Aria's `render` does not take: they say to use a link component for a link. + +The ESLint plugin no longer waits two minutes on each file when its loader worker stops before it answers, as `process.exit()` in an `onsystem.config.mjs` or an uncaught error makes it: the lint fails at once with "the design system's loader stopped before it answered", and why. diff --git a/src/eslint/worker.ts b/src/eslint/worker.ts index 372aa927..c775178b 100644 --- a/src/eslint/worker.ts +++ b/src/eslint/worker.ts @@ -249,11 +249,37 @@ export async function load(request: LoadRequest): Promise { }; } -parentPort?.on('message', ({ request, signal, port }: Call) => { +/** Calls not answered yet: the answer wakes the ESLint thread that waits for it. */ +const pending = new Set(); + +function answer({ signal, port }: Call, response: LoadResponse): void { + port.postMessage(response); + Atomics.store(signal, 0, 1); + Atomics.notify(signal, 0); +} + +if (parentPort) { + // A worker that stops before it answers (`process.exit()` in a config written in code, an + // uncaught error) left ESLint waiting the bridge's whole timeout on every file, with a message + // that blamed a slow load. Its exit handlers still run, so they answer with why it stopped. + let crash: unknown; + process.on('uncaughtExceptionMonitor', (error) => { + crash = error; + }); + process.on('exit', (code) => { + const why = crash instanceof Error ? `: ${crash.message}` : ` (exit code ${String(code)})`; + for (const call of pending) { + answer(call, { error: `the design system's loader stopped before it answered${why}` }); + } + pending.clear(); + }); +} + +parentPort?.on('message', (call: Call) => { + const { request } = call; + pending.add(call); const reply = (response: LoadResponse) => { - port.postMessage(response); - Atomics.store(signal, 0, 1); - Atomics.notify(signal, 0); + if (pending.delete(call)) answer(call, response); }; load({ ...request, file: path.resolve(request.cwd, request.file) }).then( reply, diff --git a/src/lint/rules/components.ts b/src/lint/rules/components.ts index c3969e9d..aead3480 100644 --- a/src/lint/rules/components.ts +++ b/src/lint/rules/components.ts @@ -294,6 +294,44 @@ function enclosingComponent(node: ts.Node): string | undefined { // ─── no-unknown-component ─────────────────────────────────────────────────── +/** + * The component an unknown name most likely means: the closest spelling (`DialogTitel` is + * `DialogTitle`), unless the name is a part of a family that has no such part. `DialogContent` + * in a system whose Dialog is the content (React Aria) was spelled closest to another part, + * `DialogFooter`, which means something else: the family's component is named instead, with its + * parts. Undefined when nothing is close. + */ +function guessComponent( + name: string, + target: RuleContext['target'], +): { name: string; parts?: string[] } | undefined { + const guess = closest(name, target.names()); + // The longest component name that `name` starts with, before another word: `Dialog`. + const family = target + .names() + .filter( + (n) => n.length < name.length && name.startsWith(n) && /[A-Z]/.test(name[n.length] ?? ''), + ) + .sort((a, b) => b.length - a.length)[0]; + if (family === undefined) return guess === undefined ? undefined : { name: guess }; + // Another part of the family, spelled like this one but for the family's name, is a typo of it. + const part = name.slice(family.length); + const sibling = guess !== undefined && guess !== family && guess.startsWith(family); + if (sibling && closest(part, [guess.slice(family.length)], 1 / 2) !== undefined) { + return { name: guess }; + } + const parts = target.components.get(family)?.subcomponents ?? []; + if (parts.length) return { name: family, parts }; + // A part unlike any of the family's, of a component without parts (`ButtonGroup` next to + // `Button`), is another component: not a sibling's name. + return guess === undefined || sibling ? undefined : { name: guess }; +} + +/** `, or one of its parts: DialogTrigger, DialogFooter` for a family's component. */ +function partsOf(guess: { parts?: string[] }): string { + return guess.parts ? `, or one of its parts: ${guess.parts.join(', ')}` : ''; +} + export const noUnknownComponent: Rule = { id: 'no-unknown-component', description: @@ -338,36 +376,37 @@ export const noUnknownComponent: Rule = { } if (resolution.kind === 'missing-export') { - const guess = closest(resolution.name, target.names()); + const guess = guessComponent(resolution.name, target); context.report({ ...range, message: `"${resolution.name}" is not a design-system component (imported from "${resolution.source}").` + (guess - ? ` Did you mean <${guess}>?` + ? ` Did you mean <${guess.name}>${partsOf(guess)}?` : ' Use search_components to find an existing one.'), - suggestion: guess ? `<${guess}>` : undefined, + suggestion: guess ? `<${guess.name}>` : undefined, }); continue; } if (resolution.kind === 'unresolved') { - const guess = closest(resolution.name, target.names()); - const component = guess ? target.components.get(guess) : undefined; + const guess = guessComponent(resolution.name, target); + const component = guess ? target.components.get(guess.name) : undefined; if (guess && component) { // A close name is only a safe rename when it is in scope: `` in a // fragment without imports may be an app component the agent has not // imported yet, not a typo of ``. - const binding = context.analysis.imports.get(guess.split('.')[0] ?? guess); + const binding = context.analysis.imports.get(guess.name.split('.')[0] ?? guess.name); const inScope = binding !== undefined && target.isDesignSystemImport(binding.source, context.file); context.report({ ...range, message: inScope - ? `Unknown component <${resolution.name}>. Did you mean <${guess}>?` - : `Unknown component <${resolution.name}>. Did you mean <${guess}> (${importLine(component, context)})?`, - suggestion: `<${guess}>`, - fix: inScope ? renameTag(context, element, guess) : undefined, + ? `Unknown component <${resolution.name}>. Did you mean <${guess.name}>${partsOf(guess)}?` + : `Unknown component <${resolution.name}>. Did you mean <${guess.name}> (${importLine(component, context)})${partsOf(guess)}?`, + suggestion: `<${guess.name}>`, + // The family's component is another component, not a spelling of this one. + fix: inScope && !guess.parts ? renameTag(context, element, guess.name) : undefined, }); } else if (context.analysis.imports.size > 0) { // Only for whole modules: fragments without imports routinely use @@ -448,17 +487,36 @@ const FOREIGN_COMPOSITION = new Set(['href', 'as', 'component']); * element; shadcn/ui ships both, and agents mix them up, or bring `as`, `component` * or `href` from other libraries. Renaming the prop is not enough (`asChild` is a * boolean, and a button with an `href` is still no link), so this explains the - * other way instead of offering a fix. + * other way instead of offering a fix. React Aria has neither: a trigger takes the + * element that opens it as a child, and `render` is a function that returns an + * element of the same kind, so a link is a link component. */ function compositionHint( name: string, known: Set, tag: string, -): { prop: string; text: string } | undefined { + component: ComponentInfo, +): { prop?: string; text: string } | undefined { const radix = `compose with asChild and a single child`; const radixExample = `<${tag} asChild>…`; const base = `compose with render`; const baseExample = `<${tag} render={}>…`; + const render = component.props.find((p) => p.name === 'render'); + const ariaRender = render !== undefined && /^(?:DOMRenderFunction<|\(props\b)/.test(render.type); + const composing = ['asChild', 'as', 'component', 'render'].includes(name); + if (composing && !known.has('asChild') && !known.has('render') && /Trigger$/.test(tag)) { + return { + text: `It takes the element that opens it as a child: <${tag}>….`, + }; + } + if (ariaRender && (composing || name === 'href')) { + return { + text: + name === 'href' + ? 'For a link, use a link component: React Aria’s render only swaps in an element of the same kind.' + : `React Aria components take no ${name}. For a link, use a link component: render={(props) => …} only swaps in an element of the same kind.`, + }; + } if (name === 'href') { if (known.has('asChild')) { return { prop: 'asChild', text: `For a link, Radix components ${radix}: ${radixExample}.` }; @@ -550,7 +608,7 @@ export const noUnknownProp: Rule = { ) { continue; } - const composition = compositionHint(name, known, element.tag); + const composition = compositionHint(name, known, element.tag, component); if (composition) { context.report({ start: attribute.name.getStart(context.sourceFile), diff --git a/test/component-guesses.test.ts b/test/component-guesses.test.ts new file mode 100644 index 00000000..b5bd6bbe --- /dev/null +++ b/test/component-guesses.test.ts @@ -0,0 +1,119 @@ +import path from 'node:path'; + +import { beforeAll, describe, expect, it } from 'vitest'; + +import type { DesignSystem } from '../src/design-system.js'; +import type { Diagnostic } from '../src/types.js'; +import { fixture, load, loadOnce, TSCONFIG } from './helpers.js'; + +// What no-unknown-component and no-unknown-prop say in a design system that composes neither the +// Radix way (asChild) nor the Base UI way (a render element): React Aria Components, as in a +// shadcn/ui style built on them, where Dialog is the content and DialogTrigger takes its button +// as a child. + +const RAC_ROOT = path.resolve(import.meta.dirname, 'fixtures/rac-ui'); + +/** A flat Dialog family, as shadcn/ui on React Aria writes it: no DialogContent. */ +const ARIA_DIALOG = { + 'tsconfig.json': TSCONFIG, + 'components/ui/dialog.tsx': `import * as React from "react" +export function Dialog(props: { children?: React.ReactNode }) { return
{props.children}
} +export function DialogTrigger(props: { children?: React.ReactNode; isOpen?: boolean }) { return <>{props.children} } +export function DialogHeader(props: { children?: React.ReactNode }) { return
{props.children}
} +export function DialogFooter(props: { children?: React.ReactNode }) { return
{props.children}
} +export function DialogTitle(props: { children?: React.ReactNode }) { return

{props.children}

}`, + 'components/ui/button.tsx': `import * as React from "react" +export function Button(props: React.ComponentProps<"button">) { return `; + expect(findings(aria, code, 'no-unknown-component').map((d) => d.message)).toEqual([ + '"ButtonGroup" is not a design-system component (imported from "@/components/ui/button"). Use search_components to find an existing one.', + ]); + }); +}); + +describe('composition in React Aria Components', () => { + let rac: DesignSystem; + beforeAll(async () => { + rac = await loadOnce(RAC_ROOT); + }); + + it('says a trigger takes the element that opens it as a child, for asChild', () => { + const code = `import { Button } from "@/components/ui/button" +import { DialogTrigger } from "@/components/ui/dialog" +export default () => `; + const [d] = findings(rac, code, 'no-unknown-prop'); + expect(d?.message).toBe( + ' has no prop "asChild". It takes the element that opens it as a child: ….', + ); + expect(d?.suggestion).toBeUndefined(); + expect(d?.fix).toBeUndefined(); + }); + + it("does not point at Base UI's render element, which React Aria's render function is not", () => { + const code = `import { Button } from "@/components/ui/button" +import Link from "next/link" +export default () => ( + <> + + + +)`; + expect(findings(rac, code, 'no-unknown-prop').map((d) => [d.message, d.suggestion])).toEqual([ + [ + '`; @@ -96,6 +142,23 @@ export default () => { + const code = `import { Button } from "@/components/ui/button" +import { ContextMenuTrigger, Menu, MenuItem, MenuTrigger, SubmenuTrigger } from "@/components/ui/menu" +export default () => ( + <> + Edit + ShareMail +
Right-click here
Copy + +)`; + expect(findings(rac, code, 'no-unknown-prop').map((d) => d.message)).toEqual([ + ' has no prop "asChild". It takes the element that opens it as a child: ….', + ' has no prop "asChild". It takes the element that opens it as a child: …….', + ' has no prop "asChild". It takes the area that opens the menu on a right-click as a child: ….', + ]); + }); + it("does not point at Base UI's render element, which React Aria's render function is not", () => { const code = `import { Button } from "@/components/ui/button" import Link from "next/link" @@ -107,7 +170,7 @@ export default () => ( )`; expect(findings(rac, code, 'no-unknown-prop').map((d) => [d.message, d.suggestion])).toEqual([ [ - ' } +export function CarouselNext(props: { children?: React.ReactNode }) { return }`, + }, + { nodeModules: true }, + ), + ); + const code = `import { Card, CardTitle } from "@/components/ui/card" +import { Carousel, CarouselPrevious } from "@/components/ui/carousel" +export default () => ( + <> + HiBody + + +)`; + const found = findings(families, code, 'no-unknown-component'); + expect(found.map((d) => [d.message, d.fix?.[0]?.text])).toEqual([ + [ + 'Unknown component . Did you mean , or one of its parts: CardHeader, CardTitle, CardContent?', + undefined, + ], + [ + 'Unknown component . Did you mean (import { CardContent } from "@/components/ui/card")?', + undefined, + ], + [ + 'Unknown component . Did you mean , or one of its parts: CarouselContent, CarouselItem, CarouselPrevious, CarouselNext?', + undefined, + ], + ]); + }); + it('does not take a component of its own for a part of a component without parts', () => { const code = `import { Button, ButtonGroup } from "@/components/ui/button" export default () => `; @@ -195,6 +242,31 @@ export default () => Docs { + it('gets no hint to put the button inside, which would nest one button in another', async () => { + const own = await load( + fixture( + { + 'tsconfig.json': TSCONFIG, + 'components/ui/dialog.tsx': `import * as React from "react" +export function Dialog(props: { children?: React.ReactNode }) { return
{props.children}
} +export function DialogTrigger(props: React.ComponentProps<"button">) { return `; + expect(findings(own, code, 'no-unknown-prop').map((d) => [d.message, d.suggestion])).toEqual([ + [' has no prop "asChild".', undefined], + ]); + }); +}); + describe('a render function of the design system’s own', () => { it('keeps the render advice, since it may return a link', async () => { // shadcn/ui's attachment on React Aria: its trigger is a button with a render function of diff --git a/test/eslint-worker-exit.test.ts b/test/eslint-worker-exit.test.ts index e23bb930..8a605799 100644 --- a/test/eslint-worker-exit.test.ts +++ b/test/eslint-worker-exit.test.ts @@ -1,3 +1,4 @@ +import fs from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -10,8 +11,9 @@ import { fixture } from './helpers.js'; // The ESLint plugin waits for its loader worker to answer. A worker that stopped before it // answered (`process.exit()` in a config written in code, an uncaught error) left every file of -// the lint waiting the bridge's whole timeout, 120 seconds, and then blamed a slow load. Built as it ships, into a folder of -// its own, as in eslint-bridge.test.ts, since test files run in parallel. +// the lint waiting the bridge's whole timeout, 120 seconds, and then blamed a slow load. Built as +// it ships, into a folder of its own, as in eslint-bridge.test.ts, since test files run in +// parallel. const REPO = path.resolve(import.meta.dirname, '..'); const OUT = path.join(REPO, 'node_modules/.cache/onsystem-eslint-worker-exit-test'); @@ -85,4 +87,43 @@ describe('a loader worker that stops before it answers', () => { }, 400_000, ); + + // A file posted while the worker is busy, which then stops before it reads it: the call is + // still on the worker's port, not among those it is answering. + it('fails a file it never read, posted while it was busy', async () => { + const root = fixture({ + // Busy for 3 seconds once the test writes `stop`, then gone. + 'onsystem.config.mjs': `import fs from "node:fs"; +const stop = new URL("./stop", import.meta.url); +const timer = setInterval(() => { + if (!fs.existsSync(stop)) return; + clearInterval(timer); + const end = Date.now() + 3000; + while (Date.now() < end); + process.exit(3); +}, 20); +export default {}; +`, + 'components/ui/button.tsx': + 'export function Button(props: React.ComponentProps<"button">) { return