fix(runner): resolve every @source path and stop scanning test files

.changeset/8454-runner-stylesheet-source-set.md

@@ -0,0 +1,38 @@
+---
+'@object-ui/runner': patch
+---
+
+Fix every `@source` path in the runner's Tailwind entry, and keep test files out
+of its published stylesheet (objectui#8454).
+
+`packages/runner/src/index.css` is a published input: this package is
+`private: false` with `files: ["dist"]`, so what it compiles to is bytes a
+consumer downloads and serves. All five of its `@source` lines were wrong by one
+path segment. Tailwind resolves a relative `@source` against the directory of the
+entry CSS — `packages/runner/src/` — so `./src/**` meant
+`packages/runner/src/src` and `../../packages/<pkg>/src` meant
+`packages/packages/<pkg>/src`. A glob whose base directory does not exist scans
+nothing and raises no error, so the file read as if it declared its inputs while
+declaring none: deleting all five lines produced a byte-identical artifact.
+
+What kept the sheet non-empty was Tailwind's automatic source detection, whose
+base defaults to the process CWD — `packages/runner`, where `pnpm build` runs.
+That covers this package's own tree and nothing else, so every utility belonging
+to `@object-ui/components`, `@object-ui/react`, `@object-ui/plugin-kanban` and
+`@object-ui/plugin-charts` was missing from the shipped app. Measured from the
+package directory, repairing the four sibling paths takes the compiled sheet from
+228 to 1456 selectors (21 kB to 136 kB): `bg-popover`, `bg-accent`,
+`bg-destructive`, `animate-out` and 1224 more had no source anywhere.
+
+The same automatic root also swept this package's own test files into the
+published bytes. Two `@source not` lines — the spelling
+`packages/plugin-kanban/src/index.css` already uses, anchored one level up
+because this entry scans four sibling packages — remove nine test-sourced
+classes, several of them ordinary English words lifted out of prose comments
+(`paused`, `invert`, `flex-nowrap`).
+
+`patch`: this package exposes no importable surface at all (no `main`, `module`,
+`types` or `exports` — it publishes a built application under `dist`), so nothing
+a consumer imports changes shape. The nine removed classes are unreachable from
+the app's own markup by construction, which is why the scan never had a shipped
+source for them.

packages/runner/src/__tests__/published-stylesheet-sources.test.ts

@@ -0,0 +1,180 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * Pins the SOURCE SET of `@object-ui/runner`'s stylesheet (objectui#8454).
+ *
+ * This package is `private: false` with `files: ["dist"]`, so `src/index.css`
+ * compiles into bytes a consumer installs. Two things about it went wrong at
+ * once, and neither one produced any error at build time:
+ *
+ *  1. every `@source` path named a directory that does not exist. Tailwind
+ *     resolves a relative `@source` against `dirname(<entry css>)` — here
+ *     `packages/runner/src/` — so `./src/**` meant `packages/runner/src/src`
+ *     and `../../packages/<pkg>/src` meant `packages/packages/<pkg>/src`. A
+ *     glob whose base is missing scans nothing and is not an error: deleting
+ *     all five lines was measured byte-identical to keeping them.
+ *  2. nothing excluded test files. What kept the sheet non-empty was Tailwind's
+ *     automatic source detection, which roots at the PROCESS CWD (`base`
+ *     defaults to `process.cwd()`), i.e. `packages/runner` when `pnpm build`
+ *     runs — so runner's own tests were scanned and shipped, down to ordinary
+ *     English words picked out of prose comments.
+ *
+ * ## Why a naive assertion passes while broken
+ *
+ * "The published sheet contains no test-sourced class" is satisfied perfectly
+ * by a sheet compiled from NOTHING — which is exactly the state defect (1) put
+ * this package in for the four sibling trees. So the negative assertion below
+ * is paired with a positive one that a sheet compiled from nothing fails: a
+ * themed utility that ONLY `@object-ui/components`' shipped source can supply.
+ * And the negative is not a bare absence either — it is a DIFFERENCE against a
+ * second compile of the same entry with the `@source not` lines stripped, so an
+ * exclusion that silently stopped matching anything fails here rather than
+ * passing quietly.
+ *
+ * ## Why it compiles instead of reading `dist/`
+ *
+ * CI runs the suite on an unbuilt worktree, so `dist/` is legitimately absent
+ * and a test that read the artifact would pass vacuously — the same reasoning
+ * `scripts/__tests__/plugin-published-stylesheet.test.ts` records. It runs the
+ * real `@tailwindcss/postcss` over the real entry instead, with `base` pinned
+ * to the package root so the automatic-detection root is the one `pnpm build`
+ * uses no matter which directory the suite was launched from.
+ */
+import { describe, expect, it } from 'vitest';
+import { existsSync, readFileSync } from 'node:fs';
+import { dirname, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import postcss from 'postcss';
+import tailwindPostcss from '@tailwindcss/postcss';
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+/** `packages/runner` — what `process.cwd()` is when this package builds. */
+const PACKAGE_ROOT = resolve(HERE, '../..');
+const ENTRY = resolve(PACKAGE_ROOT, 'src/index.css');
+const ENTRY_DIR = dirname(ENTRY);
+
+const entryCss = readFileSync(ENTRY, 'utf8');
+
+/**
+ * Class names in a compiled selector, undoing Tailwind's CSS escapes.
+ * Mirrors `classesIn` in `scripts/build-plugin-stylesheet.mjs`; not imported
+ * from there because the root tsconfig sets `allowJs: false`, so a `.ts` test
+ * under `packages/` cannot pull in a plain `.mjs` helper.
+ */
+function classesIn(selector: string): string[] {
+  const found: string[] = [];
+  const re = /\.((?:\\.|[^\s.,>+~()[\]:#*'"\\])+)/g;
+  let m: RegExpExecArray | null;
+  while ((m = re.exec(selector))) {
+    found.push(
+      m[1]
+        .replace(/\\([0-9a-fA-F]{1,6})\s?/g, (_, hex: string) =>
+          String.fromCodePoint(parseInt(hex, 16)),
+        )
+        .replace(/\\(.)/g, '$1'),
+    );
+  }
+  return found;
+}
+
+/**
+ * Compile the entry the way this package's build does. `base` is the
+ * automatic-detection root and defaults to `process.cwd()` — pinning it to the
+ * package root is what makes this reading independent of where vitest started
+ * (from the repo root the same bytes scan the whole monorepo instead).
+ */
+async function compileClasses(css: string): Promise<Set<string>> {
+  const result = await postcss([tailwindPostcss({ base: PACKAGE_ROOT })]).process(css, {
+    from: ENTRY,
+  });
+  const classes = new Set<string>();
+  postcss.parse(result.css, { from: ENTRY }).walkRules((rule) => {
+    for (const selector of rule.selectors) for (const cls of classesIn(selector)) classes.add(cls);
+  });
+  return classes;
+}
+
+/** `@source` / `@source not` directives, in file order, params unquoted. */
+function sourceDirectives(css: string): { negated: boolean; pattern: string }[] {
+  const out: { negated: boolean; pattern: string }[] = [];
+  for (const line of css.split('\n')) {
+    const m = /^@source\s+(not\s+)?['"](.+)['"]\s*;/.exec(line.trim());
+    if (m) out.push({ negated: Boolean(m[1]), pattern: m[2] });
+  }
+  return out;
+}
+
+/** The literal directory prefix of a glob — everything before the first `*`. */
+function globBase(pattern: string): string {
+  const star = pattern.indexOf('*');
+  const literal = star === -1 ? pattern : pattern.slice(0, star);
+  return resolve(ENTRY_DIR, literal.replace(/\/[^/]*$/, ''));
+}
+
+// Cost paid at import time on purpose: module evaluation is bound by no test or
+// hook timeout, and each compile is ~0.6s. AGENTS.md's testing-discipline
+// section is explicit that a `beforeAll` would be the WORSE place for it:
+// `hookTimeout` (10s) is narrower than `testTimeout` (15s).
+const shipped = await compileClasses(entryCss);
+const withTestSources = await compileClasses(
+  entryCss
+    .split('\n')
+    .filter((line) => !line.trimStart().startsWith('@source not '))
+    .join('\n'),
+);
+
+describe('@object-ui/runner published stylesheet — source set', () => {
+  it('resolves every @source path to a directory that exists', () => {
+    const directives = sourceDirectives(entryCss);
+    // Non-vacuity: the check above is trivially satisfied by a file with no
+    // `@source` at all, which is the shape this card had to rule out.
+    expect(directives.filter((d) => !d.negated).length).toBeGreaterThanOrEqual(5);
+
+    const missing = directives
+      .map((d) => ({ ...d, base: globBase(d.pattern) }))
+      .filter((d) => !existsSync(d.base));
+    expect(missing).toEqual([]);
+  });
+
+  it('the historical spellings really did point at nothing (control)', () => {
+    // Without this, the assertion above could be green because every path
+    // resolves to the package dir by accident rather than by being correct.
+    expect(existsSync(resolve(ENTRY_DIR, 'src'))).toBe(false);
+    expect(existsSync(resolve(ENTRY_DIR, '../../packages/components/src'))).toBe(false);
+  });
+
+  it('compiles utilities that only a dependency can supply', () => {
+    // `bg-popover` is used by ten shipped files under `packages/components/src`
+    // (popover, dropdown-menu, tooltip, select, …) and by nothing in this
+    // package's own `src`. It can therefore only come from the
+    // `../../components/src/**` line, and it is absent the moment that line
+    // stops resolving — which is what shipped before objectui#8454.
+    expect(shipped).toContain('bg-popover');
+    expect(readFileSync(resolve(PACKAGE_ROOT, 'src/LayoutRenderer.tsx'), 'utf8')).not.toContain(
+      'bg-popover',
+    );
+    // Lit control for the two assertions above: a class this package's OWN
+    // source supplies, so a compile that produced nothing cannot pass either.
+    expect(shipped).toContain('flex-col');
+  });
+
+  it('keeps test-sourced classes out of the published sheet', () => {
+    // `paused` reaches the candidate list only as an English word inside a
+    // prose comment in `packages/components/src/__tests__/`. It is a real
+    // Tailwind utility (`animation-play-state: paused`), so without the
+    // exclusions it compiles into bytes every consumer downloads.
+    expect(withTestSources).toContain('paused');
+    expect(shipped).not.toContain('paused');
+
+    // And the exclusions must still be subtracting something at all: an
+    // `@source not` that quietly stopped matching would otherwise leave every
+    // assertion in this file green.
+    const removed = [...withTestSources].filter((cls) => !shipped.has(cls));
+    expect(removed.length).toBeGreaterThan(0);
+    expect([...shipped].filter((cls) => !withTestSources.has(cls))).toEqual([]);
+  });
+});

packages/runner/src/index.css

@@ -6,12 +6,57 @@
    shared styles entry. Keep in sync with packages/app-shell/src/styles.css. */
 @custom-variant dark (&:where(.dark, .dark *));
 
-/* Scan sources for Tailwind classes */
-@source './src/**/*.{ts,tsx}';
-@source '../../packages/components/src/**/*.{ts,tsx}';
-@source '../../packages/react/src/**/*.{ts,tsx}';
-@source '../../packages/plugin-kanban/src/**/*.{ts,tsx}';
-@source '../../packages/plugin-charts/src/**/*.{ts,tsx}';
+/*
+ * Scan sources for Tailwind classes.
+ *
+ * ⚠️ Every path below is resolved relative to THIS FILE's directory
+ * (`packages/runner/src/`) — not the package root, not the CWD.
+ * `@tailwindcss/postcss` compiles with `base = dirname(<entry css>)`, and the
+ * `@source` params are rewritten against that same base before the scanner ever
+ * sees them. All five lines used to get it wrong by exactly one `src` segment
+ * (`./src/**` -> `packages/runner/src/src`, `../../packages/<pkg>/src` ->
+ * `packages/packages/<pkg>/src`), so every one of them named a directory that
+ * does not exist and contributed nothing: measured on objectui#8454, deleting
+ * all five produced a byte-identical artifact.
+ *
+ * What kept this sheet non-empty was Tailwind's automatic source detection,
+ * which roots at the PROCESS CWD — `packages/runner`, where `pnpm build` runs.
+ * That covers runner's own tree and nothing else, so the utilities its
+ * dependencies need were simply missing from the published bundle: fixing the
+ * four sibling paths takes it from 228 to 1456 compiled selectors (21 kB ->
+ * 136 kB). The automatic root is still on (there is no `source(none)` here, and
+ * that parity question is objectui#8455), which is why the first line below is
+ * a no-op today and still belongs here: it is the declared intent, and it is
+ * what keeps runner's own source scanned the day the root moves.
+ */
+@source './**/*.{ts,tsx}';
+@source '../../components/src/**/*.{ts,tsx}';
+@source '../../react/src/**/*.{ts,tsx}';
+@source '../../plugin-kanban/src/**/*.{ts,tsx}';
+@source '../../plugin-charts/src/**/*.{ts,tsx}';
+
+/*
+ * Only shipped source. A `*.test.tsx` never reaches a consumer, and neither
+ * does a helper that lives beside one, so a utility used solely by tests must
+ * not become a published byte. Same two-line spelling as
+ * `packages/plugin-kanban/src/index.css`, anchored one level up because this
+ * entry scans four sibling packages instead of only its own tree — `../../`
+ * bases these on `packages/`, which contains every tree named above AND the
+ * automatic-detection root, so runner's own tests are covered by the same two
+ * lines (measured: negations do apply to files the automatic root found).
+ *
+ * Both lines earn their place. The file line removes nine test-sourced classes
+ * (`flex-grow`, `flex-nowrap`, `h-[125px]`, `h-[400px]`, `invert`, `paused`,
+ * `slide-in-from-bottom`, `text-green-500`, `w-[250px]` — several of them
+ * ordinary English words picked out of prose comments). The directory line
+ * removes nothing extra today, but it is the only thing covering the
+ * non-`*.test.*` sources under `packages/components/src/__tests__/`
+ * (`test-utils.tsx`, `page-header-action-ids.dist.spec.tsx`); ablated with a
+ * marker class injected into the first of those, the file line alone ships it
+ * and the pair does not.
+ */
+@source not '../../**/*.test.{ts,tsx}';
+@source not '../../**/__tests__/**';
 
 /* Tailwind plugin for animations */
 @plugin 'tailwindcss-animate';

packages/runner/tsconfig.test.json

@@ -17,7 +17,14 @@
     // pulls `lib/MetadataLoader.ts` into this program, and that file calls
     // `import.meta.glob` — declared by `vite/client`, which the root config
     // this extends does not carry (it is not a Vite app).
-    "types": ["vite/client"]
+    //
+    // `node` joins it for `src/__tests__/published-stylesheet-sources.test.ts`,
+    // which reads and compiles this package's Tailwind entry through
+    // `node:fs` / `node:path` / `node:url`. Naming `types` at all switches off
+    // automatic `@types/*` inclusion, so the entry is required rather than
+    // implied; `@types/node` resolves from the workspace root, which is how
+    // `packages/components/tsconfig.test.json` gets it too.
+    "types": ["node", "vite/client"]
   },
   "include": ["src/**/*.test.ts", "src/**/*.test.tsx"]
 }