diff --git a/.changeset/package-mode-correctness.md b/.changeset/package-mode-correctness.md new file mode 100644 index 00000000..d3ea6178 --- /dev/null +++ b/.changeset/package-mode-correctness.md @@ -0,0 +1,11 @@ +--- +'onsystem': patch +--- + +A design-system package whose tokens are a Tailwind v4 theme partial (`@theme`, with no `@import "tailwindcss"`, since the app imports Tailwind) no longer turns off `no-hardcoded-spacing` in the apps that read its config. In a project on Tailwind v4 (`@tailwindcss/postcss`, `@tailwindcss/vite` or `@tailwindcss/cli` in its `package.json`, or `tailwindcss` in a range other than v3's, such as `^4`), such a theme gets Tailwind's default spacing unit and radius scale, as a stylesheet that imports Tailwind does, unless it clears them (`--spacing-*: initial`). `p-[13px]` is reported again, with `p-3` as the nearest step. + +A component imported through a subpath export that leads to a source file that does not exist, such as `` from `@acme/ui/components/tooltip` where `"./components/*": "./src/components/*.tsx"` has no `src/components/tooltip.tsx`, is reported by `no-unknown-component` in workspace apps that read the package's config, as it already was in apps that install the package. It was said not to have been checked, with advice to add the missing files to "components". An import whose export leads to a build folder (`dist/`) with no file there, as before the package is built, is still said not to have been checked, whatever its sources are named. + +In an app whose design-system package publishes neither its component sources nor their type declarations, `list_components`, `get_component` and `inspect --explain` now say so, and what its team can do, instead of telling you to check the "components" globs in the config, which would not help. + +`init` in a design-system package adds `ONSYSTEM.md` to `files` in its `package.json`, next to the config, so the teams of apps that install the package get the guide in `node_modules`, unless `files` leaves it out (`"!ONSYSTEM.md"`). diff --git a/docs/design-system-packages.md b/docs/design-system-packages.md index 151af03d..28ec07a2 100644 --- a/docs/design-system-packages.md +++ b/docs/design-system-packages.md @@ -14,7 +14,7 @@ npx -y onsystem init `init` treats the folder as a design-system package when zero config reads the components through the package's own `exports`, or when the package is named like one (`@acme/ui`, `@acme/design-system`, `acme-ui`), as apps look for it. There it writes: - **`onsystem.config.json`**, from what zero config finds in the package, as in any project ([configuration](configuration.md#config-file)). -- **`"onsystem": "./onsystem.config.json"` in `package.json`**, which declares the config to the apps. When `package.json` lists `files`, the config is added to them, so it is published with the package. +- **`"onsystem": "./onsystem.config.json"` in `package.json`**, which declares the config to the apps. When `package.json` lists `files`, the config and `ONSYSTEM.md` are added to them, so they are published with the package, unless `files` leaves `ONSYSTEM.md` out (`"!ONSYSTEM.md"`). - **`ONSYSTEM.md`**, for the apps' teams: how to see what their app finds, how to give their agents the plugin, the skill or the MCP server, and the CI check. An `ONSYSTEM.md` that is already there is kept, unless `--force` is passed. Then it checks the package once and offers a baseline, as in any project. When `files` leaves out the component or token files (a package that publishes only `dist/`), `init` says so: apps that install the package from a registry will not find them, and read the components from the type declarations it publishes instead, when it names some (`types` in `package.json`). Commit what it wrote. @@ -43,9 +43,10 @@ detected workspace package @acme/ui - **Paths come from the package.** `components`, `exclude`, `tokens` and `docs` are relative to the package's folder, wherever the app finds it: `../../packages/ui`, or `node_modules/@acme/ui`. They stay in the package when it is installed, and in the workspace when it is a workspace package, so `../tokens/theme.css` in a sibling package works. A path that leaves them is ignored, with a warning. - **`exclude` adds to the app's defaults.** The app still leaves out `node_modules`, `.d.ts` files, tests and stories, and the package's patterns are added. The package's components are found with its own list, as in the package. -- **Components.** When the config leaves `components` to zero config, the app reads the package as zero config reads a design-system package (step 3 of [zero config](configuration.md#zero-config)), whatever the package's name, and also when it is installed. When the config sets `components`, they are imported by `importPath` when it is set, else by the package's `exports`. +- **Components.** When the config leaves `components` to zero config, the app reads the package as zero config reads a design-system package (step 3 of [zero config](configuration.md#zero-config)), whatever the package's name, and also when it is installed. When the config sets `components`, they are imported by `importPath` when it is set, else by the package's `exports`. An import through an export that leads to a source file that is not there names no component: `` from `@acme/ui/components/tooltip`, where `"./components/*": "./src/components/*.tsx"` leads to no `src/components/tooltip.tsx`, is reported by `no-unknown-component`. The export is the one Node.js picks, and a `.jsx` target is there when the `.tsx` file TypeScript reads for it is. One whose export leads to a build folder with no file there (`"./*": "./dist/components/*.js"`, not built yet, whatever the extension) is said not to have been checked, since its source may be any file the build reads (`src/components/tooltip.ts`). - **What applies to the app:** the rules' severities and options, `elements`, `importPath` and `libraries`. - **What stays with the package:** `tsconfig` (the app keeps its own), `overrides` (their globs are about the package's files) and `includeDesignSystem`. +- **A Tailwind theme partial keeps Tailwind's defaults.** A theme the package ships for apps that import Tailwind themselves (`@theme`, with no `@import "tailwindcss"` in it) sets no spacing unit, and only the radii it names. In an app on Tailwind v4 (`@tailwindcss/postcss`, `@tailwindcss/vite` or `@tailwindcss/cli` in its `package.json`, or `tailwindcss` in a range other than v3's, such as `^4`), Tailwind's default spacing unit and radius scale apply too, as from a stylesheet that imports Tailwind, unless the theme clears them (`--spacing-*: initial`), so `no-hardcoded-spacing` runs. - **A missing file is not an error in the app.** A token file the config names that is not there matches nothing, since a published package ships only some of its files. In the package itself it is an error, so a typo is caught there. A project reads up to two configs, the first over the second, field by field as [an app's own config](#an-apps-own-config) says, and zero config fills in the rest: @@ -65,7 +66,7 @@ config onsystem.config.json, over node_modules/@acme/ui/onsystem.config.json ( `init --force` in an app on such a package, without a config or with one whose `components` match no file outside the package, writes a config with only `$schema` in it, for the app's own rules and overrides, and says what it builds on. In an app whose config names components of its own, it writes them again, as it does without such a package. -`"extends": false` in the app's config reads it alone. Zero config then fills in only the fields it leaves out, as in any project: it finds a package named like a design system, as a workspace package or installed in `node_modules` with `exports` that point at its component sources ([without the config](#without-the-config)), but not one named otherwise (`@acme/kit`) or one whose `exports` point at compiled code: of those, it reads only the type declarations a package publishes, when it declares its config ([what apps get](#what-apps-get)). So such a config that only changes a rule's severity can leave the app without the package's components and tokens, and `check` then says which rules did not run. Copy `components`, `importPath` and `tokens` from the package's config into the app's, with paths into the package: `node_modules/@acme/ui/src/**/*.tsx`, or `../../packages/ui/src/**/*.tsx` from a workspace app. `init` writes no `importPath` for a package that apps import by subpath (`@acme/ui/components/button`), so when the package's config has none, set it to the package's name (`@acme/ui`). Without it, `no-unknown-component`, `no-unknown-prop`, `no-unknown-variant`, and `icon-button-accessible-name` on the package's ``, with `ChevronLeft` from `@acme/ui`, is reported for having no accessible name. Without `importPath`, that button is not reported. - **The token rules check against the published tokens:** `no-hardcoded-color` when they hold colors, `no-hardcoded-spacing` and `no-hardcoded-radius` when they set a spacing scale and radii. - **`--require-design-system` fails** (exit code 2), and it is on by default in the [GitHub Action](required-check.md). diff --git a/docs/rules.md b/docs/rules.md index f228989c..6760a3cc 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -21,7 +21,7 @@ Class strings are checked where they are written: in `className` or `class`, and Color matches under ΔE 0.02 count as the same color. Under 0.1 the fix is offered, except for a Tailwind palette class: it is fixed only to a token that matches it or that is nearest to the class's own shade (of the shades in its family) and within ΔE 0.037 of it, the most Tailwind v3 and v4 differ on one shade. Another close token is suggested without a fix, and the message says why (one shade darker, or closest to another color of the same shade). Beyond 0.1 the message names the nearest token but leaves the choice to the agent. Opacity is part of the color: a token more than 0.02 apart in opacity is never a fix, so `bg-[#e7000b33]` (the red at 20%) is not fixed to an opaque `bg-destructive`, nor `dark:border-[#fff]` to a dark `border` that is 10% white. The message gives both opacities, and an opacity modifier stays on the token (`bg-[#fff]/40` → `bg-background/40`). A fix is only offered for a token of the same hue (within 15°), or a gray for a gray, whether the color is a palette class, an arbitrary value or a literal: a pale color counts as a hue by its chroma relative to its lightness, so `bg-[#fef2f2]` is never swapped for a light gray that happens to be close. Status colors (`destructive`, `warning`, `success`, `info` and their foregrounds) are only offered for nearly the same hue (within 8°), never for a gray, so a brand-orange button does not become `bg-destructive`. The other way round, a fix to another token is held back when a status color of the color's hue (within 15°) is close too and at most ΔE 0.03 further: next to a rose `brand`, `bg-[#ef4444]` may be an error, so the message offers `bg-destructive` if the color shows a status and `bg-brand` otherwise, and error text does not follow the next rebrand. When a color is a hue and no token has it, the finding is a warning that names no token: add a token for the hue, or keep the color with a suppression comment and a reason (`{/* */}` between JSX children). A chart token outside a chart, such as `text-chart-2` on a green "Active" label in a theme with no success color, is a warning with no fix: it is a token, but it changes with the chart palette and does not say what a status color says, so add a status token ([status colors](tokens.md#status-colors)) or use the token that fits. A file that draws a chart (it imports a chart library, renders `` or ``, declares `chartData`, or is named for charts) is left alone, legend dots and all, as is an element in an ``, a `
` or a `role="img"`, and every chart token when the design system's own components use them outside a chart. Tailwind's `white` and `black` (`bg-white`, `border-black`) are fixed only to a token of exactly that color, such as `bg-background` for `bg-white` in a stock shadcn/ui theme. A hardcoded background (white or black, a palette class or an arbitrary color) is not fixed to a token while hardcoded text on it stays as it is and would no longer stand out from the token in some mode. The text is white or black, a tint of them (`text-black/80`), or a color whose own finding has no fix (`text-neutral-900` held back as another shade, `text-[#000]`, an allowed class), on the element or an element inside it, in the background's state (`hover:bg-black`), or on an element around it whose text color the element inherits (no element in between sets one or is a component, which can set its own, and the element has content). So `bg-black text-white` and `bg-zinc-900 text-white` are not fixed to `bg-primary`, which is near white in shadcn/ui's dark mode, and `bg-white text-neutral-600` is not fixed to a background token that is black in dark mode. The text stands out from a color 0.25 apart in lightness (OKLCH L) or with 3:1 contrast (WCAG's minimum for large text), and the fix is held when the token takes away either one that the hardcoded background gave it: `text-neutral-600` is 0.44 lighter than black, but has 2.7:1 on it. The message names the token to change the text to with it (`text-primary-foreground`), except for a hue (`text-green-800`), which a gray foreground would not keep. The fix stays when the text's own finding fixes it too, when the class list sets the dark-mode background or text itself (`dark:bg-zinc-900`, `dark:text-white`), or when the token keeps the text standing out in each mode; a token with no dark value onsystem reads may still have one, so it counts as changing. A theme that defines `--color-white` or `--color-black` makes them its own tokens, which are not reported. White or black text, fill or stroke is a warning without a fix, since its token depends on the surface under it, and is not reported on a surface it stands out from in each mode it shows in (dark mode too, unless its class list sets its dark color): a color at least 0.25 apart in lightness (OKLCH L), a gradient, an image, a background in `style` (a swatch known only at run time counts) or a tint it stands out from (`bg-black/60` under white text), set in its class list (in the same state: `hover:bg-red-600 hover:text-white`), by its component's variants or by the nearest element around it that sets a background. A tint it does not stand out from (`bg-white/10` under white text) shows what is under it: the element around it, or else the theme's `background`. So shadcn/ui's `bg-destructive text-white` passes, and `text-white` on `bg-muted`, on `bg-white`, or on `bg-primary` where `primary` is near white in dark mode, is reported. With an opacity modifier (`bg-black/50`, `border-white/10`) white and black tint what is under them and are left alone, as they are inside an SVG ``. Among the tokens that qualify, the one made for the utility wins: `foreground` and `muted-foreground` for `text-*`, `fill-*` and `stroke-*`; surfaces such as `muted` for `bg-*`; `border`, `input` and `ring` for `border-*` and `ring-*`. Tokens named `on-*` are text colors, and so are `content-*` tokens unless the rest of the name says border or background (`content-border`, `content-background`). A close token made for another role is named in the message but never swapped in. `sidebar-*` and `chart-*` tokens are only suggested in a sidebar or a chart (by file, enclosing component or classes), and so is any other family that at most one design-system file uses, such as syntax-highlighting colors or one screen's background, outside that file's components; `dark:` classes are compared with dark-mode values. A class under a condition (`hover:`, `focus-visible:`, `md:`) is not fixed to the token the same class list has or gets without it, nor the other way round, unless the two already have exactly the same color: `bg-[#2563eb] hover:bg-[#1d4ed8]` would become `bg-primary hover:bg-primary`, and the hover would change nothing. The message suggests keeping them apart (`hover:bg-primary/90`). `dark:` is not such a condition: a `dark:` class is compared with dark-mode values, so the same token there is a fix. Nor is a variant for a pseudo-element or other elements (`placeholder:`, `*:`, `prose-a:`, `[&_svg]:`), whose color the class without it does not set. Two hardcoded classes of exactly the same color (a palette class and its hex count as one) keep their fixes too, since the condition changed nothing before either: `border-red-500 focus:border-red-500` keeps an error border red on focus. A slight change is still a change: `bg-[#fff] hover:bg-[#fafafa]` keeps its hover, though both are within 0.02 of `background`. A state class is compared with the class it starts from: a `dark:hover:` class with the list's `dark:` class, when there is one. -Likewise, a spacing or radius step that is off by more than half the value (and more than 4px) is suggested but not auto-fixed. Spacing of 1px or less is not reported unless the scale has that exact step and the value is not negative (`py-[1px]` → `py-px`): the nearest step (4px, say) would change the layout, and a negative hairline (`mt-[-1px]`, `-mt-[1px]`, `marginTop: -1`) is a deliberate border overlap. A length in `em` (`px-[0.5em]`, `padding: "0.5em"`) grows with the font, which no step of the scale does, so its nearest step is suggested and never fixed. Rules that need tokens are skipped when the design system defines none of that category; a stylesheet that imports `tailwindcss` brings Tailwind's default spacing unit and radius scale, unless the theme resets that namespace (`--spacing-*: initial`, `--radius-*: initial`), in which case only the project's own steps are suggested. Syntax errors are reported as `syntax`, and so is code nested too deeply for TypeScript's parser (thousands of levels), which is then not checked. +Likewise, a spacing or radius step that is off by more than half the value (and more than 4px) is suggested but not auto-fixed. Spacing of 1px or less is not reported unless the scale has that exact step and the value is not negative (`py-[1px]` → `py-px`): the nearest step (4px, say) would change the layout, and a negative hairline (`mt-[-1px]`, `-mt-[1px]`, `marginTop: -1`) is a deliberate border overlap. A length in `em` (`px-[0.5em]`, `padding: "0.5em"`) grows with the font, which no step of the scale does, so its nearest step is suggested and never fixed. Rules that need tokens are skipped when the design system defines none of that category; a stylesheet that imports `tailwindcss` brings Tailwind's default spacing unit and radius scale, and so does a Tailwind v4 theme partial (`@theme`, where no token stylesheet imports `tailwindcss`, such as a design-system package ships) in a project on Tailwind v4 (`@tailwindcss/postcss`, `@tailwindcss/vite` or `@tailwindcss/cli` in its `package.json`, or `tailwindcss` in a range other than v3's), unless the theme resets that namespace (`--spacing-*: initial`, `--radius-*: initial`), in which case only the project's own steps are suggested. Syntax errors are reported as `syntax`, and so is code nested too deeply for TypeScript's parser (thousands of levels), which is then not checked. An icon button's label is fixed only when the icon means one thing on a button (a trash can, pencil, magnifier, gear, copy, download, upload, share or filter icon, named plainly: `Trash2` or `TrashIcon`, not `Binary` or `FilterX`) or the code says what the button does: "More options" on a menu's trigger, and for an X, "Close" in a close part (`DialogClose`) whatever its `onClick` calls, or elsewhere "Remove", "Clear" or "Close" when every function its `onClick` calls says that one action (`removeAssertion(index)`, `clearSearch()`, `setOpen(false)`; `clearTimeout` does not count). Otherwise the message names a label without a fix, since a plus may add or expand, a chevron go to the next page or expand a row, and an X close, remove or clear: an X that resets a form and closes (`form.reset(); setOpen(false)`) is named "Close", and one that calls `closeDeleteDialog` or `onCancelDelete`, or acts only in a callback such as `onSuccess`, is named without a fix. A chevron or arrow in a collapsible's or accordion's trigger, or on a button with `aria-expanded`, `data-state` or `data-[state=open]:` classes, is named "Toggle", not "Next" or "Previous". diff --git a/src/detect.ts b/src/detect.ts index 7c305e3b..5a702d1c 100644 --- a/src/detect.ts +++ b/src/detect.ts @@ -1338,7 +1338,10 @@ function exportedFile( /** * The entries of package `dir`'s `exports` that map `subpath`, with what their `*` stands for: - * an exact subpath first, then the most specific pattern, as Node.js picks them. + * an exact subpath first, then the patterns, as Node.js orders them (`PATTERN_KEY_COMPARE`): the + * longest prefix before the `*` first, then the longest key, so `./components/*.js` comes before + * `./components/*` for `./components/tip.js`. A pattern has one `*`, which stands for at least + * one character. */ function exportMatches(dir: string, subpath: string): { middle: string; value: unknown }[] { const exports = readJson(path.join(dir, 'package.json'))?.exports; @@ -1350,25 +1353,38 @@ function exportMatches(dir: string, subpath: string): { middle: string; value: u : exports === undefined ? [] : [['.', exports]]; - const matches: { middle: string; value: unknown; rank: number }[] = []; + const matches: { middle: string; value: unknown; rank: number; length: number }[] = []; for (const [key, value] of entries) { const star = key.indexOf('*'); if (star === -1) { - if (key === subpath) matches.push({ middle: '', value, rank: Infinity }); + if (key === subpath) matches.push({ middle: '', value, rank: Infinity, length: key.length }); continue; } + if (key.lastIndexOf('*') !== star) continue; const prefix = key.slice(0, star); const suffix = key.slice(star + 1); if ( subpath.startsWith(prefix) && subpath.endsWith(suffix) && - subpath.length >= prefix.length + suffix.length + subpath.length > prefix.length + suffix.length ) { const middle = subpath.slice(prefix.length, subpath.length - suffix.length); - matches.push({ middle, value, rank: prefix.length }); + matches.push({ middle, value, rank: prefix.length, length: key.length }); } } - return matches.sort((a, b) => b.rank - a.rank); + // Only one exact subpath can match, so its rank never ties. + return matches.sort((a, b) => b.rank - a.rank || b.length - a.length); +} + +/** + * The targets package `dir`'s `exports` map `subpath` (`./components/tooltip`) to, as written: + * through the entry Node.js picks (its exact subpath, else the most specific pattern) and every + * condition but `types`, with `*` filled in. Undefined when no entry maps the subpath; none when + * the entry blocks it (`null`). + */ +export function exportedTargets(dir: string, subpath: string): string[] | undefined { + const [match] = exportMatches(dir, subpath); + return match && exportTargets(match.value).map((t) => t.replaceAll('*', match.middle)); } /** Every target of an export condition map, in order, but its types, or those of `conditions` alone. */ diff --git a/src/explain.ts b/src/explain.ts index 3c5e2249..441fab63 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -6,7 +6,7 @@ import type { DesignSystem } from './design-system.js'; import { detectProject, findDesignSystemPackage, type DetectionStep } from './detect.js'; import { defaultProjectConfig } from './extract/program.js'; import { LIBRARY_IDS, notReadYet, type LibrarySource } from './libraries/index.js'; -import { relativePath } from './util/paths.js'; +import { pathOf, relativePath } from './util/paths.js'; /** * What `inspect --explain` says about one project: every candidate zero config looked @@ -31,6 +31,8 @@ export function explainProject(ds: DesignSystem, cwd: string, indent = ''): stri ? `${display(cwd, config.configFile)}${over(config, cwd)}` : '--components'; out(` components come from ${source}, so zero config did not look`); + const unpublished = unpublishedComponents(ds); + if (unpublished) out(` ${unpublished}`); } else { const steps: DetectionStep[] = []; detectProject(config.root, config.tsconfig, steps, config.designSystemPackage); @@ -192,6 +194,29 @@ export function explainProject(ds: DesignSystem, cwd: string, indent = ''): stri return lines; } +/** + * Why a project that takes on the config of a design-system package installed in node_modules + * knows none of its components: the package publishes neither their sources nor their type + * declarations, or `"libraries": false` turns off the declarations it publishes. The one message + * for it, where a project without components would otherwise be told to check the "components" + * globs. Undefined otherwise. + */ +export function unpublishedComponents( + ds: Pick, +): string | undefined { + const declared = ds.config.designSystemPackage; + if (!declared?.installed || !ds.config.inherits || ds.model.components.length) return undefined; + const tokens = ds.model.tokens.length + ? ' The token rules still check against the tokens it publishes.' + : ''; + const name = pathOf(declared.name); + const see = 'See https://onsystem.vercel.app/docs/design-system-packages#what-apps-get'; + if (ds.config.librariesOff?.some((library) => library.package === declared.name)) { + return `${name} publishes no component sources, and "libraries": false in the config turns off the type declarations it publishes, so its components are unknown here and the component rules did not run: set "libraries" to [], which reads its declarations and no library, or remove it.${tokens} ${see}`; + } + return `${name} publishes neither its component sources nor their type declarations, so its components are unknown here and the component rules did not run: ask its team to add its sources to "files" in its package.json, or to publish type declarations.${tokens} ${see}`; +} + /** Why a library the project depends on is not read: `left out by "libraries" in onsystem.config.json`. */ export function librariesLeftOut(config: Pick, cwd: string): string { return `left out by "libraries" in ${config.configFile ? display(cwd, config.configFile) : 'the config'}`; diff --git a/src/init.ts b/src/init.ts index b024c14f..695d2061 100644 --- a/src/init.ts +++ b/src/init.ts @@ -495,7 +495,9 @@ function readManifest(root: string): Manifest | undefined { /** * Declares the config in the package's package.json, `"onsystem": "./onsystem.config.json"`, * and adds it to `files` when the package lists the files it publishes, so apps that install it - * get it too. Writes ONSYSTEM.md for the apps' teams unless one exists. Returns what it wrote. + * get it too. Writes ONSYSTEM.md for the apps' teams unless one exists, and adds it to `files` + * as well, for the teams of apps that install the package, unless `files` leaves it out + * (`!ONSYSTEM.md`). Returns what it wrote. */ function declarePackage( root: string, @@ -513,8 +515,12 @@ function declarePackage( changed = true; } const files = Array.isArray(json.files) ? (json.files as unknown[]) : undefined; - if (files && !ships(files, CONFIG_FILE)) { - files.push(CONFIG_FILE); + const added: string[] = []; + for (const file of [CONFIG_FILE, GUIDE_FILE]) { + // The apps need the config the field names; leaving the guide out is the team's call. + if (!files || ships(files, file) || (file === GUIDE_FILE && excludes(files, file))) continue; + files.push(file); + added.push(file); changed = true; } if (changed) { @@ -525,7 +531,7 @@ function declarePackage( written.push('package.json'); } notes.push( - `This is a design-system package: package.json declares its config ("${PACKAGE_FIELD}": "${value}"${files ? ', also added to "files"' : ''}), which every app that depends on ${name} reads, under its own config when it has one.`, + `This is a design-system package: package.json declares its config ("${PACKAGE_FIELD}": "${value}"${added.length ? `, with ${added.join(' and ')} added to "files"` : ''}), which every app that depends on ${name} reads, under its own config when it has one.`, ); const guide = path.join(root, GUIDE_FILE); if (force || !fs.existsSync(guide)) { @@ -925,6 +931,15 @@ function aliasFolder(alias: string, project: ProjectConfig): string | undefined return undefined; } +/** Whether npm's `files` list leaves `file` out with a negated entry: `!ONSYSTEM.md`, `!*.md`. */ +function excludes(files: readonly unknown[], file: string): boolean { + return files.some((entry) => { + if (typeof entry !== 'string' || !entry.startsWith('!')) return false; + const pattern = entry.slice(1).replace(/^\.?\//, ''); + return pattern === file || matchesGlob(file, pattern); + }); +} + /** Whether npm's `files` list publishes `file`, a path relative to the package. */ function ships(files: readonly unknown[], file: string): boolean { return files.some((entry) => { diff --git a/src/lint/target.ts b/src/lint/target.ts index af8870a1..d8bc7afe 100644 --- a/src/lint/target.ts +++ b/src/lint/target.ts @@ -1,11 +1,13 @@ +import fs from 'node:fs'; import path from 'node:path'; import type { ResolvedConfig } from '../config.js'; +import { exportedTargets } from '../detect.js'; import type { ModuleResolver } from '../modules.js'; import { TokenIndex } from '../tokens/index.js'; import { tokenFamily } from '../tokens/roles.js'; import type { ComponentInfo, DesignSystemModel, Token } from '../types.js'; -import { matchesGlob, toPosix } from '../util/paths.js'; +import { isInside, matchesGlob, relativePath, toPosix } from '../util/paths.js'; import { unique } from '../util/strings.js'; import { COLOR_UTILITY, parseUtility } from './tailwind.js'; @@ -131,6 +133,8 @@ export class LintTarget { private readonly sources?: Set; /** Published libraries the model read from type declarations: `@mui/material`. */ readonly libraries: Set; + /** The name in each package.json read, by its path: undefined where there is none. */ + private readonly manifests = new Map(); /** * `resolver` finds the module an import points at and what it exports @@ -141,7 +145,7 @@ export class LintTarget { constructor( readonly model: DesignSystemModel, readonly config: Pick & - Partial>, + Partial>, private readonly resolver?: Pick, ) { // With several design systems, the most imported one's components win a shared name: @@ -260,16 +264,58 @@ export class LintTarget { * model left out rather than an invented one: its module exists outside the * folders extraction read and exports `name`, or re-exports from such a * module (an excluded `icons/` folder, a barrel's `export * from "./icons"`). - * A module declared in a package's `exports` but not on disk counts too, - * except the design system's own import path. + * A module declared in a package's `exports` but not on disk counts too (built to `dist/`, + * not built yet), except the design system's own import path, and one whose export, as the + * package.json writes it, leads to a source file only: `@acme/ui/components/tooltip` is + * `src/components/tooltip.tsx` through `"./components/*": "./src/components/*.tsx"`, and with + * no such file it does not exist. */ outsideModel(specifier: string, name: string, fromFile = 'snippet.tsx'): boolean { const location = this.resolver?.locate(specifier, fromFile); if (!location) return false; - if (!location.file) { - return !this.importPaths.has(specifier) && specifier !== this.config.importPath; + if (location.file) return this.mayExport(location.file, name, 0); + if (this.importPaths.has(specifier) || specifier === this.config.importPath) return false; + const source = this.exportedSource(specifier); + return source === undefined || this.mayExport(source, name, 0); + } + + /** + * The module a design-system package's `exports` lead `specifier` to, relative to the root, as + * its package.json writes them: the first of the entry's targets that is on disk, else, when + * each of them is a source file, the first (which does not exist). Undefined when a target + * leads to a build folder (`"./*": "./dist/components/*.js"`, maybe not built yet, whose + * sources may be anywhere), the entry blocks the subpath (`null`), or no entry maps it. + */ + private exportedSource(specifier: string): string | undefined { + const name = packageOf(specifier); + // The package is where a file its exports lead to is. + const led = this.config.imports?.find((m) => packageOf(m.specifier) === name); + const dir = led && this.packageDir(path.resolve(this.model.root, led.target), name); + const targets = dir ? exportedTargets(dir, `.${specifier.slice(name.length)}`) : undefined; + if (!dir || !targets?.length) return undefined; + const files = targets.map((target) => path.resolve(dir, target)); + // TypeScript resolves a `.jsx` target to the `.tsx` file of that name too. + const file = + files + .flatMap((f) => (f.endsWith('.jsx') ? [f, `${f.slice(0, -4)}.tsx`] : [f])) + .find((f) => isInside(dir, f) && isFile(f)) ?? + (targets.every(isSourceFile) ? files[0] : undefined); + return file && relativePath(this.model.root, file); + } + + /** + * The folder of package `name` that `file` is in: the nearest package.json above it with a + * name, when that name is `name`. One without a name (`{"type": "module"}` in a subfolder) is + * no package's. Each package.json is read once. + */ + private packageDir(file: string, name: string): string | undefined { + for (let dir = path.dirname(file); dir !== path.dirname(dir); dir = path.dirname(dir)) { + const manifest = path.join(dir, 'package.json'); + if (!this.manifests.has(manifest)) this.manifests.set(manifest, manifestName(manifest)); + const found = this.manifests.get(manifest); + if (found !== undefined) return found === name ? dir : undefined; } - return this.mayExport(location.file, name, 0); + return undefined; } /** @@ -510,6 +556,39 @@ function isPackageName(specifier: string): boolean { : /^[a-z0-9][^/]*$/i.test(specifier); } +/** The name a package.json gives its package; undefined when it has none, or there is none. */ +function manifestName(manifest: string): string | undefined { + try { + const { name } = (JSON.parse(fs.readFileSync(manifest, 'utf8')) ?? {}) as { name?: unknown }; + return typeof name === 'string' && name ? name : undefined; + } catch { + return undefined; + } +} + +function isFile(file: string): boolean { + return fs.statSync(file, { throwIfNoEntry: false })?.isFile() ?? false; +} + +/** The package an import specifier names: `@acme/ui` for `@acme/ui/components/button`. */ +function packageOf(specifier: string): string { + return specifier + .split('/') + .slice(0, specifier.startsWith('@') ? 2 : 1) + .join('/'); +} + +/** + * An export target that is a TypeScript or JSX source file (`./src/components/tooltip.tsx`), not + * a declaration file or a file in a build folder (`./dist/tooltip.tsx`, not built yet). + */ +function isSourceFile(target: string): boolean { + if (/\.d\.[cm]?ts$/.test(target) || /^(?:\.\/)?(?:dist|build|lib|out|esm|cjs)\//.test(target)) { + return false; + } + return /\.(?:[cm]?tsx?|jsx)$/.test(target); +} + function stripExtension(file: string): string { return file.replace(/\.(?:[cm]?[jt]sx?)$/, ''); } diff --git a/src/server/index.ts b/src/server/index.ts index efe74543..96bfd92b 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -19,6 +19,7 @@ import * as z from 'zod'; import type { Catalog, CatalogEntry, ProjectSet, ServedProject } from '../catalog.js'; import { excludedBy } from '../config.js'; import type { DesignSystem } from '../design-system.js'; +import { unpublishedComponents } from '../explain.js'; import { uncheckedNotice } from '../lint/index.js'; import { notReadNotices } from '../not-read.js'; import { descriptionOf, type SearchHit } from '../search/index.js'; @@ -702,7 +703,11 @@ function register(serve: Registrar, { getDesignSystem, getProjects }: CreateServ const { ds } = found; const component = ds.getComponent(name); if (!component) { - if (!ds.roots().length) return refuse(noComponentsMessage(ds.root, found.project?.dir)); + if (!ds.roots().length) { + return refuse( + noComponentsMessage(ds.root, found.project?.dir, false, unpublishedComponents(ds)), + ); + } const names = ds.roots().map((c) => c.name); return refuse(renderNotFound(name, ds.suggestComponent(name), names)); } diff --git a/src/server/render.ts b/src/server/render.ts index 2cbdf0d8..e3866b9b 100644 --- a/src/server/render.ts +++ b/src/server/render.ts @@ -1,5 +1,6 @@ import type { Catalog, CatalogEntry, CatalogToken } from '../catalog.js'; import type { DesignSystem } from '../design-system.js'; +import { unpublishedComponents } from '../explain.js'; import { descriptionOf, type SearchHit } from '../search/index.js'; import { formatPx } from '../tokens/units.js'; import type { CheckResult, ComponentInfo, Diagnostic, PropInfo, Token } from '../types.js'; @@ -455,8 +456,17 @@ function listTail(listed: Listing): string[] { return lines; } -/** The shared explanation when a project has no design-system components to show. */ -export function noComponentsMessage(root: string, project?: string, monorepo = false): Answer { +/** + * The shared explanation when a project has no design-system components to show. `reason`, when + * known, says why instead ({@link unpublishedComponents}). + */ +export function noComponentsMessage( + root: string, + project?: string, + monorepo = false, + reason?: string, +): Answer { + if (reason) return answer([reason]); if (monorepo) { return answer([ `No design-system components found in any project of this monorepo (root: ${root}). Check the "components" globs in the config. Run \`onsystem inspect\` there to see what was looked at.`, @@ -477,7 +487,7 @@ export function renderComponentList( const roots = ds.roots(); const where = project ? ` in ${pathOf(project)}` : ''; if (!roots.length) { - return noComponentsMessage(ds.root, project); + return noComponentsMessage(ds.root, project, false, unpublishedComponents(ds)); } const listed = listComponents(roots, (c) => c, filter); if (!listed.matched) { @@ -650,7 +660,7 @@ export function renderNotFound( return answer([ `No component named ${json(name)}${where}.` + (guess ? ` Did you mean ${json(guess)}?` : '') + - ` Available: ${names.join(', ')}.`, + (names.length ? ` Available: ${names.join(', ')}.` : ''), ]); } diff --git a/src/tokens/index.ts b/src/tokens/index.ts index b12212a1..2e4d58ca 100644 --- a/src/tokens/index.ts +++ b/src/tokens/index.ts @@ -49,6 +49,7 @@ export async function loadTokens( // Stylesheets form one theme (`.dark` in one file, `:root` in another), so // they become tokens together, where the first of them was listed. const sheets: CssSheet[] = []; + const sheetFiles: string[] = []; let sheetsAt: number | undefined; for (const source of sources) { const rel = relativePath(root, source.file); @@ -72,6 +73,7 @@ export async function loadTokens( warnings.push(...result.warnings); } else { sheets.push(readCssSheet(text, rel)); + sheetFiles.push(rel); sheetsAt ??= all.length; } } catch (error) { @@ -79,6 +81,15 @@ export async function loadTokens( } } if (sheets.length) { + // A Tailwind v4 theme partial (`@theme`, without `@import "tailwindcss"`), such as a + // design-system package ships, is read by an app that imports Tailwind itself. In a project + // that runs Tailwind v4, then, its default spacing unit and radius scale apply, unless the + // partial clears them. + const partial = sheets.findIndex((s) => s.theme.size > 0 || s.resets.size > 0); + const sheet = sheets[partial]; + if (sheet && !sheets.some((s) => s.tailwind) && (await runsTailwindV4(root))) { + sheet.tailwind = sheetFiles[partial]; + } const result = cssSheetTokens(sheets); all.splice(sheetsAt ?? all.length, 0, ...result.tokens); warnings.push(...result.warnings); @@ -86,6 +97,50 @@ export async function loadTokens( return { tokens: mergeTokens(all), warnings }; } +/** The packages that run Tailwind CSS v4 in a build: PostCSS's, Vite's and its CLI. */ +const TAILWIND_V4_PACKAGES = ['@tailwindcss/postcss', '@tailwindcss/vite', '@tailwindcss/cli']; + +/** + * Whether the project at `root` runs Tailwind CSS v4, whose default theme has a spacing unit and + * the radius scale up to `rounded-4xl`: its package.json depends on a package that runs v4, or on + * `tailwindcss` in a range that is not v3's (`^3.4.1`, `~3.4`, `3.x`), which has neither. Its + * plugins (`@tailwindcss/typography`) say nothing of the version. + */ +async function runsTailwindV4(root: string): Promise { + let pkg: unknown; + try { + pkg = JSON.parse(await fs.readFile(path.join(root, 'package.json'), 'utf8')); + } catch { + return false; + } + if (typeof pkg !== 'object' || pkg === null) return false; + // What is installed comes first: `dependencies`, then `devDependencies`, then a peer range. + const lists: Record[] = []; + for (const field of ['dependencies', 'devDependencies', 'peerDependencies']) { + const dependencies = (pkg as Record)[field]; + if (typeof dependencies === 'object' && dependencies !== null) { + lists.push(dependencies as Record); + } + } + if (lists.some((list) => TAILWIND_V4_PACKAGES.some((name) => Object.hasOwn(list, name)))) { + return true; + } + const declared = lists.find((list) => Object.hasOwn(list, 'tailwindcss')); + return declared !== undefined && !beforeV4(declared.tailwindcss); +} + +/** A range of Tailwind CSS v3 or older in package.json: `^3.4.1`, `~3`, `3.x`, `^2 || ^3`. */ +function beforeV4(range: unknown): boolean { + return ( + typeof range === 'string' && + range + .split('||') + .every((alternative) => + /^(?:npm:tailwindcss@)?[\^~=v]*\s*[0-3](?:\.|$)/.test(alternative.trim()), + ) + ); +} + /** * DTCG files and the CSS generated from them often describe the same tokens. * Merge entries that share a custom property: keep the DTCG name and diff --git a/test/design-system-package.test.ts b/test/design-system-package.test.ts index 794868d0..c07a5560 100644 --- a/test/design-system-package.test.ts +++ b/test/design-system-package.test.ts @@ -102,7 +102,7 @@ describe('init in a design-system package', () => { const init = await run(['init', '--yes', '--no-cache'], kit); expect(init.code).toBe(0); expect(init.stdout).toContain( - 'This is a design-system package: package.json declares its config ("onsystem": "./onsystem.config.json", also added to "files"), which every app that depends on @acme/kit reads, under its own config when it has one.', + 'This is a design-system package: package.json declares its config ("onsystem": "./onsystem.config.json", with onsystem.config.json and ONSYSTEM.md added to "files"), which every app that depends on @acme/kit reads, under its own config when it has one.', ); expect(init.stdout).toContain( 'Apps that install @acme/kit from a registry get only what "files" in package.json publishes, which leaves out src/ (3 component files and 1 token file)', @@ -113,7 +113,7 @@ describe('init in a design-system package', () => { ); const manifest = read(path.join(kit, 'package.json')); expect(manifest.onsystem).toBe('./onsystem.config.json'); - expect(manifest.files).toEqual(['dist/**', 'onsystem.config.json']); + expect(manifest.files).toEqual(['dist/**', 'onsystem.config.json', 'ONSYSTEM.md']); expect(read(path.join(kit, 'onsystem.config.json'))).toEqual({ $schema: 'https://unpkg.com/onsystem/schema.json', components: ['src/button.tsx', 'src/card.tsx', 'src/input.tsx'], @@ -179,7 +179,7 @@ describe('init in a design-system package', () => { expect(init.stdout).not.toContain('from a registry'); expect(fs.readFileSync(path.join(kit, 'ONSYSTEM.md'), 'utf8')).toBe('Ours.\n'); expect(fs.readFileSync(path.join(kit, 'package.json'), 'utf8')).toBe( - `{\n\t"name": "@acme/kit",\n\t"private": true,\n\t"files": [\n\t\t"dist",\n\t\t"src",\n\t\t"onsystem.config.json"\n\t],\n\t"exports": {\n\t\t".": "./dist/index.mjs"\n\t},\n\t"onsystem": "./onsystem.config.json"\n}`, + `{\n\t"name": "@acme/kit",\n\t"private": true,\n\t"files": [\n\t\t"dist",\n\t\t"src",\n\t\t"onsystem.config.json",\n\t\t"ONSYSTEM.md"\n\t],\n\t"exports": {\n\t\t".": "./dist/index.mjs"\n\t},\n\t"onsystem": "./onsystem.config.json"\n}`, ); }); diff --git a/test/design-system-packages-docs.test.ts b/test/design-system-packages-docs.test.ts index 22b34153..8f71968c 100644 --- a/test/design-system-packages-docs.test.ts +++ b/test/design-system-packages-docs.test.ts @@ -330,6 +330,7 @@ describe('what apps get from a design-system package (docs/design-system-package const pkg = await initialised(); const withTypes = without(published(pkg), (file) => file.startsWith('src/')); expect(Object.keys(withTypes).sort()).toEqual([ + 'ONSYSTEM.md', 'dist/index.d.ts', 'dist/index.mjs', 'onsystem.config.json', @@ -360,14 +361,17 @@ describe('what apps get from a design-system package (docs/design-system-package '`no-unknown-component`, `no-unknown-prop` and `no-unknown-variant` check none of the components the app imports from the package', ); expect(whatAppsGet).toContain(`\`check\` says \`${notice}\`\n`); - // Agents find no component. - expect(await tool(app, 'list_components', {})).toMatch(/^No design-system components found/); + // Agents find no component, and list_components and get_component say why. + const why = + '@acme/ui publishes neither its component sources nor their type declarations, so its components are unknown here and the component rules did not run: ask its team to add its sources to "files" in its package.json, or to publish type declarations.'; + expect(await tool(app, 'list_components', {})).toContain(why); + expect(whatAppsGet).toContain( + `**\`list_components\`, \`get_component\` and \`inspect --explain\` say why:** \`${why}\``, + ); expect(await tool(app, 'search_components', { query: 'button' })).toMatch( /^No components match/, ); - expect(await tool(app, 'get_component', { name: 'Button' })).toMatch( - /^No design-system components found/, - ); + expect(await tool(app, 'get_component', { name: 'Button' })).toContain(why); // check_ui reports from the same rules. const checked = await tool(app, 'check_ui', { path: 'app/page.tsx' }); expect(new Set([...checked.matchAll(/ \[([a-z-]+)\] /g)].map((m) => m[1]))).toEqual(rules); @@ -851,10 +855,16 @@ describe("an app's own config (docs/design-system-packages.md#an-apps-own-config tokens: config.tokens.map((file) => `${prefix}/${file}`), rules: { 'no-hardcoded-color': 'warn' }, }); - // No config of its own: the package's config, imports by subpath. + // No config of its own: the package's config, imports by subpath. Its exports map + // `@acme/ui/components/badge` onto a file there is none of. const declared = await check(app(pkg, page)); expect(declared.rules).toContain('no-unknown-prop'); expect(declared.rules).toContain('no-unknown-variant'); + expect( + declared.findings.filter((d) => d.ruleId === 'no-unknown-component').map((d) => d.message), + ).toEqual([ + '"Badge" is not a design-system component (imported from "@acme/ui/components/badge"). Use search_components to find an existing one.', + ]); expect(iconLines(declared.findings)).toEqual([NATIVE_BUTTON_LINE, NATIVE_BUTTON_LINE + 1]); expect(inputImport(declared.findings)).toBe('@acme/ui/components/input'); // A config of its own that builds on the package's: the same, imports by subpath. @@ -869,10 +879,21 @@ describe("an app's own config (docs/design-system-packages.md#an-apps-own-config } expect(iconLines(bare.findings)).toEqual([NATIVE_BUTTON_LINE + 1]); expect(bare.stderr).toBe(''); - // With the package's name as importPath: the same rules as the package's config. + // With the package's name as importPath: the same rules as the package's config. In the + // workspace, whose exports declare the subpath, this config does not read them: the + // invented is said to be unchecked there. const named = app(pkg, { ...page, 'onsystem.config.json': copied('@acme/ui') }); const withName = await check(named); - expect(withName.rules).toEqual(declared.rules); + if (app === installedApp) { + expect(withName.rules).toEqual(declared.rules); + } else { + expect(withName.rules).toEqual( + new Set([...declared.rules].filter((rule) => rule !== 'no-unknown-component')), + ); + expect(withName.stderr).toContain( + ' comes from "@acme/ui/components/badge", which the design-system model does not include, so it was not checked', + ); + } expect(iconLines(withName.findings)).toEqual([NATIVE_BUTTON_LINE, NATIVE_BUTTON_LINE + 1]); expect(inputImport(withName.findings)).toBe('@acme/ui'); expect(await tool(named, 'get_component', { name: 'Button' })).toContain( diff --git a/test/init-registry-apps.test.ts b/test/init-registry-apps.test.ts index fb1681f4..44935e3f 100644 --- a/test/init-registry-apps.test.ts +++ b/test/init-registry-apps.test.ts @@ -90,9 +90,9 @@ describe('init in a design-system package tells its team what installed apps get expect(next).toBe( 'Next: commit onsystem.config.json, package.json and ONSYSTEM.md. A workspace app that depends on @northwind/ui now finds it with no config of its own: run `npx onsystem inspect` in one to see it, and give its team ONSYSTEM.md. An app that installs @northwind/ui from a registry gets no component checks until "files" publishes src/.', ); - // What it publishes is the team's call: only the config is added. + // What it publishes is the team's call: only the config and ONSYSTEM.md are added. expect(JSON.parse(read(path.join(ui, 'package.json')))).toMatchObject({ - files: ['dist', 'theme.css', 'onsystem.config.json'], + files: ['dist', 'theme.css', 'onsystem.config.json', 'ONSYSTEM.md'], }); // What it says is what such an app gets: the token rules, and no component. diff --git a/test/package-mode.test.ts b/test/package-mode.test.ts new file mode 100644 index 00000000..fb888664 --- /dev/null +++ b/test/package-mode.test.ts @@ -0,0 +1,629 @@ +import fs from 'node:fs'; +import { createRequire } from 'node:module'; +import path from 'node:path'; + +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import { afterAll, describe, expect, it } from 'vitest'; + +import { main, type Io } from '../src/cli.js'; +import { createServer } from '../src/server/index.js'; +import { renderNotFound } from '../src/server/render.js'; +import { fixture, load } from './helpers.js'; + +// An app reading a design-system package's config: a package whose tokens are a Tailwind theme +// partial, an import through a subpath export that has no file, an app whose package publishes +// no sources, and what `init` publishes for the apps' teams. + +const json = (value: unknown) => JSON.stringify(value, null, 2); + +async function run(args: string[], cwd: string) { + const out: string[] = []; + const err: string[] = []; + const io: Io = { cwd, color: false, stdout: (t) => out.push(t), stderr: (t) => err.push(t) }; + const code = await main(args, io); + return { code, stdout: out.join('\n'), stderr: err.join('\n') }; +} + +type Results = { diagnostics: { ruleId: string; line: number; message: string }[] }[]; + +/** What `check app/page.tsx` reports in `cwd`. */ +async function findings(cwd: string) { + const { stdout } = await run(['check', 'app/page.tsx', '--format', 'json', '--no-cache'], cwd); + return (JSON.parse(stdout) as Results).flatMap((r) => r.diagnostics); +} + +/** + * A Tailwind v4 library's theme partial: the app imports Tailwind, so the partial does not, and + * it sets four radii of the eight Tailwind has, and no spacing unit. + */ +const PARTIAL = `:root { + --primary: oklch(0.55 0.2 260); + --radius: 0.5rem; +} + +@theme inline { + --color-primary: var(--primary); + --radius-sm: calc(var(--radius) - 4px); + --radius-md: calc(var(--radius) - 2px); + --radius-lg: var(--radius); + --radius-xl: calc(var(--radius) + 4px); +} +`; + +const NORTHWIND = { + 'package.json': json({ + name: '@northwind/ui', + version: '1.4.0', + files: ['dist', 'src', 'theme.css', 'onsystem.config.json'], + exports: { '.': './src/index.ts', './theme.css': './theme.css' }, + peerDependencies: { tailwindcss: '^4.0.0' }, + onsystem: './onsystem.config.json', + }), + 'onsystem.config.json': json({ importPath: '@northwind/ui', tokens: ['theme.css'] }), + 'theme.css': PARTIAL, + 'src/index.ts': 'export * from "./button";\n', + 'src/button.tsx': + 'export function Button(props: { children?: unknown }) { return }\n', +}; + +const SPACED = `import { Button } from "@northwind/ui"; + +export default function Page() { + return ; +} + +export const Box = () =>
Box
; +`; +const OFF_SCALE = 'Hardcoded spacing `p-[13px]` (13px) is off the scale. Nearest: `p-3` (12px).'; + +/** A create-next-app app on Tailwind v4 that installs `@northwind/ui`. */ +function nextApp(devDependencies: Record = { tailwindcss: '^4' }) { + return fixture({ + 'package.json': json({ + name: 'web', + dependencies: { '@northwind/ui': '1.4.0', next: '16.0.0' }, + devDependencies, + }), + 'app/globals.css': '@import "tailwindcss";\n@import "@northwind/ui/theme.css";\n', + 'app/page.tsx': SPACED, + ...Object.fromEntries( + Object.entries(NORTHWIND).map(([file, text]) => [`node_modules/@northwind/ui/${file}`, text]), + ), + }); +} + +describe('a design-system package whose tokens are a Tailwind theme partial', () => { + it("keeps Tailwind's spacing unit and radius scale in an app on Tailwind v4", async () => { + const app = nextApp(); + const ds = await load(app); + expect(ds.config.designSystemPackage?.name).toBe('@northwind/ui'); + const spacing = ds.tokens.filter((t) => t.category === 'spacing'); + expect(spacing.map((t) => [t.cssVar, t.value])).toEqual([['--spacing', '0.25rem']]); + // Tailwind's eight radii, four of them as the partial sets them. + const radii = ds.tokens.filter((t) => t.category === 'radius'); + expect(radii.map((t) => [t.tailwind, t.origin === 'tailwind-default'])).toEqual([ + ['sm', false], + ['md', false], + ['lg', false], + ['xl', false], + ['xs', true], + ['2xl', true], + ['3xl', true], + ['4xl', true], + ]); + const all = await findings(app); + expect(all.filter((d) => d.ruleId === 'no-hardcoded-spacing').map((d) => d.message)).toEqual([ + OFF_SCALE, + ]); + }); + + it('does the same from a workspace package', async () => { + const root = fixture({ + 'pnpm-workspace.yaml': 'packages:\n - apps/*\n - packages/*\n', + 'package.json': json({ name: 'northwind', private: true }), + ...Object.fromEntries( + Object.entries(NORTHWIND).map(([file, text]) => [`packages/ui/${file}`, text]), + ), + 'apps/web/package.json': json({ + name: 'web', + dependencies: { '@northwind/ui': 'workspace:*' }, + devDependencies: { '@tailwindcss/postcss': '^4', tailwindcss: '^4' }, + }), + 'apps/web/app/page.tsx': SPACED, + }); + const all = await findings(path.join(root, 'apps/web')); + expect(all.filter((d) => d.ruleId === 'no-hardcoded-spacing').map((d) => d.message)).toEqual([ + OFF_SCALE, + ]); + }); + + it('adds nothing in an app that does not depend on Tailwind', async () => { + const ds = await load(nextApp({})); + expect(ds.tokens.filter((t) => t.category === 'spacing')).toEqual([]); + expect(ds.tokens.filter((t) => t.category === 'radius')).toHaveLength(4); + }); + + // Tailwind v3 has no `--spacing` unit and no `rounded-xs` or `rounded-4xl`, and its plugins + // run on either version. + it.each([['^3.4.0'], ['~3.4.17'], ['3.x']])( + 'adds nothing in an app on Tailwind v3 (%s)', + async (range) => { + const ds = await load(nextApp({ tailwindcss: range, '@tailwindcss/typography': '^0.5.16' })); + expect(ds.tokens.filter((t) => t.category === 'spacing')).toEqual([]); + expect(ds.tokens.filter((t) => t.category === 'radius')).toHaveLength(4); + }, + ); + + it('reads Tailwind v4 from the package that runs it, whatever the tailwindcss range', async () => { + const ds = await load(nextApp({ tailwindcss: '^3.4.0', '@tailwindcss/postcss': '^4' })); + expect(ds.tokens.filter((t) => t.category === 'spacing')).toHaveLength(1); + }); + + it("adds no spacing unit when the partial clears Tailwind's", async () => { + const app = nextApp(); + fs.writeFileSync( + path.join(app, 'node_modules/@northwind/ui/theme.css'), + PARTIAL.replace('@theme inline {', '@theme inline {\n --spacing-*: initial;'), + ); + const ds = await load(app); + expect(ds.tokens.filter((t) => t.category === 'spacing')).toEqual([]); + }); +}); + +/** + * A monorepo whose `@acme/ui` exports each component file by a subpath pattern, as shadcn/ui's + * monorepo template does, and its icons from `dist/`, not built yet. + */ +const ACME = { + 'pnpm-workspace.yaml': 'packages:\n - apps/*\n - packages/*\n', + 'package.json': json({ name: 'acme', private: true }), + 'packages/ui/package.json': json({ + name: '@acme/ui', + version: '0.0.0', + private: true, + exports: { + './components/icons': './dist/icons/index.js', + './components/*': './src/components/*.tsx', + './styles.css': './src/styles.css', + }, + }), + 'packages/ui/src/components/button.tsx': + 'export function Button(props: { variant?: "default" | "outline"; children?: unknown }) { return + ); +} +`; + +describe('an import through a subpath export with no file behind it', () => { + it('is an unknown component when the export maps it onto a source file', async () => { + const root = fixture({ ...ACME, 'apps/web/app/page.tsx': SUBPATHS }); + const ds = await load(path.join(root, 'apps/web')); + const result = ds.check(SUBPATHS, 'app/page.tsx'); + expect( + result.diagnostics + .filter((d) => d.ruleId === 'no-unknown-component') + .map((d) => [d.line, d.message]), + ).toEqual([ + [ + 8, + '"Tooltip" is not a design-system component (imported from "@acme/ui/components/tooltip"). Use search_components to find an existing one.', + ], + ]); + // The icons' own export, which comes first, leads to `dist/`, not built yet: StarIcon may + // be real, so it is said to be unchecked. + expect(result.unchecked).toEqual({ + names: ['StarIcon'], + modules: ['@acme/ui/components/icons'], + }); + }); + + it('finds the package past a package.json with no name in its folders', async () => { + const root = fixture({ + ...ACME, + 'packages/ui/src/components/package.json': json({ type: 'module' }), + 'apps/web/app/page.tsx': SUBPATHS, + }); + const ds = await load(path.join(root, 'apps/web')); + const result = ds.check(SUBPATHS, 'app/page.tsx'); + expect( + result.diagnostics.filter((d) => d.ruleId === 'no-unknown-component').map((d) => d.line), + ).toEqual([8]); + }); + + // `init`'s config in a package whose "./*" leads to `dist/components`, which `tsc` builds from + // `src/components`, with Tooltip in a `.ts` file its components glob leaves out. + const DIST_STAR = { + 'pnpm-workspace.yaml': 'packages:\n - apps/*\n - packages/*\n', + 'package.json': json({ name: 'acme', private: true }), + 'packages/ui/package.json': json({ + name: '@acme/ui', + version: '0.0.0', + private: true, + type: 'module', + exports: { + './styles.css': './src/styles.css', + './*': { types: './dist/components/*.d.ts', default: './dist/components/*.js' }, + }, + onsystem: './onsystem.config.json', + }), + 'packages/ui/onsystem.config.json': json({ components: ['src/components/**/*.{tsx,jsx}'] }), + 'packages/ui/src/components/button.tsx': + 'export function Button(props: { variant?: "default" | "outline"; children?: unknown }) { return + ); +} +`; + + it.each([ + ['not built', {}, false], + ['not built, linked', {}, true], + ['built', BUILT, false], + ['built, linked', BUILT, true], + ])( + 'is only unchecked when the export leads to dist/ (%s)', + async (_, built: Record, linked) => { + const root = fixture({ ...DIST_STAR, ...built, 'apps/web/app/page.tsx': DIST_PAGE }); + if (linked) link(root); + const ds = await load(path.join(root, 'apps/web')); + expect(ds.config.designSystemPackage?.name).toBe('@acme/ui'); + const result = ds.check(DIST_PAGE, 'app/page.tsx'); + expect(result.diagnostics.filter((d) => d.ruleId === 'no-unknown-component')).toEqual([]); + // Tooltip is real, in src/components/tooltip.ts. Popover may be too, as far as the + // package.json says: its source is any file the build reads. + expect(result.unchecked?.names.sort()).toEqual(['Popover', 'Tooltip']); + }, + ); + + // Of two patterns with the same prefix, Node.js takes the longer key: "./components/*.js" + // over "./components/*" for "./components/tip.js", whichever comes first. Tip's source is + // src/components/tip.ts, built to dist/components/tip.js. + const TIP = 'export { Root as Tip } from "@radix-ui/react-tooltip";\n'; + const ORDERED = (exports: Record) => ({ + ...DIST_STAR, + 'packages/ui/package.json': json({ + name: '@acme/ui', + version: '0.0.0', + private: true, + type: 'module', + exports, + onsystem: './onsystem.config.json', + }), + 'packages/ui/src/components/tip.ts': TIP, + 'packages/ui/dist/components/tip.js': TIP, + 'packages/ui/dist/components/tip.d.ts': TIP, + }); + const BUILT_JS = { types: './dist/components/*.d.ts', default: './dist/components/*.js' }; + const SOURCE = './src/components/*.tsx'; + + // Each order of the two keys, under "./components/" and at the package's root, each linked + // into node_modules and not. + it.each( + [ + ['./components/*', { './components/*': SOURCE, './components/*.js': BUILT_JS }], + ['./components/*.js', { './components/*.js': BUILT_JS, './components/*': SOURCE }], + ['./*', { './*': SOURCE, './*.js': BUILT_JS }], + ['./*.js', { './*.js': BUILT_JS, './*': SOURCE }], + ].flatMap(([first, exports]) => [ + [first, false, exports], + [first, true, exports], + ]) as [string, boolean, Record][], + )( + 'takes the longer of two patterns with the same prefix (%s first, linked: %s)', + async (first, linked, exports) => { + const at = first.startsWith('./components/') ? 'components/' : ''; + const page = `import { Button } from "@acme/ui/${at}button"; +import { Tip } from "@acme/ui/${at}tip.js"; + +export default function Page() { + return ; +} +`; + const root = fixture({ ...ORDERED(exports), 'apps/web/app/page.tsx': page }); + if (linked) { + link(root); + // What Node.js resolves the import to. + const resolved = createRequire(path.join(root, 'apps/web/app/page.tsx')).resolve( + `@acme/ui/${at}tip.js`, + ); + expect(resolved).toBe( + fs.realpathSync(path.join(root, 'packages/ui/dist/components/tip.js')), + ); + } + const ds = await load(path.join(root, 'apps/web')); + const result = ds.check(page, 'app/page.tsx'); + expect(result.diagnostics.filter((d) => d.ruleId === 'no-unknown-component')).toEqual([]); + expect(result.unchecked?.names).toEqual(['Tip']); + }, + ); + + it('looks for the .tsx file TypeScript resolves a .jsx export to', async () => { + const page = `import { Button } from "@acme/ui/button"; +import { Tip } from "@acme/ui/tip"; +import { Popover } from "@acme/ui/popover"; + +export default function Page() { + return ; +} +`; + const root = fixture({ + ...Object.fromEntries( + Object.entries(DIST_STAR).filter(([file]) => !file.endsWith('button.tsx')), + ), + 'packages/ui/package.json': json({ + name: '@acme/ui', + version: '0.0.0', + private: true, + type: 'module', + exports: { './*': './src/components/*.jsx' }, + onsystem: './onsystem.config.json', + }), + // A package in JSX, but for one component in TypeScript. + 'packages/ui/onsystem.config.json': json({ components: ['src/components/*.jsx'] }), + 'packages/ui/src/components/button.jsx': + 'export function Button({ variant, children }) { return }\n', + 'packages/ui/src/components/tip.tsx': + 'export function Tip(props: { children?: unknown }) { return }\n', + 'apps/web/app/page.tsx': page, + }); + const ds = await load(path.join(root, 'apps/web')); + const result = ds.check(page, 'app/page.tsx'); + // Tip is in src/components/tip.tsx, which the components glob leaves out; Popover is nowhere. + expect( + result.diagnostics.filter((d) => d.ruleId === 'no-unknown-component').map((d) => d.message), + ).toEqual([ + '"Popover" is not a design-system component (imported from "@acme/ui/popover"). Use search_components to find an existing one.', + ]); + expect(result.unchecked?.names).toEqual(['Tip']); + }); + + it('leaves an export into a build folder unchecked, whatever its extension', async () => { + const page = `import { Button } from "@acme/ui/components/button"; +import { Popover } from "@acme/ui/components/popover"; + +export default function Page() { + return ; +} +`; + const root = fixture({ + ...DIST_STAR, + 'packages/ui/package.json': json({ + name: '@acme/ui', + version: '0.0.0', + private: true, + type: 'module', + exports: { './components/*': './dist/components/*.tsx' }, + onsystem: './onsystem.config.json', + }), + 'apps/web/app/page.tsx': page, + }); + const ds = await load(path.join(root, 'apps/web')); + const result = ds.check(page, 'app/page.tsx'); + expect(result.diagnostics.filter((d) => d.ruleId === 'no-unknown-component')).toEqual([]); + expect(result.unchecked?.names).toEqual(['Popover']); + }); +}); + +/** Links the workspace's `@acme/ui` into `apps/web/node_modules`, as pnpm does. */ +function link(root: string) { + fs.mkdirSync(path.join(root, 'apps/web/node_modules/@acme'), { recursive: true }); + fs.symlinkSync( + path.join(root, 'packages/ui'), + path.join(root, 'apps/web/node_modules/@acme/ui'), + 'junction', + ); +} + +function text(result: unknown): string { + const content = (result as CallToolResult).content[0]; + return content?.type === 'text' ? content.text : ''; +} + +const clients: Client[] = []; +afterAll(async () => { + for (const client of clients.splice(0)) await client.close(); +}); + +/** `@northwind/ui` published with `dist/`, its tokens and its config, but no sources or declarations. */ +const DIST_ONLY = { + 'package.json': json({ + name: '@northwind/ui', + version: '1.4.0', + files: ['dist', 'theme.css', 'onsystem.config.json'], + exports: { '.': { import: './dist/index.js' }, './theme.css': './theme.css' }, + onsystem: './onsystem.config.json', + }), + 'onsystem.config.json': json({ + components: ['src/**/*.tsx'], + importPath: '@northwind/ui', + tokens: ['theme.css'], + }), + 'theme.css': PARTIAL, + 'dist/index.js': 'export function Button() {}\n', +}; + +describe('an app whose design-system package publishes no sources', () => { + const app = () => + fixture({ + 'package.json': json({ name: 'web', dependencies: { '@northwind/ui': '1.4.0' } }), + 'app/page.tsx': SPACED, + ...Object.fromEntries( + Object.entries(DIST_ONLY).map(([file, text]) => [ + `node_modules/@northwind/ui/${file}`, + text, + ]), + ), + }); + const REASON = + '@northwind/ui publishes neither its component sources nor their type declarations, so its components are unknown here and the component rules did not run: ask its team to add its sources to "files" in its package.json, or to publish type declarations. The token rules still check against the tokens it publishes. See https://onsystem.vercel.app/docs/design-system-packages#what-apps-get'; + + it('says why in list_components, not to check the components globs', async () => { + const ds = await load(app()); + const server = createServer({ getDesignSystem: () => Promise.resolve(ds) }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + clients.push(client); + const listed = text(await client.callTool({ name: 'list_components', arguments: {} })); + expect(listed).toBe(REASON); + expect(listed).not.toContain('globs'); + expect( + text(await client.callTool({ name: 'get_component', arguments: { name: 'Button' } })), + ).toBe(REASON); + }); + + it('says that "libraries": false turned off the declarations the package publishes', async () => { + const root = app(); + const nm = path.join(root, 'node_modules/@northwind/ui'); + const manifest = JSON.parse(fs.readFileSync(path.join(nm, 'package.json'), 'utf8')) as Record< + string, + unknown + >; + fs.writeFileSync( + path.join(nm, 'package.json'), + json({ ...manifest, types: './dist/index.d.ts' }), + ); + fs.writeFileSync( + path.join(nm, 'dist/index.d.ts'), + 'import type * as React from "react";\nexport declare function Button(props: { children?: React.ReactNode }): React.JSX.Element;\n', + ); + // With the declarations read, there is no reason to give. + const read = await load(root); + expect(read.model.components.map((c) => c.name)).toEqual(['Button']); + fs.writeFileSync(path.join(root, 'onsystem.config.json'), json({ libraries: false })); + const ds = await load(root); + const server = createServer({ getDesignSystem: () => Promise.resolve(ds) }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + clients.push(client); + const off = + '@northwind/ui publishes no component sources, and "libraries": false in the config turns off the type declarations it publishes, so its components are unknown here and the component rules did not run: set "libraries" to [], which reads its declarations and no library, or remove it. The token rules still check against the tokens it publishes. See https://onsystem.vercel.app/docs/design-system-packages#what-apps-get'; + expect(text(await client.callTool({ name: 'list_components', arguments: {} }))).toBe(off); + expect( + text(await client.callTool({ name: 'get_component', arguments: { name: 'Button' } })), + ).toBe(off); + // As it says, [] reads them. + fs.writeFileSync(path.join(root, 'onsystem.config.json'), json({ libraries: [] })); + expect((await load(root)).model.components.map((c) => c.name)).toEqual(['Button']); + }); + + it('lists no "Available" names when there are none', () => { + expect(String(renderNotFound('Button', undefined, []))).toBe('No component named "Button".'); + }); + + it('says so in inspect --explain', async () => { + const { stdout } = await run(['inspect', '--explain', '--no-cache'], app()); + expect(stdout).toContain(` ${REASON}`); + }); +}); + +describe('init in a published design-system package', () => { + it('publishes ONSYSTEM.md with the config, for the teams of apps that install it', async () => { + const root = fixture({ + 'package.json': json({ + name: '@northwind/ui', + version: '1.4.0', + files: ['dist', 'src', 'theme.css'], + exports: { '.': './src/index.ts', './theme.css': './theme.css' }, + }), + 'theme.css': PARTIAL, + 'src/index.ts': + 'export * from "./button";\nexport * from "./card";\nexport * from "./input";\n', + 'src/button.tsx': + 'export function Button(props: { children?: unknown }) { return