From 0c5bacb570c5fa2c32e5e51a79111fd5cf1f54df Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Mon, 26 Oct 2026 17:51:22 +0530 Subject: [PATCH 01/26] feat(frontend): v1 scaffold, strict TypeScript and import boundary (UN-4270) Adds frontend/src/v1/ as an isolated tree for the v1 frontend (P0-T01): - Folder taxonomy from the spec, a public entry (src/v1/index.ts, imported by v0 as `@v1`) and a cloud overlay placeholder. - src/v1/tsconfig.json: strict, no JS, unused locals/params are errors, `@v1` alias only. v0's tsconfig/jsconfig exclude src/v1. - Biome overrides for the two-way import boundary and v1 bans (no `../`, `@/`, prop-types, antd, component CSS; noExplicitAny). - scripts/check-v1.mjs for what Biome cannot express: TypeScript-only files, the folder taxonomy and import contract, PropTypes, `ant-`. - typecheck:v1, lint:v1 and test:v1 scripts; v0 scripts are unchanged. - test-fixtures/v1-boundary/ proves each rule fails and passes as specified, run with node --test so the v0 vitest suite never sees it. - .gitignore re-includes src/v1/lib/ (the Python `lib/` rule matched it). v0 build output, test counts, typecheck and Biome results are unchanged. --- .gitignore | 6 +- .pre-commit-config.yaml | 4 +- frontend/biome.json | 59 +++- frontend/jsconfig.json | 5 +- frontend/package.json | 5 +- frontend/scripts/check-v1.mjs | 253 ++++++++++++++++++ frontend/src/v1/README.md | 49 ++++ frontend/src/v1/app/.gitkeep | 0 frontend/src/v1/components/patterns/.gitkeep | 0 frontend/src/v1/components/ui/.gitkeep | 0 frontend/src/v1/hooks/.gitkeep | 0 frontend/src/v1/index.ts | 7 + frontend/src/v1/lib/.gitkeep | 0 frontend/src/v1/pages/.gitkeep | 0 frontend/src/v1/plugins/manifest.ts | 6 + frontend/src/v1/services/.gitkeep | 0 frontend/src/v1/shell/.gitkeep | 0 frontend/src/v1/stores/.gitkeep | 0 frontend/src/v1/styles/.gitkeep | 0 frontend/src/v1/test/.gitkeep | 0 frontend/src/v1/tsconfig.json | 39 +++ .../v1-boundary/boundary.fixtures.mjs | 220 +++++++++++++++ .../v0-imports-v1-entry/src/pages/Ok.jsx | 1 + .../v0-imports-v1-internal/src/pages/Bad.jsx | 3 + .../v0-imports-v1-relative/src/pages/Bad.jsx | 3 + .../src/v1/components/ui/Label.test.ts | 4 + .../src/v1/components/ui/Label.ts | 3 + .../v1-allowed-imports/src/v1/lib/format.ts | 1 + .../src/v1/pages/projects/title.ts | 1 + .../src/v1/pages/projects/view/heading.ts | 4 + .../v1-allowed-imports/src/v1/test/render.ts | 1 + .../src/v1/components/ui/Button.ts | 1 + .../src/v1/lib/bad.ts | 1 + .../src/v1/components/ui/Badge.css | 3 + .../src/v1/components/ui/Badge.tsx | 3 + .../cases/v1-explicit-any/src/v1/lib/bad.ts | 1 + .../cases/v1-implicit-any/src/v1/lib/bad.ts | 3 + .../v1-imports-v0-alias/src/v1/lib/bad.ts | 3 + .../v1-imports-v0-relative/src/v1/lib/bad.ts | 3 + .../cases/v1-js-file/src/v1/lib/legacy.js | 1 + .../src/v1/widgets/thing.ts | 1 + .../src/v1/pages/projects/index.ts | 3 + .../src/v1/pages/settings/index.ts | 1 + .../src/v1/components/ui/Box.ts | 4 + .../src/v1/components/ui/Badge.tsx | 3 + .../src/v1/components/ui/ProjectLink.tsx | 3 + .../src/v1/services/projects.ts | 1 + .../cases/v1-unused-local/src/v1/lib/bad.ts | 4 + frontend/tsconfig.json | 7 +- frontend/vite.config.js | 2 + frontend/vitest.config.mjs | 14 +- 51 files changed, 725 insertions(+), 11 deletions(-) create mode 100755 frontend/scripts/check-v1.mjs create mode 100644 frontend/src/v1/README.md create mode 100644 frontend/src/v1/app/.gitkeep create mode 100644 frontend/src/v1/components/patterns/.gitkeep create mode 100644 frontend/src/v1/components/ui/.gitkeep create mode 100644 frontend/src/v1/hooks/.gitkeep create mode 100644 frontend/src/v1/index.ts create mode 100644 frontend/src/v1/lib/.gitkeep create mode 100644 frontend/src/v1/pages/.gitkeep create mode 100644 frontend/src/v1/plugins/manifest.ts create mode 100644 frontend/src/v1/services/.gitkeep create mode 100644 frontend/src/v1/shell/.gitkeep create mode 100644 frontend/src/v1/stores/.gitkeep create mode 100644 frontend/src/v1/styles/.gitkeep create mode 100644 frontend/src/v1/test/.gitkeep create mode 100644 frontend/src/v1/tsconfig.json create mode 100644 frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-entry/src/pages/Ok.jsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/lib/format.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/view/heading.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/test/render.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-ant-class-prefix/src/v1/components/ui/Button.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-base-rules-still-apply/src/v1/lib/bad.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-component-css-file/src/v1/components/ui/Badge.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-component-css-import/src/v1/components/ui/Badge.tsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-explicit-any/src/v1/lib/bad.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-implicit-any/src/v1/lib/bad.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-js-file/src/v1/lib/legacy.js create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-outside-taxonomy/src/v1/widgets/thing.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/settings/index.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-prop-types-assignment/src/v1/components/ui/Box.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-prop-types/src/v1/components/ui/Badge.tsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/services/projects.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-unused-local/src/v1/lib/bad.ts diff --git a/.gitignore b/.gitignore index cfd13e388f..c7a5148e30 100644 --- a/.gitignore +++ b/.gitignore @@ -41,8 +41,12 @@ eggs/ lib/ lib64/ # The Python `lib/` rule above also matches the frontend's shadcn/ui helper -# directory (`@/lib/utils` per components.json). Keep that tracked. +# directory (`@/lib/utils` per components.json), the v1 `lib/` folder and the +# v1 guardrail fixtures. Keep those tracked, or their files are silently left +# out of commits. !frontend/src/lib/ +!frontend/src/v1/lib/ +!frontend/test-fixtures/**/lib/ parts/ sdist/ var/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e279326d6f..fe06ede0a0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,9 +22,9 @@ repos: - id: check-docstring-first - id: check-ast - id: check-json - # Both files are JSONC — their tooling (VS Code, tsc) allows comments, + # These files are JSONC — their tooling (VS Code, tsc) allows comments, # but this hook parses with strict `json.loads`, which does not. - exclude: "(\\.vscode/launch\\.json|frontend/tsconfig\\.json)$" + exclude: "(\\.vscode/launch\\.json|frontend/(src/v1/)?tsconfig\\.json)$" - id: check-executables-have-shebangs - id: check-shebang-scripts-are-executable - id: check-toml diff --git a/frontend/biome.json b/frontend/biome.json index eb98923c0c..c8ead404ef 100644 --- a/frontend/biome.json +++ b/frontend/biome.json @@ -129,5 +129,62 @@ "assist": { "enabled": true, "actions": { "source": { "organizeImports": "on" } } - } + }, + "overrides": [ + { + "includes": ["src/v1/**"], + "linter": { + "rules": { + "style": { + "noRestrictedImports": { + "level": "error", + "options": { + "paths": { + "prop-types": "v1 is typed with TypeScript; PropTypes are not used under src/v1.", + "antd": "antd is not used under src/v1." + }, + "patterns": [ + { + "group": ["../**", "@/**", "src/**"], + "message": "v1 imports nothing outside src/v1, and never with '../'. Use './' within a folder and '@v1/...' across folders." + }, + { + "group": ["@ant-design/**"], + "message": "antd is not used under src/v1." + }, + { + "group": ["**/*.css", "!@v1/styles/**"], + "message": "No component .css under src/v1; style with tokens and Tailwind. Global v1 styles live in @v1/styles/." + } + ] + } + } + }, + "suspicious": { + "noExplicitAny": "error" + } + } + } + }, + { + "includes": ["src/**", "!src/v1/**"], + "linter": { + "rules": { + "style": { + "noRestrictedImports": { + "level": "error", + "options": { + "patterns": [ + { + "group": ["@v1/**", "**/v1", "**/v1/**", "!@v1"], + "message": "v0 may import only the v1 public entry, as '@v1'." + } + ] + } + } + } + } + } + } + ] } diff --git a/frontend/jsconfig.json b/frontend/jsconfig.json index 082b33982c..63ea90da3d 100644 --- a/frontend/jsconfig.json +++ b/frontend/jsconfig.json @@ -2,9 +2,10 @@ "compilerOptions": { "baseUrl": ".", "paths": { - "@/*": ["./src/*"] + "@/*": ["./src/*"], + "@v1": ["./src/v1/index.ts"] } }, "include": ["src/**/*"], - "exclude": ["node_modules", "build"] + "exclude": ["node_modules", "build", "src/v1"] } diff --git a/frontend/package.json b/frontend/package.json index b4ab1e1bd5..3d7f579cc9 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -77,7 +77,10 @@ "check:fix": "biome check --write src/", "lint:all": "biome check --write src/", "lint:changed": "(git diff --name-only --diff-filter=ACMR; git ls-files --others --exclude-standard) | grep -E '\\.[jt]sx?$' | sed 's|^frontend/||' | sort -u | xargs -r biome check --write", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "typecheck:v1": "node scripts/check-v1.mjs --files && tsc -p src/v1/tsconfig.json", + "lint:v1": "biome lint src/v1/ && node scripts/check-v1.mjs --code", + "test:v1": "vitest run --dir src/v1 --passWithNoTests && node --test test-fixtures/v1-boundary/boundary.fixtures.mjs" }, "browserslist": { "production": [ diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs new file mode 100755 index 0000000000..6a27cb1783 --- /dev/null +++ b/frontend/scripts/check-v1.mjs @@ -0,0 +1,253 @@ +#!/usr/bin/env node +/* + * Guard for the v1 frontend (src/v1/): the rules Biome cannot express. + * + * --files TypeScript only (a .js under v1 is skipped by tsc, not failed, + * because allowJs is off), style files only in styles/, and every + * file inside the v1 folder taxonomy. Run by `typecheck:v1`. + * --code The folder import contract (which v1 folder may import which), + * no PropTypes, no antd `ant-` class prefix. Run by `lint:v1`. + * + * With neither flag, both run. `--root ` points it at another tree + * (used by the fixtures in test-fixtures/v1-boundary/). + * + * The v0 <-> v1 boundary itself, `../` imports and banned packages are Biome + * rules (biome.json `overrides`), so they also run in CI and in editors. + * Biome overrides replace rule options rather than merge them, which is why + * the per-folder contract lives here and not as one override per folder. + * + * The `ant-` check exists only while antd markup survives in v0; drop it + * once the shadcn migration is complete. + */ +import fs from "node:fs"; +import path from "node:path"; + +const args = process.argv.slice(2); +const rootArg = args.indexOf("--root"); +const ROOT = path.resolve( + rootArg === -1 + ? path.join(import.meta.dirname, "..", "src", "v1") + : args[rootArg + 1], +); +const runFiles = args.includes("--files") || !args.includes("--code"); +const runCode = args.includes("--code") || !args.includes("--files"); + +const ALL = "*"; +/* + * Folder -> folders it may import from, besides itself. Mirrors the contract + * table in the P0-T01 spec and src/v1/README.md. `entry` is src/v1/index.ts. + * Pages may import everything except other pages. + */ +const CONTRACT = { + entry: ALL, + app: ALL, + plugins: ALL, + test: ALL, + pages: ALL, + "components/ui": ["lib", "styles"], + "components/patterns": [ + "components/ui", + "lib", + "services", + "stores", + "hooks", + ], + shell: [ + "components/ui", + "components/patterns", + "services", + "stores", + "hooks", + "lib", + ], + services: ["lib"], + stores: ["lib"], + hooks: ["lib"], + lib: [], + styles: ["lib"], +}; +const TOP_LEVEL = new Set([ + "app", + "shell", + "services", + "stores", + "hooks", + "lib", + "styles", + "plugins", + "test", +]); +const ROOT_FILES = new Set(["index.ts", "tsconfig.json", "README.md"]); + +const CODE_EXT = /\.(ts|tsx|mts|cts)$/; +const JS_EXT = /\.(js|jsx|mjs|cjs)$/; +const STYLE_EXT = /\.(css|scss|sass|less)$/; +const TEST_FILE = /\.(test|spec)\.tsx?$/; + +/* + * The v1 folder a path (relative to ROOT, `/`-separated) belongs to, e.g. + * "components/ui" or "pages/settings"; null when it is outside the taxonomy. + * `isDir` is for import targets like "@v1/lib", which name a folder. + */ +function folderOf(rel, isDir = false) { + const segs = rel.split("/").filter(Boolean); + const depth = isDir ? segs.length + 1 : segs.length; + if (segs.length === 0 || (depth === 1 && segs[0] === "index.ts")) { + return "entry"; + } + if (depth === 1) { + return null; + } + const [top, sub] = segs; + if (top === "components") { + return (sub === "ui" || sub === "patterns") && depth >= 3 + ? `components/${sub}` + : null; + } + if (top === "pages") { + return sub && depth >= 3 ? `pages/${sub}` : null; + } + return TOP_LEVEL.has(top) ? top : null; +} + +function contractKey(folder) { + return folder.startsWith("pages/") ? "pages" : folder; +} + +function mayImport(from, to, isTestFile) { + if (from === to) { + return true; + } + if (isTestFile && to === "test") { + return true; + } + if (from.startsWith("pages/") && to.startsWith("pages/")) { + return false; + } + const allowed = CONTRACT[contractKey(from)]; + return allowed === ALL || allowed.includes(to); +} + +/* Blanks comments while keeping line numbers, so matches can be reported. */ +function stripComments(src) { + return src + .replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " ")) + .replace(/(^|[^:"'`\\])\/\/.*$/gm, "$1"); +} + +const IMPORT_RES = [ + /\b(?:import|export)\s+(?:type\s+)?(?:[\w$*{}\s,]+?\s+from\s+)?["']([^"']+)["']/g, + /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g, + /\brequire\s*\(\s*["']([^"']+)["']\s*\)/g, +]; + +/* Resolves a specifier to a v1 folder, or undefined for packages and the + * forms Biome already rejects (`../`, `@/`). */ +function targetFolder(spec, fileRel) { + if (spec === "@v1" || /^@v1\/index(\.ts)?$/.test(spec)) { + return "entry"; + } + let rel; + if (spec.startsWith("@v1/")) { + rel = path.posix.normalize(spec.slice(4)); + } else if (spec.startsWith("./")) { + rel = path.posix.join(path.posix.dirname(fileRel), spec); + } else { + return undefined; + } + // No extension on the last segment -> it may name a folder ("@v1/lib"). + const isDir = !path.posix.extname(rel); + return folderOf(rel, isDir) ?? folderOf(rel) ?? null; +} + +function lineAt(text, index) { + return text.slice(0, index).split("\n").length; +} + +function walk(dir) { + return fs + .readdirSync(dir, { recursive: true, withFileTypes: true }) + .filter((d) => d.isFile()) + .map((d) => + path + .relative(ROOT, path.join(d.parentPath, d.name)) + .replaceAll(path.sep, "/"), + ) + .sort(); +} + +const problems = []; +const report = (rel, line, msg) => + problems.push(`src/v1/${rel}${line ? `:${line}` : ""}: ${msg}`); + +if (!fs.existsSync(ROOT)) { + console.error(`check-v1: ${ROOT} does not exist`); + process.exit(2); +} + +for (const rel of walk(ROOT)) { + const name = path.posix.basename(rel); + const isRootFile = !rel.includes("/"); + const folder = folderOf(rel); + + if (runFiles) { + if (JS_EXT.test(name)) { + report(rel, 0, "v1 is TypeScript only; use .ts or .tsx."); + } + if (STYLE_EXT.test(name) && folder !== "styles") { + report(rel, 0, "No component style files; only styles/ holds CSS."); + } + if ( + isRootFile + ? !ROOT_FILES.has(name) + : folder === null && name !== ".gitkeep" + ) { + report(rel, 0, "Outside the v1 folder taxonomy (see src/v1/README.md)."); + } + } + + if (!runCode || !CODE_EXT.test(name) || folder === null) { + continue; + } + const text = stripComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); + const isTestFile = TEST_FILE.test(name); + + for (const re of IMPORT_RES) { + for (const m of text.matchAll(re)) { + const to = targetFolder(m[1], rel); + if (to === undefined) { + continue; + } + if (to === null) { + report( + rel, + lineAt(text, m.index), + `'${m[1]}' is outside the v1 folder taxonomy.`, + ); + } else if (!mayImport(folder, to, isTestFile)) { + report( + rel, + lineAt(text, m.index), + `${folder}/ may not import from ${to}/ ('${m[1]}'); see the folder contract in src/v1/README.md.`, + ); + } + } + } + for (const m of text.matchAll(/\.propTypes\b|\bpropTypes\s*[:=]/g)) { + report( + rel, + lineAt(text, m.index), + "No PropTypes under v1; type props with TypeScript.", + ); + } + for (const m of text.matchAll(/(? 0) { + console.error(problems.join("\n")); + console.error(`\ncheck-v1: ${problems.length} problem(s).`); + process.exit(1); +} +console.log("check-v1: ok"); diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md new file mode 100644 index 0000000000..0e375909e4 --- /dev/null +++ b/frontend/src/v1/README.md @@ -0,0 +1,49 @@ +# Frontend v1 + +The v1 frontend, built alongside v0 (everything else under `src/`) until v0 is +retired. The full guide is the Frontend Developer Guide for v1 (P0-T11); this +file holds only the rules that tooling enforces. + +## Rules + +- **TypeScript only.** `.ts` / `.tsx`, `strict`, unused locals and parameters + are errors, no explicit `any`. +- **Nothing outside `src/v1`.** No v0 component, store, hook, helper, CSS or + shim. Import with `./` within a folder and `@v1/...` across folders; `../` is + never used. +- **v0 imports only the public entry**, `@v1` (`src/v1/index.ts`). +- **No PropTypes, no component `.css`, no antd** (package, shim or `ant-` + class). Global v1 styles live in `styles/`. + +## Folders and what each may import + +Every folder may import from itself. Test files (`*.test.ts(x)`) may also +import `test/`. + +| Folder | Holds | May import from | +| --- | --- | --- | +| `index.ts` | Public entry, used by the version switch | everything | +| `app/` | Entry, providers, router, route registry | everything | +| `components/ui/` | Core components | `lib/`, `styles/` | +| `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/` | +| `shell/` | Top bar, rail, context bar, tab bar | `components/ui/`, `components/patterns/`, `services/`, `stores/`, `hooks/`, `lib/` | +| `pages//` | One folder per target page | everything except other pages | +| `services/` | Typed service modules; the only place a URL appears | `lib/` | +| `stores/` | Zustand stores | `lib/` | +| `hooks/` | Shared hooks | `lib/` | +| `lib/` | Pure helpers | — | +| `styles/` | Tokens and global v1 styles | `lib/` | +| `plugins/` | Cloud overlay target; empty in the public repository | everything | +| `test/` | Render helpers, fixtures, network mocks | everything | + +## Scripts + +| Script | Checks | +| --- | --- | +| `bun run typecheck:v1` | TypeScript-only files and folder taxonomy, then `tsc` with `src/v1/tsconfig.json` | +| `bun run lint:v1` | Biome on `src/v1/`, then the folder contract, PropTypes and `ant-` checks | +| `bun run test:v1` | v1 unit tests, then the guardrail fixtures in `test-fixtures/v1-boundary/` | + +The v0 ↔ v1 boundary and the banned imports are Biome rules in `biome.json`, +so `biome ci src/` (CI) and editors report them too. The rest live in +`scripts/check-v1.mjs`. diff --git a/frontend/src/v1/app/.gitkeep b/frontend/src/v1/app/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/components/patterns/.gitkeep b/frontend/src/v1/components/patterns/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/components/ui/.gitkeep b/frontend/src/v1/components/ui/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/hooks/.gitkeep b/frontend/src/v1/hooks/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/index.ts b/frontend/src/v1/index.ts new file mode 100644 index 0000000000..8a6566ee67 --- /dev/null +++ b/frontend/src/v1/index.ts @@ -0,0 +1,7 @@ +/* + * Public entry of the v1 frontend — the only v1 module v0 may import, as + * `@v1`. Lint rejects any other v0 → v1 import (see src/v1/README.md). + * + * Empty until the version switch (P0-T03) mounts v1 through it. + */ +export {}; diff --git a/frontend/src/v1/lib/.gitkeep b/frontend/src/v1/lib/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/pages/.gitkeep b/frontend/src/v1/pages/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/plugins/manifest.ts b/frontend/src/v1/plugins/manifest.ts new file mode 100644 index 0000000000..dcfd546ab4 --- /dev/null +++ b/frontend/src/v1/plugins/manifest.ts @@ -0,0 +1,6 @@ +/* + * Cloud overlay target. Empty in the public repository: the cloud build + * copies its v1 plugins into this folder. The manifest's shape and contents + * belong to P0-T02 and P0-T07. + */ +export const pluginManifest = [] as const; diff --git a/frontend/src/v1/services/.gitkeep b/frontend/src/v1/services/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/shell/.gitkeep b/frontend/src/v1/shell/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/stores/.gitkeep b/frontend/src/v1/stores/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/styles/.gitkeep b/frontend/src/v1/styles/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/test/.gitkeep b/frontend/src/v1/test/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/tsconfig.json b/frontend/src/v1/tsconfig.json new file mode 100644 index 0000000000..f6b1c148ac --- /dev/null +++ b/frontend/src/v1/tsconfig.json @@ -0,0 +1,39 @@ +{ + /* + * TypeScript config for the v1 frontend, checked by `bun run typecheck:v1`. + * It lives here rather than beside tsconfig.json so editors pick it up for + * files under src/v1; tsconfig.json and jsconfig.json exclude this folder. + * + * `allowJs: false` makes tsc skip a .js file rather than fail on it, so the + * "TypeScript only" rule is enforced by scripts/check-v1.mjs, which + * typecheck:v1 runs first. + */ + "compilerOptions": { + "target": "ES2022", + "lib": ["ES2022", "DOM", "DOM.Iterable"], + "module": "ESNext", + "moduleResolution": "bundler", + "jsx": "react-jsx", + + "allowJs": false, + "noEmit": true, + "skipLibCheck": true, + "esModuleInterop": true, + "resolveJsonModule": true, + "isolatedModules": true, + "forceConsistentCasingInFileNames": true, + + "strict": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true, + + /* Only `@v1` — deliberately no `@/*`, so a v0 import fails typecheck as + * well as lint. Mirrored in vite.config.js and vitest.config.mjs. */ + "paths": { + "@v1": ["./index.ts"], + "@v1/*": ["./*"] + } + }, + "include": ["./**/*", "../../vite-env.d.ts"] +} diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs new file mode 100644 index 0000000000..a76001c182 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -0,0 +1,220 @@ +/* + * Proves the v1 guardrails fail when they should and pass when they should. + * Run by `bun run test:v1` (with `node --test`, not vitest, so the v0 suite + * never collects it). + * + * Each case builds a throwaway copy of the frontend: biome.json, + * package.json, scripts/, the real src/v1 scaffold and a node_modules + * symlink. It then overlays the files under cases// and runs the real + * package.json scripts (or a raw command) inside it. The known-bad files + * live here, outside src/, so they never reach the real lint or typecheck. + */ +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { test } from "node:test"; + +const FRONTEND = path.resolve(import.meta.dirname, "..", ".."); +const CASES_DIR = path.join(import.meta.dirname, "cases"); +const { scripts } = JSON.parse( + fs.readFileSync(path.join(FRONTEND, "package.json"), "utf8"), +); + +/* + * `run` holds package.json script names, or raw commands for the v0 side, + * which has no v1 script. `fails` is a substring the output must contain; + * cases without it must pass. + */ +const CASES = [ + // The empty scaffold passes everything. + { name: null, run: ["typecheck:v1", "lint:v1"] }, + + // Import boundary, v1 -> v0 (Biome). + { + name: "v1-imports-v0-alias", + run: ["lint:v1"], + fails: "v1 imports nothing outside src/v1", + }, + { + name: "v1-imports-v0-relative", + run: ["lint:v1"], + fails: "v1 imports nothing outside src/v1", + }, + + // Import boundary, v0 -> v1 (Biome). + { + name: "v0-imports-v1-internal", + run: ["biome lint src/"], + fails: "v0 may import only the v1 public entry", + }, + { + name: "v0-imports-v1-relative", + run: ["biome lint src/"], + fails: "v0 may import only the v1 public entry", + }, + { name: "v0-imports-v1-entry", run: ["biome lint src/"] }, + + // Banned under v1. + { name: "v1-prop-types", run: ["lint:v1"], fails: "PropTypes are not used" }, + { + name: "v1-prop-types-assignment", + run: ["lint:v1"], + fails: "No PropTypes under v1", + }, + { + name: "v1-component-css-import", + run: ["lint:v1"], + fails: "No component .css under src/v1", + }, + { + name: "v1-component-css-file", + run: ["typecheck:v1"], + fails: "No component style files", + }, + { + name: "v1-ant-class-prefix", + run: ["lint:v1"], + fails: "No antd 'ant-' class names", + }, + { name: "v1-explicit-any", run: ["lint:v1"], fails: "noExplicitAny" }, + // The v1 override must add to the shared rules, not replace them. + { + name: "v1-base-rules-still-apply", + run: ["lint:v1"], + fails: "noDoubleEquals", + }, + + // TypeScript only, strict. + { name: "v1-js-file", run: ["typecheck:v1"], fails: "TypeScript only" }, + { + name: "v1-implicit-any", + run: ["typecheck:v1"], + fails: "implicitly has an 'any' type", + }, + { + name: "v1-unused-local", + run: ["typecheck:v1"], + fails: "is declared but its value is never read", + }, + + // Folder taxonomy and contract. + { + name: "v1-outside-taxonomy", + run: ["typecheck:v1"], + fails: "Outside the v1 folder taxonomy", + }, + { + name: "v1-ui-imports-services", + run: ["lint:v1"], + fails: "components/ui/ may not import from services/", + }, + { + name: "v1-page-imports-other-page", + run: ["lint:v1"], + fails: "pages/projects/ may not import from pages/settings/", + }, + { name: "v1-allowed-imports", run: ["typecheck:v1", "lint:v1"] }, +]; + +function makeTree(caseName) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "v1-boundary-")); + for (const file of ["biome.json", "package.json", "vite-env.d.ts"]) { + fs.copyFileSync(path.join(FRONTEND, file), path.join(dir, file)); + } + // Copied, not linked: check-v1.mjs finds src/v1 relative to itself. + fs.cpSync(path.join(FRONTEND, "scripts"), path.join(dir, "scripts"), { + recursive: true, + }); + fs.cpSync(path.join(FRONTEND, "src", "v1"), path.join(dir, "src", "v1"), { + recursive: true, + }); + fs.symlinkSync( + path.join(FRONTEND, "node_modules"), + path.join(dir, "node_modules"), + "dir", + ); + if (caseName) { + fs.cpSync(path.join(CASES_DIR, caseName), dir, { recursive: true }); + } + // biome.json enables VCS integration with `useIgnoreFile`, which needs a + // repository root holding an ignore file. + execFileSync("git", ["init", "-q"], { cwd: dir }); + fs.writeFileSync(path.join(dir, ".gitignore"), "node_modules\n"); + return dir; +} + +function removeTree(dir) { + // Unlink first so the recursive delete can never walk into node_modules. + fs.unlinkSync(path.join(dir, "node_modules")); + fs.rmSync(dir, { recursive: true, force: true }); +} + +function runAll(dir, run) { + let output = ""; + for (const step of run) { + const command = scripts[step] ?? step; + const result = spawnSync("sh", ["-c", command], { + cwd: dir, + encoding: "utf8", + env: { + ...process.env, + NO_COLOR: "1", + PATH: `${path.join(dir, "node_modules", ".bin")}${path.delimiter}${process.env.PATH}`, + }, + }); + output += `$ ${command}\n${result.stdout}${result.stderr}`; + if (result.status !== 0) { + return { ok: false, output }; + } + } + return { ok: true, output }; +} + +/* + * The repository's Python `lib/` ignore rule also matches src/v1/lib/ and the + * fixtures' lib/ folders; .gitignore re-includes them. `--no-index` checks the + * rules even for files already tracked. + */ +test("no v1 or fixture file is git-ignored", () => { + const files = ["src/v1", "test-fixtures"].flatMap((top) => + fs + .readdirSync(path.join(FRONTEND, top), { + recursive: true, + withFileTypes: true, + }) + .filter((d) => d.isFile()) + .map((d) => path.relative(FRONTEND, path.join(d.parentPath, d.name))), + ); + const result = spawnSync("git", ["check-ignore", "--no-index", "--stdin"], { + cwd: FRONTEND, + encoding: "utf8", + input: files.join("\n"), + }); + // Exit 1 means "nothing matched"; 0 means some file is ignored. + assert.equal(result.status, 1, `git-ignored:\n${result.stdout}`); +}); + +test("every case folder is listed", () => { + const listed = new Set(CASES.map((c) => c.name).filter(Boolean)); + assert.deepEqual(fs.readdirSync(CASES_DIR).sort(), [...listed].sort()); +}); + +for (const { name, run, fails } of CASES) { + const label = `${name ?? "empty scaffold"}: ${fails ? "fails" : "passes"} ${run.join(", ")}`; + test(label, () => { + const dir = makeTree(name); + try { + const { ok, output } = runAll(dir, run); + if (fails) { + assert.equal(ok, false, `expected a failure, got:\n${output}`); + assert.ok(output.includes(fails), `expected "${fails}" in:\n${output}`); + } else { + assert.equal(ok, true, output); + } + } finally { + removeTree(dir); + } + }); +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-entry/src/pages/Ok.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-entry/src/pages/Ok.jsx new file mode 100644 index 0000000000..8e96182e95 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-entry/src/pages/Ok.jsx @@ -0,0 +1 @@ +import "@v1"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx new file mode 100644 index 0000000000..d822a2d078 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx @@ -0,0 +1,3 @@ +import { pluginManifest } from "@v1/plugins/manifest"; + +export const manifest = pluginManifest; diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx new file mode 100644 index 0000000000..d462e4d68a --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx @@ -0,0 +1,3 @@ +import { pluginManifest } from "../v1/plugins/manifest"; + +export const manifest = pluginManifest; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts new file mode 100644 index 0000000000..1cb7e9a430 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts @@ -0,0 +1,4 @@ +import { renderLabel } from "@v1/test/render"; +import { label } from "./Label"; + +export const rendered = renderLabel(label("ok")); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.ts new file mode 100644 index 0000000000..ef8a7487be --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.ts @@ -0,0 +1,3 @@ +import { upper } from "@v1/lib/format"; + +export const label = (text: string): string => upper(text); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/lib/format.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/lib/format.ts new file mode 100644 index 0000000000..ee479a8063 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/lib/format.ts @@ -0,0 +1 @@ +export const upper = (value: string): string => value.toUpperCase(); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.ts new file mode 100644 index 0000000000..b902577b52 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.ts @@ -0,0 +1 @@ +export const title = "Projects"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/view/heading.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/view/heading.ts new file mode 100644 index 0000000000..671f792f33 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/view/heading.ts @@ -0,0 +1,4 @@ +import { label } from "@v1/components/ui/Label"; +import { title } from "@v1/pages/projects/title"; + +export const heading = label(title); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/test/render.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/test/render.ts new file mode 100644 index 0000000000..858e72d057 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/test/render.ts @@ -0,0 +1 @@ +export const renderLabel = (text: string): string => text; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-ant-class-prefix/src/v1/components/ui/Button.ts b/frontend/test-fixtures/v1-boundary/cases/v1-ant-class-prefix/src/v1/components/ui/Button.ts new file mode 100644 index 0000000000..a87d0cde10 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-ant-class-prefix/src/v1/components/ui/Button.ts @@ -0,0 +1 @@ +export const buttonClass = "ant-btn primary"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-base-rules-still-apply/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-base-rules-still-apply/src/v1/lib/bad.ts new file mode 100644 index 0000000000..32f9a0a998 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-base-rules-still-apply/src/v1/lib/bad.ts @@ -0,0 +1 @@ +export const same = (a: number, b: number): boolean => a == b; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-component-css-file/src/v1/components/ui/Badge.css b/frontend/test-fixtures/v1-boundary/cases/v1-component-css-file/src/v1/components/ui/Badge.css new file mode 100644 index 0000000000..63bd92461e --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-component-css-file/src/v1/components/ui/Badge.css @@ -0,0 +1,3 @@ +.badge { + color: red; +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-component-css-import/src/v1/components/ui/Badge.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-component-css-import/src/v1/components/ui/Badge.tsx new file mode 100644 index 0000000000..be8a92895c --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-component-css-import/src/v1/components/ui/Badge.tsx @@ -0,0 +1,3 @@ +import "./Badge.css"; + +export const badge = "badge"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-explicit-any/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-explicit-any/src/v1/lib/bad.ts new file mode 100644 index 0000000000..6724498208 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-explicit-any/src/v1/lib/bad.ts @@ -0,0 +1 @@ +export const value: any = 1; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-implicit-any/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-implicit-any/src/v1/lib/bad.ts new file mode 100644 index 0000000000..cf9f27088d --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-implicit-any/src/v1/lib/bad.ts @@ -0,0 +1,3 @@ +export function identity(value) { + return value; +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts new file mode 100644 index 0000000000..b6e5854b2b --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts @@ -0,0 +1,3 @@ +import { getBaseUrl } from "@/helpers/GetStaticData"; + +export const base = getBaseUrl; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts new file mode 100644 index 0000000000..d2c250ae68 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts @@ -0,0 +1,3 @@ +import { getBaseUrl } from "../../helpers/GetStaticData"; + +export const base = getBaseUrl; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-js-file/src/v1/lib/legacy.js b/frontend/test-fixtures/v1-boundary/cases/v1-js-file/src/v1/lib/legacy.js new file mode 100644 index 0000000000..7e15a4a640 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-js-file/src/v1/lib/legacy.js @@ -0,0 +1 @@ +export const legacy = 1; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-outside-taxonomy/src/v1/widgets/thing.ts b/frontend/test-fixtures/v1-boundary/cases/v1-outside-taxonomy/src/v1/widgets/thing.ts new file mode 100644 index 0000000000..61483b3477 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-outside-taxonomy/src/v1/widgets/thing.ts @@ -0,0 +1 @@ +export const thing = 1; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts new file mode 100644 index 0000000000..1a075e1cc3 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts @@ -0,0 +1,3 @@ +import { settingsTitle } from "@v1/pages/settings"; + +export const title = settingsTitle; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/settings/index.ts b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/settings/index.ts new file mode 100644 index 0000000000..317694fcb8 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/settings/index.ts @@ -0,0 +1 @@ +export const settingsTitle = "Settings"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-assignment/src/v1/components/ui/Box.ts b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-assignment/src/v1/components/ui/Box.ts new file mode 100644 index 0000000000..e6e9ef769a --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-assignment/src/v1/components/ui/Box.ts @@ -0,0 +1,4 @@ +export function Box(): null { + return null; +} +Box.propTypes = {}; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-prop-types/src/v1/components/ui/Badge.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types/src/v1/components/ui/Badge.tsx new file mode 100644 index 0000000000..64c5471dfb --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types/src/v1/components/ui/Badge.tsx @@ -0,0 +1,3 @@ +import PropTypes from "prop-types"; + +export const badgeShape = PropTypes.string; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx new file mode 100644 index 0000000000..3863836b20 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx @@ -0,0 +1,3 @@ +import { projectsUrl } from "@v1/services/projects"; + +export const href = projectsUrl; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/services/projects.ts b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/services/projects.ts new file mode 100644 index 0000000000..934524c2f4 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/services/projects.ts @@ -0,0 +1 @@ +export const projectsUrl = "/api/v1/projects"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-unused-local/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-unused-local/src/v1/lib/bad.ts new file mode 100644 index 0000000000..ac320e04db --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-unused-local/src/v1/lib/bad.ts @@ -0,0 +1,4 @@ +export function double(value: number): number { + const unused = 1; + return value * 2; +} diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index f7e7014f49..779791063e 100644 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -40,9 +40,12 @@ * `baseUrl` was removed in TypeScript 6, so the paths are relative to * this file's directory. */ "paths": { - "@/*": ["./src/*"] + "@/*": ["./src/*"], + /* v0 may import only the v1 public entry, so only that is mapped. */ + "@v1": ["./src/v1/index.ts"] } }, "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"], - "exclude": ["node_modules", "build", "dist"] + /* src/v1 is checked by src/v1/tsconfig.json under stricter settings. */ + "exclude": ["node_modules", "build", "dist", "src/v1"] } diff --git a/frontend/vite.config.js b/frontend/vite.config.js index af124153fc..e9882c5d8f 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -184,6 +184,8 @@ export default defineConfig(({ mode }) => { resolve: { alias: { "@": path.resolve(import.meta.dirname, "./src"), + // v1 frontend; mirrored in src/v1/tsconfig.json and vitest.config.mjs. + "@v1": path.resolve(import.meta.dirname, "./src/v1"), }, }, diff --git a/frontend/vitest.config.mjs b/frontend/vitest.config.mjs index 128c7f042f..915c23d11d 100644 --- a/frontend/vitest.config.mjs +++ b/frontend/vitest.config.mjs @@ -94,6 +94,8 @@ export default defineConfig({ resolve: { alias: { "@": path.resolve(import.meta.dirname, "./src"), + // Mirror of the `@v1` alias in vite.config.js. + "@v1": path.resolve(import.meta.dirname, "./src/v1"), }, }, test: { @@ -107,8 +109,16 @@ export default defineConfig({ * The Playwright suite (the rig's `ui` group) lives in `tests/e2e/ui`, out * of this tree entirely, so vitest's default glob no longer reaches it. * `build/` is listed because a production build lands there and its - * bundled chunks would otherwise be walked. + * bundled chunks would otherwise be walked. `test-fixtures/` holds + * deliberately broken trees (some with `*.test.ts` files) that + * test-fixtures/v1-boundary/boundary.fixtures.mjs lints and type-checks; + * they are inputs, not tests. */ - exclude: ["**/node_modules/**", "**/dist/**", "**/build/**"], + exclude: [ + "**/node_modules/**", + "**/dist/**", + "**/build/**", + "test-fixtures/**", + ], }, }); From 5c55a7c12aca0e21c69094b2d39456e702f8f78f Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Mon, 26 Oct 2026 18:04:01 +0530 Subject: [PATCH 02/26] fix(frontend): close v1 guardrail bypasses found in self-review (UN-4270) check-v1.mjs: - Scan comments with a string/template-aware tokenizer; a `/*` or `//` inside a string or line comment no longer hides the imports after it. - Read template-literal specifiers, import.meta.glob and `import(..., opts)`. - Flag specifiers that leave src/v1 (`/`, `..`, `@/`, `src/`) itself, not only via Biome, which skips template literals. - Check `@import`/`url()` and `ant-` in styles/ CSS; report symlinks; catch built `ant-` names; treat .test/.spec .mts/.cts as test files; ignore .DS_Store; drop the unused --root option. biome.json: also ban `..`, `./../**`, `/**`, `antd/**`, `prop-types/**`. tsconfig: skipLibCheck off so v1's own .d.ts files are checked. .gitignore: re-include lib/ at any depth under src/v1. Fixtures: a generated matrix over every folder-contract pair, every import form, every outside spelling, a symlink case, and cases for each new rule; tsc cases assert error codes; output matching excludes the command text and spawn errors fail loudly. Comment and README corrections. --- .gitignore | 2 +- frontend/biome.json | 15 +- frontend/scripts/check-v1.mjs | 178 ++++++++--- frontend/src/v1/README.md | 23 +- frontend/src/v1/index.ts | 2 +- frontend/src/v1/plugins/manifest.ts | 2 +- frontend/src/v1/tsconfig.json | 9 +- .../v1-boundary/boundary.fixtures.mjs | 294 ++++++++++++++++-- .../src/v1/components/ui/Tag.tsx | 7 + .../v1-allowed-imports/src/v1/styles/index.ts | 3 + .../src/v1/styles/tokens.css | 3 + .../src/v1/components/ui/Tag.ts | 1 + .../src/v1/components/ui/Button.ts | 3 + .../src/v1/components/ui/Button.ts | 3 + .../src/v1/lib/logo.ts | 3 + .../src/v1/components/ui/Helper.ts | 3 + .../src/v1/test/render.ts | 1 + .../src/v1/components/ui/Badge.ts | 3 + .../cases/v1-root-file/src/v1/main.ts | 1 + .../cases/v1-strict-null/src/v1/lib/length.ts | 1 + .../src/v1/styles/global.css | 3 + .../src/v1/styles/global.css | 1 + .../src/v1/components/ui/Link.test.ts | 3 + .../src/v1/services/projects.ts | 1 + .../v1-unused-parameter/src/v1/lib/one.ts | 1 + frontend/vite.config.js | 3 +- frontend/vitest.config.mjs | 2 +- 27 files changed, 494 insertions(+), 77 deletions(-) create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/index.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/tokens.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-ant-built-class/src/v1/components/ui/Tag.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-antd-subpath/src/v1/components/ui/Button.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-imports-root-absolute/src/v1/lib/logo.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/components/ui/Helper.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/test/render.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-prop-types-subpath/src/v1/components/ui/Badge.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-root-file/src/v1/main.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-strict-null/src/v1/lib/length.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-class/src/v1/styles/global.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-styles-import-outside/src/v1/styles/global.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/components/ui/Link.test.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/services/projects.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-unused-parameter/src/v1/lib/one.ts diff --git a/.gitignore b/.gitignore index c7a5148e30..4817bafa05 100644 --- a/.gitignore +++ b/.gitignore @@ -45,7 +45,7 @@ lib64/ # v1 guardrail fixtures. Keep those tracked, or their files are silently left # out of commits. !frontend/src/lib/ -!frontend/src/v1/lib/ +!frontend/src/v1/**/lib/ !frontend/test-fixtures/**/lib/ parts/ sdist/ diff --git a/frontend/biome.json b/frontend/biome.json index c8ead404ef..6799918af6 100644 --- a/frontend/biome.json +++ b/frontend/biome.json @@ -145,13 +145,24 @@ }, "patterns": [ { - "group": ["../**", "@/**", "src/**"], + "group": [ + "../**", + "..", + "./../**", + "/**", + "@/**", + "src/**" + ], "message": "v1 imports nothing outside src/v1, and never with '../'. Use './' within a folder and '@v1/...' across folders." }, { - "group": ["@ant-design/**"], + "group": ["@ant-design/**", "antd/**"], "message": "antd is not used under src/v1." }, + { + "group": ["prop-types/**"], + "message": "v1 is typed with TypeScript; PropTypes are not used under src/v1." + }, { "group": ["**/*.css", "!@v1/styles/**"], "message": "No component .css under src/v1; style with tokens and Tailwind. Global v1 styles live in @v1/styles/." diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index 6a27cb1783..e5d6612ae9 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -5,16 +5,19 @@ * --files TypeScript only (a .js under v1 is skipped by tsc, not failed, * because allowJs is off), style files only in styles/, and every * file inside the v1 folder taxonomy. Run by `typecheck:v1`. - * --code The folder import contract (which v1 folder may import which), - * no PropTypes, no antd `ant-` class prefix. Run by `lint:v1`. + * --code Imports: nothing outside src/v1 and the folder import contract + * (which v1 folder may import which). No PropTypes, no antd `ant-` + * class names. Run by `lint:v1`. * - * With neither flag, both run. `--root ` points it at another tree - * (used by the fixtures in test-fixtures/v1-boundary/). + * With neither flag, both run. It always checks the src/v1 beside this + * script; the fixtures in test-fixtures/v1-boundary/ copy it into each + * throwaway tree for that reason. * - * The v0 <-> v1 boundary itself, `../` imports and banned packages are Biome - * rules (biome.json `overrides`), so they also run in CI and in editors. - * Biome overrides replace rule options rather than merge them, which is why - * the per-folder contract lives here and not as one override per folder. + * The v0 <-> v1 boundary is also a Biome rule (biome.json `overrides`), so CI + * and editors report it. The script repeats the v1 -> outside half because it + * also reads template-literal imports, which Biome does not. Biome overrides + * replace rule options rather than merge them, which is why the per-folder + * contract lives here and not as one override per folder. * * The `ant-` check exists only while antd markup survives in v0; drop it * once the shadcn migration is complete. @@ -22,21 +25,16 @@ import fs from "node:fs"; import path from "node:path"; +const ROOT = path.join(import.meta.dirname, "..", "src", "v1"); const args = process.argv.slice(2); -const rootArg = args.indexOf("--root"); -const ROOT = path.resolve( - rootArg === -1 - ? path.join(import.meta.dirname, "..", "src", "v1") - : args[rootArg + 1], -); const runFiles = args.includes("--files") || !args.includes("--code"); const runCode = args.includes("--code") || !args.includes("--files"); const ALL = "*"; /* - * Folder -> folders it may import from, besides itself. Mirrors the contract - * table in the P0-T01 spec and src/v1/README.md. `entry` is src/v1/index.ts. - * Pages may import everything except other pages. + * Folder -> folders it may import from, besides itself. `entry` is + * src/v1/index.ts. Pages may import everything except other pages. + * Mirrors the table in src/v1/README.md; change both together. */ const CONTRACT = { entry: ALL, @@ -82,10 +80,12 @@ const ROOT_FILES = new Set(["index.ts", "tsconfig.json", "README.md"]); const CODE_EXT = /\.(ts|tsx|mts|cts)$/; const JS_EXT = /\.(js|jsx|mjs|cjs)$/; const STYLE_EXT = /\.(css|scss|sass|less)$/; -const TEST_FILE = /\.(test|spec)\.tsx?$/; +const TEST_FILE = /\.(test|spec)\.(ts|tsx|mts|cts)$/; +// Specifiers that leave src/v1: root-absolute, parent, the v0 alias, src/. +const OUTSIDE = /^(\/|\.\.(\/|$)|@\/|src\/)/; /* - * The v1 folder a path (relative to ROOT, `/`-separated) belongs to, e.g. + * The v1 folder a path (relative to src/v1, `/`-separated) belongs to, e.g. * "components/ui" or "pages/settings"; null when it is outside the taxonomy. * `isDir` is for import targets like "@v1/lib", which name a folder. */ @@ -128,22 +128,93 @@ function mayImport(from, to, isTestFile) { return allowed === ALL || allowed.includes(to); } -/* Blanks comments while keeping line numbers, so matches can be reported. */ -function stripComments(src) { - return src - .replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " ")) - .replace(/(^|[^:"'`\\])\/\/.*$/gm, "$1"); +/* + * Blanks comments, keeping line numbers so matches can be reported. Tracks + * strings and template literals (including `${}` expressions), so a `/*` or + * `//` inside a string does not swallow the code after it. Regex literals are + * not tracked; a quote inside one is read as a string to the end of the line + * at most, so it can hide that line only. + */ +function blankComments(src) { + let out = ""; + let i = 0; + // "`" = inside a template literal; "{" = inside a `${}` or a brace in one. + const stack = []; + while (i < src.length) { + const c = src[i]; + const next = src[i + 1]; + if (stack.at(-1) === "`") { + if (c === "\\") { + out += src.slice(i, i + 2); + i += 2; + } else if (c === "`") { + stack.pop(); + out += c; + i += 1; + } else if (c === "$" && next === "{") { + stack.push("{"); + out += "${"; + i += 2; + } else { + out += c; + i += 1; + } + continue; + } + if (c === "/" && (next === "/" || next === "*")) { + const close = + next === "/" ? src.indexOf("\n", i) : src.indexOf("*/", i + 2); + const end = close === -1 ? src.length : next === "/" ? close : close + 2; + out += src.slice(i, end).replace(/[^\n]/g, " "); + i = end; + } else if (c === '"' || c === "'") { + let j = i + 1; + while (j < src.length && src[j] !== c && src[j] !== "\n") { + j += src[j] === "\\" ? 2 : 1; + } + out += src.slice(i, j + 1); + i = j + 1; + } else { + if (c === "`") { + stack.push("`"); + } else if (c === "{" && stack.length > 0) { + stack.push("{"); + } else if (c === "}" && stack.length > 0) { + stack.pop(); + } + out += c; + i += 1; + } + } + return out; } +// A specifier: a quoted or backtick string with no interpolation. +const SPEC = "[\"'`]([^\"'`$]+)[\"'`]"; const IMPORT_RES = [ - /\b(?:import|export)\s+(?:type\s+)?(?:[\w$*{}\s,]+?\s+from\s+)?["']([^"']+)["']/g, - /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g, - /\brequire\s*\(\s*["']([^"']+)["']\s*\)/g, + new RegExp( + `\\b(?:import|export)\\s+(?:type\\s+)?(?:[\\w$*{}\\s,]+?\\s+from\\s+)?${SPEC}`, + "g", + ), + new RegExp(`\\bimport\\s*\\(\\s*${SPEC}\\s*[,)]`, "g"), + new RegExp(`\\brequire\\s*\\(\\s*${SPEC}\\s*\\)`, "g"), + // Vite globs; the first pattern of an array is checked. + new RegExp( + `\\bimport\\.meta\\.glob\\s*(?:<[^>]*>)?\\(\\s*\\[?\\s*${SPEC}`, + "g", + ), ]; +// CSS `@import` and `url()` targets, for the files in styles/. +const CSS_IMPORT_RE = /(?:@import\s+(?:url\(\s*)?|url\(\s*)["']?([^"')\s;]+)/g; -/* Resolves a specifier to a v1 folder, or undefined for packages and the - * forms Biome already rejects (`../`, `@/`). */ +/* + * Classifies a specifier: "outside" when it leaves src/v1; the v1 folder it + * names; null for a v1 path outside the taxonomy; undefined for a package. + */ function targetFolder(spec, fileRel) { + if (OUTSIDE.test(spec)) { + return "outside"; + } if (spec === "@v1" || /^@v1\/index(\.ts)?$/.test(spec)) { return "entry"; } @@ -155,6 +226,9 @@ function targetFolder(spec, fileRel) { } else { return undefined; } + if (rel.startsWith("..")) { + return "outside"; + } // No extension on the last segment -> it may name a folder ("@v1/lib"). const isDir = !path.posix.extname(rel); return folderOf(rel, isDir) ?? folderOf(rel) ?? null; @@ -164,10 +238,15 @@ function lineAt(text, index) { return text.slice(0, index).split("\n").length; } +// Finder litter; never committed (.gitignore), so not worth reporting. +const IGNORED = new Set([".DS_Store"]); + +/* Every file under dir, plus symlinks, which are reported rather than + * followed: a linked file would bring outside code in unchecked. */ function walk(dir) { return fs .readdirSync(dir, { recursive: true, withFileTypes: true }) - .filter((d) => d.isFile()) + .filter((d) => (d.isFile() || d.isSymbolicLink()) && !IGNORED.has(d.name)) .map((d) => path .relative(ROOT, path.join(d.parentPath, d.name)) @@ -190,6 +269,13 @@ for (const rel of walk(ROOT)) { const isRootFile = !rel.includes("/"); const folder = folderOf(rel); + if (fs.lstatSync(path.join(ROOT, rel)).isSymbolicLink()) { + if (runFiles) { + report(rel, 0, "No symlinks under v1; they bypass these checks."); + } + continue; + } + if (runFiles) { if (JS_EXT.test(name)) { report(rel, 0, "v1 is TypeScript only; use .ts or .tsx."); @@ -206,28 +292,47 @@ for (const rel of walk(ROOT)) { } } + if (runCode && STYLE_EXT.test(name) && folder === "styles") { + const css = blankComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); + for (const m of css.matchAll(CSS_IMPORT_RE)) { + if (targetFolder(m[1], rel) === "outside") { + report( + rel, + lineAt(css, m.index), + `'${m[1]}' is outside src/v1; v1 styles import nothing outside it.`, + ); + } + } + for (const m of css.matchAll(/(?/ and runs the real - * package.json scripts (or a raw command) inside it. The known-bad files - * live here, outside src/, so they never reach the real lint or typecheck. + * Each case builds a minimal throwaway copy of the frontend (biome.json, + * package.json, vite-env.d.ts, scripts/, the real src/v1 scaffold, a + * node_modules symlink and a git root), overlays the files under + * cases// and runs the real package.json scripts, or a raw command, + * inside it. The known-bad files live here, outside src/, so CI lint, the + * pre-commit hook and typecheck never see them. */ import assert from "node:assert/strict"; import { execFileSync, spawnSync } from "node:child_process"; @@ -24,8 +25,8 @@ const { scripts } = JSON.parse( /* * `run` holds package.json script names, or raw commands for the v0 side, - * which has no v1 script. `fails` is a substring the output must contain; - * cases without it must pass. + * which has no v1 script. `fails` is a substring the tool output must + * contain; cases without it must pass. */ const CASES = [ // The empty scaffold passes everything. @@ -42,6 +43,11 @@ const CASES = [ run: ["lint:v1"], fails: "v1 imports nothing outside src/v1", }, + { + name: "v1-imports-root-absolute", + run: ["lint:v1"], + fails: "v1 imports nothing outside src/v1", + }, // Import boundary, v0 -> v1 (Biome). { @@ -63,6 +69,31 @@ const CASES = [ run: ["lint:v1"], fails: "No PropTypes under v1", }, + { + name: "v1-antd-import", + run: ["lint:v1"], + fails: "antd is not used under src/v1", + }, + { + name: "v1-antd-subpath", + run: ["lint:v1"], + fails: "antd is not used under src/v1", + }, + { + name: "v1-prop-types-subpath", + run: ["lint:v1"], + fails: "PropTypes are not used", + }, + { + name: "v1-styles-import-outside", + run: ["lint:v1"], + fails: "v1 styles import nothing outside it", + }, + { + name: "v1-styles-ant-class", + run: ["lint:v1"], + fails: "No antd 'ant-' class names", + }, { name: "v1-component-css-import", run: ["lint:v1"], @@ -78,6 +109,11 @@ const CASES = [ run: ["lint:v1"], fails: "No antd 'ant-' class names", }, + { + name: "v1-ant-built-class", + run: ["lint:v1"], + fails: "No antd 'ant-' class names", + }, { name: "v1-explicit-any", run: ["lint:v1"], fails: "noExplicitAny" }, // The v1 override must add to the shared rules, not replace them. { @@ -91,20 +127,35 @@ const CASES = [ { name: "v1-implicit-any", run: ["typecheck:v1"], - fails: "implicitly has an 'any' type", + fails: "TS7006", + }, + { + name: "v1-strict-null", + run: ["typecheck:v1"], + fails: "TS18048", }, { name: "v1-unused-local", run: ["typecheck:v1"], - fails: "is declared but its value is never read", + fails: "TS6133", + }, + { + name: "v1-unused-parameter", + run: ["typecheck:v1"], + fails: "TS6133", }, - // Folder taxonomy and contract. + // Folder taxonomy and contract (the full contract is the matrix below). { name: "v1-outside-taxonomy", run: ["typecheck:v1"], fails: "Outside the v1 folder taxonomy", }, + { + name: "v1-root-file", + run: ["typecheck:v1"], + fails: "Outside the v1 folder taxonomy", + }, { name: "v1-ui-imports-services", run: ["lint:v1"], @@ -115,6 +166,17 @@ const CASES = [ run: ["lint:v1"], fails: "pages/projects/ may not import from pages/settings/", }, + // Only test files may use test/, and being a test file allows nothing else. + { + name: "v1-non-test-imports-test", + run: ["lint:v1"], + fails: "components/ui/ may not import from test/", + }, + { + name: "v1-test-file-imports-services", + run: ["lint:v1"], + fails: "components/ui/ may not import from services/", + }, { name: "v1-allowed-imports", run: ["typecheck:v1", "lint:v1"] }, ]; @@ -151,8 +213,20 @@ function removeTree(dir) { fs.rmSync(dir, { recursive: true, force: true }); } +function writeFile(dir, rel, content) { + const file = path.join(dir, rel); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); +} + +/* + * Runs the steps in order, stopping at the first failure. `output` holds only + * what the tools printed, so an expected message can never match the command + * text. A command that could not start at all is an error, not a "failure". + */ function runAll(dir, run) { let output = ""; + let log = ""; for (const step of run) { const command = scripts[step] ?? step; const result = spawnSync("sh", ["-c", command], { @@ -164,12 +238,34 @@ function runAll(dir, run) { PATH: `${path.join(dir, "node_modules", ".bin")}${path.delimiter}${process.env.PATH}`, }, }); - output += `$ ${command}\n${result.stdout}${result.stderr}`; + if (result.error || result.status === null) { + throw new Error( + `could not run "${command}": ${result.error ?? result.signal}`, + ); + } + // 127: sh could not find the command, e.g. a tool missing from PATH. + assert.notEqual(result.status, 127, `command not found: ${command}`); + output += result.stdout + result.stderr; + log += `$ ${command}\n${result.stdout}${result.stderr}`; if (result.status !== 0) { - return { ok: false, output }; + return { ok: false, output, log }; + } + } + return { ok: true, output, log }; +} + +/* Runs `check-v1.mjs --code` over generated files; returns the reported lines. */ +function checkCode(files) { + const dir = makeTree(null); + try { + for (const [rel, content] of Object.entries(files)) { + writeFile(dir, rel, content); } + const { output } = runAll(dir, ["node scripts/check-v1.mjs --code"]); + return output.split("\n").filter((l) => l.startsWith("src/v1/")); + } finally { + removeTree(dir); } - return { ok: true, output }; } /* @@ -184,9 +280,11 @@ test("no v1 or fixture file is git-ignored", () => { recursive: true, withFileTypes: true, }) - .filter((d) => d.isFile()) + .filter((d) => d.isFile() && d.name !== ".DS_Store") .map((d) => path.relative(FRONTEND, path.join(d.parentPath, d.name))), ); + // A lib/ folder anywhere under v1, not only the top-level one. + files.push("src/v1/pages/projects/lib/x.ts", "src/v1/lib/sub/lib/x.ts"); const result = spawnSync("git", ["check-ignore", "--no-index", "--stdin"], { cwd: FRONTEND, encoding: "utf8", @@ -206,15 +304,171 @@ for (const { name, run, fails } of CASES) { test(label, () => { const dir = makeTree(name); try { - const { ok, output } = runAll(dir, run); + const { ok, output, log } = runAll(dir, run); if (fails) { - assert.equal(ok, false, `expected a failure, got:\n${output}`); - assert.ok(output.includes(fails), `expected "${fails}" in:\n${output}`); + assert.equal(ok, false, `expected a failure, got:\n${log}`); + assert.ok(output.includes(fails), `expected "${fails}" in:\n${log}`); } else { - assert.equal(ok, true, output); + assert.equal(ok, true, log); } } finally { removeTree(dir); } }); } + +/* + * The folder contract, written out independently of check-v1.mjs so a typo + * in either shows up here. Source: the table in src/v1/README.md. Folders + * may always import themselves; pages/a and pages/b stand for two pages. + */ +const FOLDERS = { + entry: "", + app: "app", + "components/ui": "components/ui", + "components/patterns": "components/patterns", + shell: "shell", + "pages/a": "pages/a", + "pages/b": "pages/b", + services: "services", + stores: "stores", + hooks: "hooks", + lib: "lib", + styles: "styles", + plugins: "plugins", + test: "test", +}; +const EVERYTHING = Object.keys(FOLDERS); +const MAY_IMPORT = { + app: EVERYTHING, + plugins: EVERYTHING, + test: EVERYTHING, + "pages/a": EVERYTHING.filter((f) => f !== "pages/b"), + "components/ui": ["lib", "styles"], + "components/patterns": [ + "components/ui", + "lib", + "services", + "stores", + "hooks", + ], + shell: [ + "components/ui", + "components/patterns", + "services", + "stores", + "hooks", + "lib", + ], + services: ["lib"], + stores: ["lib"], + hooks: ["lib"], + lib: [], + styles: ["lib"], +}; + +test("folder contract: every folder pair, by file and by folder", () => { + const files = {}; + const expected = []; + for (const [from, allowed] of Object.entries(MAY_IMPORT)) { + for (const to of EVERYTHING) { + const rel = `src/v1/${FOLDERS[from]}/m-${to.replace("/", "-")}.ts`; + // The entry is a file, so it has only the one form. + files[rel] = + to === "entry" + ? 'import "@v1";\n' + : `import "@v1/${FOLDERS[to]}/target";\nimport "@v1/${FOLDERS[to]}";\n`; + if (from !== to && !allowed.includes(to)) { + const lines = to === "entry" ? [1] : [1, 2]; + for (const line of lines) { + expected.push(`${rel}:${line}: ${from}/ may not import from ${to}/`); + } + } + } + } + const reported = checkCode(files).map((l) => l.replace(/ \(.*$/, "")); + assert.deepEqual(reported.sort(), expected.sort()); +}); + +test("folder contract: every import form is read", () => { + // Each file breaks the contract once (components/ui -> services). + const forms = { + static: 'import { a } from "@v1/services/a";', + multiline: 'import {\n a,\n b,\n} from "@v1/services/a";', + type: 'import type { A } from "@v1/services/a";', + inlineType: 'import { type A } from "@v1/services/a";', + sideEffect: 'import "@v1/services/a";', + namespace: 'import * as a from "@v1/services/a";', + exportFrom: 'export { a } from "@v1/services/a";', + exportStar: 'export * from "@v1/services/a";', + exportStarAs: 'export * as a from "@v1/services/a";', + dynamic: 'export const a = () => import("@v1/services/a");', + dynamicTemplate: "export const a = () => import(`@v1/services/a`);", + require: 'export const a = require("@v1/services/a");', + relative: 'import { a } from "./../../services/a";', + afterSlashStarInString: + 'export const route = "/projects/*";\nimport { a } from "@v1/services/a";\n/** doc */', + afterSlashStarInComment: + '// matches /api/*\nimport { a } from "@v1/services/a";\n/** doc */', + afterDoubleSlashInString: + 'const u = "a//b"; export { a } from "@v1/services/a";', + glob: 'export const a = import.meta.glob("@v1/services/*.ts");', + afterTemplateExpression: + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + 'const t = `${"}"}/*`;\nimport { a } from "@v1/services/a";\n/** doc */', + }; + const files = {}; + for (const [form, code] of Object.entries(forms)) { + files[`src/v1/components/ui/${form}.ts`] = `${code}\n`; + } + const reported = checkCode(files); + for (const form of Object.keys(forms)) { + const file = `src/v1/components/ui/${form}.ts:`; + assert.ok( + reported.some((l) => l.startsWith(file)), + `${form}: not reported\n${reported.join("\n")}`, + ); + } +}); + +test("a symlinked file under v1 fails typecheck:v1", () => { + const dir = makeTree(null); + try { + fs.symlinkSync( + path.join(dir, "vite-env.d.ts"), + path.join(dir, "src", "v1", "lib", "linked.d.ts"), + ); + const { ok, output, log } = runAll(dir, ["typecheck:v1"]); + assert.equal(ok, false, log); + assert.ok(output.includes("No symlinks under v1"), log); + } finally { + removeTree(dir); + } +}); + +test("v1 imports nothing outside src/v1, in any spelling", () => { + const specs = [ + "@/helpers/x", + "../helpers/x", + "..", + "./../../helpers/x", + "/src/assets/logo.svg", + "src/helpers/x", + ]; + const files = {}; + specs.forEach((spec, i) => { + files[`src/v1/lib/outside-${i}.ts`] = + `export const a = () => import(\`${spec}\`);\n`; + }); + const reported = checkCode(files); + specs.forEach((spec, i) => { + assert.ok( + reported.some( + (l) => + l.startsWith(`src/v1/lib/outside-${i}.ts:`) && + l.includes("is outside src/v1"), + ), + `'${spec}': not reported\n${reported.join("\n")}`, + ); + }); +}); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx new file mode 100644 index 0000000000..f00b602b5d --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx @@ -0,0 +1,7 @@ +import { upper } from "@v1/lib/format"; + +type TagProps = { label: string }; + +export function Tag({ label }: TagProps) { + return {upper(label)}; +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/index.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/index.ts new file mode 100644 index 0000000000..7ab5ff7d43 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/index.ts @@ -0,0 +1,3 @@ +import "@v1/styles/tokens.css"; + +export const themed = true; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/tokens.css b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/tokens.css new file mode 100644 index 0000000000..ac38c30204 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/styles/tokens.css @@ -0,0 +1,3 @@ +:root { + --v1-accent: #000; +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-ant-built-class/src/v1/components/ui/Tag.ts b/frontend/test-fixtures/v1-boundary/cases/v1-ant-built-class/src/v1/components/ui/Tag.ts new file mode 100644 index 0000000000..758e2c7350 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-ant-built-class/src/v1/components/ui/Tag.ts @@ -0,0 +1 @@ +export const tagClass = (kind: string): string => `ant-${kind}`; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts b/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts new file mode 100644 index 0000000000..fda637b2e3 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts @@ -0,0 +1,3 @@ +import { Button } from "antd"; + +export const AntButton = Button; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-antd-subpath/src/v1/components/ui/Button.ts b/frontend/test-fixtures/v1-boundary/cases/v1-antd-subpath/src/v1/components/ui/Button.ts new file mode 100644 index 0000000000..be5137ace6 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-antd-subpath/src/v1/components/ui/Button.ts @@ -0,0 +1,3 @@ +import Button from "antd/es/button"; + +export const AntButton = Button; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-imports-root-absolute/src/v1/lib/logo.ts b/frontend/test-fixtures/v1-boundary/cases/v1-imports-root-absolute/src/v1/lib/logo.ts new file mode 100644 index 0000000000..b306ec899d --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-imports-root-absolute/src/v1/lib/logo.ts @@ -0,0 +1,3 @@ +import logo from "/src/assets/logo.svg"; + +export const logoUrl = logo; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/components/ui/Helper.ts b/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/components/ui/Helper.ts new file mode 100644 index 0000000000..3ae7c334e1 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/components/ui/Helper.ts @@ -0,0 +1,3 @@ +import { render } from "@v1/test/render"; + +export const helper = render("x"); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/test/render.ts b/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/test/render.ts new file mode 100644 index 0000000000..a70ecac7f4 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-non-test-imports-test/src/v1/test/render.ts @@ -0,0 +1 @@ +export const render = (text: string): string => text; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-subpath/src/v1/components/ui/Badge.ts b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-subpath/src/v1/components/ui/Badge.ts new file mode 100644 index 0000000000..cec0f63a16 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-prop-types-subpath/src/v1/components/ui/Badge.ts @@ -0,0 +1,3 @@ +import PropTypes from "prop-types/index"; + +export const badgeShape = PropTypes.string; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-root-file/src/v1/main.ts b/frontend/test-fixtures/v1-boundary/cases/v1-root-file/src/v1/main.ts new file mode 100644 index 0000000000..c8d21f3f94 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-root-file/src/v1/main.ts @@ -0,0 +1 @@ +export const main = 1; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-strict-null/src/v1/lib/length.ts b/frontend/test-fixtures/v1-boundary/cases/v1-strict-null/src/v1/lib/length.ts new file mode 100644 index 0000000000..b42e7ba01f --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-strict-null/src/v1/lib/length.ts @@ -0,0 +1 @@ +export const length = (value: string | undefined): number => value.length; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-class/src/v1/styles/global.css b/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-class/src/v1/styles/global.css new file mode 100644 index 0000000000..d077e5ea0f --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-class/src/v1/styles/global.css @@ -0,0 +1,3 @@ +.ant-btn { + color: red; +} diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-outside/src/v1/styles/global.css b/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-outside/src/v1/styles/global.css new file mode 100644 index 0000000000..e5971a61a2 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-outside/src/v1/styles/global.css @@ -0,0 +1 @@ +@import "../../index.css"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/components/ui/Link.test.ts b/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/components/ui/Link.test.ts new file mode 100644 index 0000000000..a84d47e627 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/components/ui/Link.test.ts @@ -0,0 +1,3 @@ +import { projectsUrl } from "@v1/services/projects"; + +export const url = projectsUrl; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/services/projects.ts b/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/services/projects.ts new file mode 100644 index 0000000000..934524c2f4 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-test-file-imports-services/src/v1/services/projects.ts @@ -0,0 +1 @@ +export const projectsUrl = "/api/v1/projects"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-unused-parameter/src/v1/lib/one.ts b/frontend/test-fixtures/v1-boundary/cases/v1-unused-parameter/src/v1/lib/one.ts new file mode 100644 index 0000000000..58bdb81c4e --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-unused-parameter/src/v1/lib/one.ts @@ -0,0 +1 @@ +export const one = (value: number): number => 1; diff --git a/frontend/vite.config.js b/frontend/vite.config.js index e9882c5d8f..fecc6b40f5 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -184,7 +184,8 @@ export default defineConfig(({ mode }) => { resolve: { alias: { "@": path.resolve(import.meta.dirname, "./src"), - // v1 frontend; mirrored in src/v1/tsconfig.json and vitest.config.mjs. + // v1 frontend. Also in src/v1/tsconfig.json and vitest.config.mjs; + // tsconfig.json/jsconfig.json map only the entry, for v0. "@v1": path.resolve(import.meta.dirname, "./src/v1"), }, }, diff --git a/frontend/vitest.config.mjs b/frontend/vitest.config.mjs index 915c23d11d..25cbe07f68 100644 --- a/frontend/vitest.config.mjs +++ b/frontend/vitest.config.mjs @@ -110,7 +110,7 @@ export default defineConfig({ * of this tree entirely, so vitest's default glob no longer reaches it. * `build/` is listed because a production build lands there and its * bundled chunks would otherwise be walked. `test-fixtures/` holds - * deliberately broken trees (some with `*.test.ts` files) that + * known-good and known-bad trees (some with `*.test.ts` files) that * test-fixtures/v1-boundary/boundary.fixtures.mjs lints and type-checks; * they are inputs, not tests. */ From 87c803c626230b6db9d5c23b710ef088e5898f6b Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Tue, 27 Oct 2026 10:15:00 +0530 Subject: [PATCH 03/26] feat(frontend): run v1 checks in CI and tighten what pages may import (UN-4270) - ci-frontend-lint: the typecheck job also runs `typecheck:v1` and `lint:v1`, so the strict v1 tsconfig and the scripts/check-v1.mjs checks (folder contract, TypeScript-only files, PropTypes, `ant-`) gate PRs now rather than from P0-T10. - Pages may no longer import app/, test/ or the v1 entry, only the shared folders (components, shell, services, stores, hooks, lib, styles, plugins). Test files in a page may still import test/. Contract table, README and the fixture matrix updated together. --- .github/workflows/ci-frontend-lint.yaml | 12 ++++++++++++ frontend/scripts/check-v1.mjs | 15 +++++++++++++-- frontend/src/v1/README.md | 2 +- .../v1-boundary/boundary.fixtures.mjs | 12 +++++++++++- .../src/v1/pages/projects/title.test.ts | 4 ++++ 5 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.test.ts diff --git a/.github/workflows/ci-frontend-lint.yaml b/.github/workflows/ci-frontend-lint.yaml index 1c87375a96..2b5fbac9c0 100644 --- a/.github/workflows/ci-frontend-lint.yaml +++ b/.github/workflows/ci-frontend-lint.yaml @@ -121,6 +121,18 @@ jobs: working-directory: frontend run: bun run typecheck + # src/v1 is excluded from the step above and checked under its own + # stricter tsconfig, after scripts/check-v1.mjs rejects non-TS files. + - name: Run v1 typecheck + working-directory: frontend + run: bun run typecheck:v1 + + # Biome on src/v1, then the folder import contract, PropTypes and `ant-` + # checks in scripts/check-v1.mjs, which `biome ci` cannot express. + - name: Run v1 lint + working-directory: frontend + run: bun run lint:v1 + # ci-test.yaml runs the same suite as the rig's `frontend` group, but its # tiers skip frontend-only PRs; this job is what gates those. vitest: diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index e5d6612ae9..1096abaf8e 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -33,7 +33,8 @@ const runCode = args.includes("--code") || !args.includes("--files"); const ALL = "*"; /* * Folder -> folders it may import from, besides itself. `entry` is - * src/v1/index.ts. Pages may import everything except other pages. + * src/v1/index.ts. Pages may not import other pages, app/, test/ (except + * from test files) or the entry. * Mirrors the table in src/v1/README.md; change both together. */ const CONTRACT = { @@ -41,7 +42,17 @@ const CONTRACT = { app: ALL, plugins: ALL, test: ALL, - pages: ALL, + pages: [ + "components/ui", + "components/patterns", + "shell", + "services", + "stores", + "hooks", + "lib", + "styles", + "plugins", + ], "components/ui": ["lib", "styles"], "components/patterns": [ "components/ui", diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 181b42bfaa..aeb7dfb32f 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -28,7 +28,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | `components/ui/` | Core components | `lib/`, `styles/` | | `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/` | | `shell/` | Top bar, rail, context bar, tab bar | `components/ui/`, `components/patterns/`, `services/`, `stores/`, `hooks/`, `lib/` | -| `pages//` | One folder per target page | everything except other pages | +| `pages//` | One folder per target page | `components/ui/`, `components/patterns/`, `shell/`, `services/`, `stores/`, `hooks/`, `lib/`, `styles/`, `plugins/` — not other pages, `app/`, `test/` or the entry | | `services/` | Typed service modules; the only place a URL appears | `lib/` | | `stores/` | Zustand stores | `lib/` | | `hooks/` | Shared hooks | `lib/` | diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index 26902335b9..1c7852a4b6 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -343,7 +343,17 @@ const MAY_IMPORT = { app: EVERYTHING, plugins: EVERYTHING, test: EVERYTHING, - "pages/a": EVERYTHING.filter((f) => f !== "pages/b"), + "pages/a": [ + "components/ui", + "components/patterns", + "shell", + "services", + "stores", + "hooks", + "lib", + "styles", + "plugins", + ], "components/ui": ["lib", "styles"], "components/patterns": [ "components/ui", diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.test.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.test.ts new file mode 100644 index 0000000000..73138391d0 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/pages/projects/title.test.ts @@ -0,0 +1,4 @@ +import { renderLabel } from "@v1/test/render"; +import { title } from "./title"; + +export const rendered = renderLabel(title); From f86de0b6e5d4b3cb14674d4ea3c188393e102fe8 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Tue, 27 Oct 2026 10:21:09 +0530 Subject: [PATCH 04/26] refactor(frontend): address SonarCloud findings on the v1 guardrails (UN-4270) check-v1.mjs: split the comment scanner into small helpers (cognitive complexity 40 -> under 15), drop the nested ternary and nested template literal, use String.raw for the import regexes, keep CLI flags in a Set. Fixture runner: - Cases and the expected folder contract are parsed text tables instead of repeated object literals (the 10% duplication). - Sorts use an explicit localeCompare comparator. - The contract matrix is built by a helper. - Spawns /bin/sh by absolute path and no longer needs `git init` (Biome only needs the ignore file). The git check-ignore call keeps a PATH lookup, with the reason and a NOSONAR marker. Fixture cases: re-exports rewritten as small functions and read-only props, without changing what each case exercises. --- frontend/scripts/check-v1.mjs | 118 ++++--- .../v1-boundary/boundary.fixtures.mjs | 312 ++++++------------ .../v0-imports-v1-internal/src/pages/Bad.jsx | 2 +- .../v0-imports-v1-relative/src/pages/Bad.jsx | 2 +- .../src/v1/components/ui/Tag.tsx | 2 +- .../src/v1/components/ui/Button.ts | 2 +- .../v1-imports-v0-alias/src/v1/lib/bad.ts | 2 +- .../v1-imports-v0-relative/src/v1/lib/bad.ts | 2 +- .../src/v1/pages/projects/index.ts | 2 +- .../src/v1/components/ui/ProjectLink.tsx | 2 +- 10 files changed, 180 insertions(+), 266 deletions(-) diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index 1096abaf8e..0866dec496 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -26,9 +26,9 @@ import fs from "node:fs"; import path from "node:path"; const ROOT = path.join(import.meta.dirname, "..", "src", "v1"); -const args = process.argv.slice(2); -const runFiles = args.includes("--files") || !args.includes("--code"); -const runCode = args.includes("--code") || !args.includes("--files"); +const args = new Set(process.argv.slice(2)); +const runFiles = args.has("--files") || !args.has("--code"); +const runCode = args.has("--code") || !args.has("--files"); const ALL = "*"; /* @@ -152,66 +152,86 @@ function blankComments(src) { // "`" = inside a template literal; "{" = inside a `${}` or a brace in one. const stack = []; while (i < src.length) { - const c = src[i]; - const next = src[i + 1]; + let end; if (stack.at(-1) === "`") { - if (c === "\\") { - out += src.slice(i, i + 2); - i += 2; - } else if (c === "`") { - stack.pop(); - out += c; - i += 1; - } else if (c === "$" && next === "{") { - stack.push("{"); - out += "${"; - i += 2; - } else { - out += c; - i += 1; - } - continue; - } - if (c === "/" && (next === "/" || next === "*")) { - const close = - next === "/" ? src.indexOf("\n", i) : src.indexOf("*/", i + 2); - const end = close === -1 ? src.length : next === "/" ? close : close + 2; + end = i + templateStep(src, i, stack); + } else if (src[i] === "/" && (src[i + 1] === "/" || src[i + 1] === "*")) { + end = commentEnd(src, i); out += src.slice(i, end).replace(/[^\n]/g, " "); i = end; - } else if (c === '"' || c === "'") { - let j = i + 1; - while (j < src.length && src[j] !== c && src[j] !== "\n") { - j += src[j] === "\\" ? 2 : 1; - } - out += src.slice(i, j + 1); - i = j + 1; + continue; + } else if (src[i] === '"' || src[i] === "'") { + end = stringEnd(src, i); } else { - if (c === "`") { - stack.push("`"); - } else if (c === "{" && stack.length > 0) { - stack.push("{"); - } else if (c === "}" && stack.length > 0) { - stack.pop(); - } - out += c; - i += 1; + codeStep(src[i], stack); + end = i + 1; } + out += src.slice(i, end); + i = end; } return out; } +/* Index just past the line or block comment that starts at i. */ +function commentEnd(src, i) { + if (src[i + 1] === "/") { + const newline = src.indexOf("\n", i); + return newline === -1 ? src.length : newline; + } + const close = src.indexOf("*/", i + 2); + return close === -1 ? src.length : close + 2; +} + +/* Index just past the '...' or "..." string that starts at i. An unclosed + * string ends at the newline. */ +function stringEnd(src, i) { + let j = i + 1; + while (j < src.length && src[j] !== src[i] && src[j] !== "\n") { + j += src[j] === "\\" ? 2 : 1; + } + return j + 1; +} + +/* One step inside a template literal: how many characters to copy. Closing + * the literal or opening `${` updates the stack. */ +function templateStep(src, i, stack) { + if (src[i] === "\\") { + return 2; + } + if (src[i] === "`") { + stack.pop(); + return 1; + } + if (src[i] === "$" && src[i + 1] === "{") { + stack.push("{"); + return 2; + } + return 1; +} + +/* Tracks template literals and the braces inside their `${}` expressions. */ +function codeStep(c, stack) { + if (c === "`") { + stack.push("`"); + } else if (c === "{" && stack.length > 0) { + stack.push("{"); + } else if (c === "}" && stack.length > 0) { + stack.pop(); + } +} + // A specifier: a quoted or backtick string with no interpolation. const SPEC = "[\"'`]([^\"'`$]+)[\"'`]"; const IMPORT_RES = [ new RegExp( - `\\b(?:import|export)\\s+(?:type\\s+)?(?:[\\w$*{}\\s,]+?\\s+from\\s+)?${SPEC}`, + String.raw`\b(?:import|export)\s+(?:type\s+)?(?:[\w$*{}\s,]+?\s+from\s+)?${SPEC}`, "g", ), - new RegExp(`\\bimport\\s*\\(\\s*${SPEC}\\s*[,)]`, "g"), - new RegExp(`\\brequire\\s*\\(\\s*${SPEC}\\s*\\)`, "g"), + new RegExp(String.raw`\bimport\s*\(\s*${SPEC}\s*[,)]`, "g"), + new RegExp(String.raw`\brequire\s*\(\s*${SPEC}\s*\)`, "g"), // Vite globs; the first pattern of an array is checked. new RegExp( - `\\bimport\\.meta\\.glob\\s*(?:<[^>]*>)?\\(\\s*\\[?\\s*${SPEC}`, + String.raw`\bimport\.meta\.glob\s*(?:<[^>]*>)?\(\s*\[?\s*${SPEC}`, "g", ), ]; @@ -267,8 +287,10 @@ function walk(dir) { } const problems = []; -const report = (rel, line, msg) => - problems.push(`src/v1/${rel}${line ? `:${line}` : ""}: ${msg}`); +const report = (rel, line, msg) => { + const where = line ? `${rel}:${line}` : rel; + problems.push(`src/v1/${where}: ${msg}`); +}; if (!fs.existsSync(ROOT)) { console.error(`check-v1: ${ROOT} does not exist`); diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index 1c7852a4b6..fb84fc072c 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -11,7 +11,7 @@ * pre-commit hook and typecheck never see them. */ import assert from "node:assert/strict"; -import { execFileSync, spawnSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -23,162 +23,69 @@ const { scripts } = JSON.parse( fs.readFileSync(path.join(FRONTEND, "package.json"), "utf8"), ); -/* - * `run` holds package.json script names, or raw commands for the v0 side, - * which has no v1 script. `fails` is a substring the tool output must - * contain; cases without it must pass. - */ -const CASES = [ - // The empty scaffold passes everything. - { name: null, run: ["typecheck:v1", "lint:v1"] }, - - // Import boundary, v1 -> v0 (Biome). - { - name: "v1-imports-v0-alias", - run: ["lint:v1"], - fails: "v1 imports nothing outside src/v1", - }, - { - name: "v1-imports-v0-relative", - run: ["lint:v1"], - fails: "v1 imports nothing outside src/v1", - }, - { - name: "v1-imports-root-absolute", - run: ["lint:v1"], - fails: "v1 imports nothing outside src/v1", - }, - - // Import boundary, v0 -> v1 (Biome). - { - name: "v0-imports-v1-internal", - run: ["biome lint src/"], - fails: "v0 may import only the v1 public entry", - }, - { - name: "v0-imports-v1-relative", - run: ["biome lint src/"], - fails: "v0 may import only the v1 public entry", - }, - { name: "v0-imports-v1-entry", run: ["biome lint src/"] }, - - // Banned under v1. - { name: "v1-prop-types", run: ["lint:v1"], fails: "PropTypes are not used" }, - { - name: "v1-prop-types-assignment", - run: ["lint:v1"], - fails: "No PropTypes under v1", - }, - { - name: "v1-antd-import", - run: ["lint:v1"], - fails: "antd is not used under src/v1", - }, - { - name: "v1-antd-subpath", - run: ["lint:v1"], - fails: "antd is not used under src/v1", - }, - { - name: "v1-prop-types-subpath", - run: ["lint:v1"], - fails: "PropTypes are not used", - }, - { - name: "v1-styles-import-outside", - run: ["lint:v1"], - fails: "v1 styles import nothing outside it", - }, - { - name: "v1-styles-ant-class", - run: ["lint:v1"], - fails: "No antd 'ant-' class names", - }, - { - name: "v1-component-css-import", - run: ["lint:v1"], - fails: "No component .css under src/v1", - }, - { - name: "v1-component-css-file", - run: ["typecheck:v1"], - fails: "No component style files", - }, - { - name: "v1-ant-class-prefix", - run: ["lint:v1"], - fails: "No antd 'ant-' class names", - }, - { - name: "v1-ant-built-class", - run: ["lint:v1"], - fails: "No antd 'ant-' class names", - }, - { name: "v1-explicit-any", run: ["lint:v1"], fails: "noExplicitAny" }, - // The v1 override must add to the shared rules, not replace them. - { - name: "v1-base-rules-still-apply", - run: ["lint:v1"], - fails: "noDoubleEquals", - }, +/* Rows of "a | b | c" cells; blank lines and "#" comments are skipped. */ +function parseTable(text) { + return text + .split("\n") + .map((line) => line.trim()) + .filter((line) => line && !line.startsWith("#")) + .map((line) => line.split("|").map((cell) => cell.trim())); +} - // TypeScript only, strict. - { name: "v1-js-file", run: ["typecheck:v1"], fails: "TypeScript only" }, - { - name: "v1-implicit-any", - run: ["typecheck:v1"], - fails: "TS7006", - }, - { - name: "v1-strict-null", - run: ["typecheck:v1"], - fails: "TS18048", - }, - { - name: "v1-unused-local", - run: ["typecheck:v1"], - fails: "TS6133", - }, - { - name: "v1-unused-parameter", - run: ["typecheck:v1"], - fails: "TS6133", - }, +const sorted = (items) => items.toSorted((a, b) => a.localeCompare(b)); - // Folder taxonomy and contract (the full contract is the matrix below). - { - name: "v1-outside-taxonomy", - run: ["typecheck:v1"], - fails: "Outside the v1 folder taxonomy", - }, - { - name: "v1-root-file", - run: ["typecheck:v1"], - fails: "Outside the v1 folder taxonomy", - }, - { - name: "v1-ui-imports-services", - run: ["lint:v1"], - fails: "components/ui/ may not import from services/", - }, - { - name: "v1-page-imports-other-page", - run: ["lint:v1"], - fails: "pages/projects/ may not import from pages/settings/", - }, - // Only test files may use test/, and being a test file allows nothing else. - { - name: "v1-non-test-imports-test", - run: ["lint:v1"], - fails: "components/ui/ may not import from test/", - }, - { - name: "v1-test-file-imports-services", - run: ["lint:v1"], - fails: "components/ui/ may not import from services/", - }, - { name: "v1-allowed-imports", run: ["typecheck:v1", "lint:v1"] }, -]; +/* + * One case per row: the folder under cases/ ("-" for the empty scaffold); + * the steps, joined by "+", each a package.json script name or a raw command + * (the v0 side has no v1 script); and a substring the tool output must + * contain, or "-" when the steps must pass. + */ +const CASES = parseTable(` + # The empty scaffold passes everything. + - | typecheck:v1 + lint:v1 | - + # Import boundary, v1 -> v0 (Biome). + v1-imports-v0-alias | lint:v1 | v1 imports nothing outside src/v1 + v1-imports-v0-relative | lint:v1 | v1 imports nothing outside src/v1 + v1-imports-root-absolute | lint:v1 | v1 imports nothing outside src/v1 + # Import boundary, v0 -> v1 (Biome). + v0-imports-v1-internal | biome lint src/ | v0 may import only the v1 public entry + v0-imports-v1-relative | biome lint src/ | v0 may import only the v1 public entry + v0-imports-v1-entry | biome lint src/ | - + # Banned under v1. + v1-prop-types | lint:v1 | PropTypes are not used + v1-prop-types-subpath | lint:v1 | PropTypes are not used + v1-prop-types-assignment | lint:v1 | No PropTypes under v1 + v1-antd-import | lint:v1 | antd is not used under src/v1 + v1-antd-subpath | lint:v1 | antd is not used under src/v1 + v1-styles-import-outside | lint:v1 | v1 styles import nothing outside it + v1-styles-ant-class | lint:v1 | No antd 'ant-' class names + v1-component-css-import | lint:v1 | No component .css under src/v1 + v1-component-css-file | typecheck:v1 | No component style files + v1-ant-class-prefix | lint:v1 | No antd 'ant-' class names + v1-ant-built-class | lint:v1 | No antd 'ant-' class names + v1-explicit-any | lint:v1 | noExplicitAny + # The v1 override must add to the shared rules, not replace them. + v1-base-rules-still-apply | lint:v1 | noDoubleEquals + # TypeScript only, strict. + v1-js-file | typecheck:v1 | TypeScript only + v1-implicit-any | typecheck:v1 | TS7006 + v1-strict-null | typecheck:v1 | TS18048 + v1-unused-local | typecheck:v1 | TS6133 + v1-unused-parameter | typecheck:v1 | TS6133 + # Folder taxonomy and contract (the full contract is the matrix below). + v1-outside-taxonomy | typecheck:v1 | Outside the v1 folder taxonomy + v1-root-file | typecheck:v1 | Outside the v1 folder taxonomy + v1-ui-imports-services | lint:v1 | components/ui/ may not import from services/ + v1-page-imports-other-page | lint:v1 | pages/projects/ may not import from pages/settings/ + # Only test files may use test/, and being a test file allows nothing else. + v1-non-test-imports-test | lint:v1 | components/ui/ may not import from test/ + v1-test-file-imports-services | lint:v1 | components/ui/ may not import from services/ + v1-allowed-imports | typecheck:v1 + lint:v1 | - +`).map(([name, run, fails]) => ({ + name: name === "-" ? null : name, + run: run.split(" + "), + fails: fails === "-" ? null : fails, +})); function makeTree(caseName) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "v1-boundary-")); @@ -200,9 +107,8 @@ function makeTree(caseName) { if (caseName) { fs.cpSync(path.join(CASES_DIR, caseName), dir, { recursive: true }); } - // biome.json enables VCS integration with `useIgnoreFile`, which needs a - // repository root holding an ignore file. - execFileSync("git", ["init", "-q"], { cwd: dir }); + // biome.json enables VCS integration with `useIgnoreFile`, which needs an + // ignore file at the root. fs.writeFileSync(path.join(dir, ".gitignore"), "node_modules\n"); return dir; } @@ -229,7 +135,7 @@ function runAll(dir, run) { let log = ""; for (const step of run) { const command = scripts[step] ?? step; - const result = spawnSync("sh", ["-c", command], { + const result = spawnSync("/bin/sh", ["-c", command], { cwd: dir, encoding: "utf8", env: { @@ -285,18 +191,18 @@ test("no v1 or fixture file is git-ignored", () => { ); // A lib/ folder anywhere under v1, not only the top-level one. files.push("src/v1/pages/projects/lib/x.ts", "src/v1/lib/sub/lib/x.ts"); - const result = spawnSync("git", ["check-ignore", "--no-index", "--stdin"], { - cwd: FRONTEND, - encoding: "utf8", - input: files.join("\n"), - }); + const gitArgs = ["check-ignore", "--no-index", "--stdin"]; + const options = { cwd: FRONTEND, encoding: "utf8", input: files.join("\n") }; + // git from PATH on purpose: this is a developer/CI test, and git has no + // fixed location across macOS and Linux. + const result = spawnSync("git", gitArgs, options); // NOSONAR // Exit 1 means "nothing matched"; 0 means some file is ignored. assert.equal(result.status, 1, `git-ignored:\n${result.stdout}`); }); test("every case folder is listed", () => { const listed = new Set(CASES.map((c) => c.name).filter(Boolean)); - assert.deepEqual(fs.readdirSync(CASES_DIR).sort(), [...listed].sort()); + assert.deepEqual(sorted(fs.readdirSync(CASES_DIR)), sorted([...listed])); }); for (const { name, run, fails } of CASES) { @@ -339,65 +245,51 @@ const FOLDERS = { test: "test", }; const EVERYTHING = Object.keys(FOLDERS); -const MAY_IMPORT = { - app: EVERYTHING, - plugins: EVERYTHING, - test: EVERYTHING, - "pages/a": [ - "components/ui", - "components/patterns", - "shell", - "services", - "stores", - "hooks", - "lib", - "styles", - "plugins", - ], - "components/ui": ["lib", "styles"], - "components/patterns": [ - "components/ui", - "lib", - "services", - "stores", - "hooks", - ], - shell: [ - "components/ui", - "components/patterns", - "services", - "stores", - "hooks", - "lib", - ], - services: ["lib"], - stores: ["lib"], - hooks: ["lib"], - lib: [], - styles: ["lib"], -}; +const MAY_IMPORT = Object.fromEntries( + parseTable(` + app | * + plugins | * + test | * + pages/a | components/ui components/patterns shell services stores hooks lib styles plugins + components/ui | lib styles + components/patterns | components/ui lib services stores hooks + shell | components/ui components/patterns services stores hooks lib + services | lib + stores | lib + hooks | lib + lib | + styles | lib + `).map(([from, to]) => [from, to === "*" ? EVERYTHING : to.split(/\s+/)]), +); -test("folder contract: every folder pair, by file and by folder", () => { +/* One file per folder pair, importing the target by file and by folder; and + * the problems check-v1 must report for them. */ +function contractMatrix() { const files = {}; const expected = []; for (const [from, allowed] of Object.entries(MAY_IMPORT)) { for (const to of EVERYTHING) { const rel = `src/v1/${FOLDERS[from]}/m-${to.replace("/", "-")}.ts`; // The entry is a file, so it has only the one form. + const target = FOLDERS[to]; files[rel] = to === "entry" ? 'import "@v1";\n' - : `import "@v1/${FOLDERS[to]}/target";\nimport "@v1/${FOLDERS[to]}";\n`; - if (from !== to && !allowed.includes(to)) { - const lines = to === "entry" ? [1] : [1, 2]; - for (const line of lines) { - expected.push(`${rel}:${line}: ${from}/ may not import from ${to}/`); - } + : `import "@v1/${target}/target";\nimport "@v1/${target}";\n`; + const denied = from !== to && !allowed.includes(to); + const lines = to === "entry" ? [1] : [1, 2]; + for (const line of denied ? lines : []) { + expected.push(`${rel}:${line}: ${from}/ may not import from ${to}/`); } } } + return { files, expected }; +} + +test("folder contract: every folder pair, by file and by folder", () => { + const { files, expected } = contractMatrix(); const reported = checkCode(files).map((l) => l.replace(/ \(.*$/, "")); - assert.deepEqual(reported.sort(), expected.sort()); + assert.deepEqual(sorted(reported), sorted(expected)); }); test("folder contract: every import form is read", () => { diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx index d822a2d078..ecddfc6910 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-internal/src/pages/Bad.jsx @@ -1,3 +1,3 @@ import { pluginManifest } from "@v1/plugins/manifest"; -export const manifest = pluginManifest; +export const manifestSize = () => pluginManifest.length; diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx index d462e4d68a..953cb550c6 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-relative/src/pages/Bad.jsx @@ -1,3 +1,3 @@ import { pluginManifest } from "../v1/plugins/manifest"; -export const manifest = pluginManifest; +export const manifestSize = () => pluginManifest.length; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx index f00b602b5d..07afb25230 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Tag.tsx @@ -1,6 +1,6 @@ import { upper } from "@v1/lib/format"; -type TagProps = { label: string }; +type TagProps = Readonly<{ label: string }>; export function Tag({ label }: TagProps) { return {upper(label)}; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts b/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts index fda637b2e3..1973bc6c11 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts +++ b/frontend/test-fixtures/v1-boundary/cases/v1-antd-import/src/v1/components/ui/Button.ts @@ -1,3 +1,3 @@ import { Button } from "antd"; -export const AntButton = Button; +export const buttonName = Button.name; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts index b6e5854b2b..4f64a2f1e2 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts +++ b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-alias/src/v1/lib/bad.ts @@ -1,3 +1,3 @@ import { getBaseUrl } from "@/helpers/GetStaticData"; -export const base = getBaseUrl; +export const base = () => getBaseUrl(); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts index d2c250ae68..acba160b3c 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts +++ b/frontend/test-fixtures/v1-boundary/cases/v1-imports-v0-relative/src/v1/lib/bad.ts @@ -1,3 +1,3 @@ import { getBaseUrl } from "../../helpers/GetStaticData"; -export const base = getBaseUrl; +export const base = () => getBaseUrl(); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts index 1a075e1cc3..f6e2ea9482 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts +++ b/frontend/test-fixtures/v1-boundary/cases/v1-page-imports-other-page/src/v1/pages/projects/index.ts @@ -1,3 +1,3 @@ import { settingsTitle } from "@v1/pages/settings"; -export const title = settingsTitle; +export const title = `Projects · ${settingsTitle}`; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx index 3863836b20..8024a7c0bb 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx +++ b/frontend/test-fixtures/v1-boundary/cases/v1-ui-imports-services/src/v1/components/ui/ProjectLink.tsx @@ -1,3 +1,3 @@ import { projectsUrl } from "@v1/services/projects"; -export const href = projectsUrl; +export const href = `${projectsUrl}/`; From 67be34b11e113804f7b1e06ddcedc47e874c26c6 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Tue, 27 Oct 2026 10:22:45 +0530 Subject: [PATCH 05/26] refactor(frontend): split the contract-matrix builder to cut its complexity (UN-4270) --- .../v1-boundary/boundary.fixtures.mjs | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index fb84fc072c..6b2aeaa56d 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -262,25 +262,30 @@ const MAY_IMPORT = Object.fromEntries( `).map(([from, to]) => [from, to === "*" ? EVERYTHING : to.split(/\s+/)]), ); -/* One file per folder pair, importing the target by file and by folder; and - * the problems check-v1 must report for them. */ +/* The file for one folder pair: it imports the target by file and by folder. + * The entry is a file, so it has only the one form. */ +function pairFile(to) { + const target = FOLDERS[to]; + return to === "entry" + ? ['import "@v1";'] + : [`import "@v1/${target}/target";`, `import "@v1/${target}";`]; +} + +/* One file per folder pair, and the problems check-v1 must report for them. */ function contractMatrix() { const files = {}; const expected = []; for (const [from, allowed] of Object.entries(MAY_IMPORT)) { for (const to of EVERYTHING) { const rel = `src/v1/${FOLDERS[from]}/m-${to.replace("/", "-")}.ts`; - // The entry is a file, so it has only the one form. - const target = FOLDERS[to]; - files[rel] = - to === "entry" - ? 'import "@v1";\n' - : `import "@v1/${target}/target";\nimport "@v1/${target}";\n`; + const imports = pairFile(to); + files[rel] = `${imports.join("\n")}\n`; const denied = from !== to && !allowed.includes(to); - const lines = to === "entry" ? [1] : [1, 2]; - for (const line of denied ? lines : []) { - expected.push(`${rel}:${line}: ${from}/ may not import from ${to}/`); - } + expected.push( + ...(denied ? imports : []).map( + (_, i) => `${rel}:${i + 1}: ${from}/ may not import from ${to}/`, + ), + ); } } return { files, expected }; From 81e15b1597413bac74ae4f4a4e476577326fc836 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Tue, 27 Oct 2026 12:55:55 +0530 Subject: [PATCH 06/26] fix(frontend): check every glob pattern and v0 -> v1 imports Biome cannot see (UN-4270) Addresses Greptile review on #2380: - import.meta.glob([...]): every pattern in the array is checked, not only the first, so an allowed first pattern no longer hides a forbidden one. - check-v1 --code also scans v0 (src/ outside v1, including the cloud src/plugins overlay) for template-literal imports, globs, require(), @/v1, src/v1, relative paths and CSS @import that reach into v1 other than through `@v1`. Biome still covers plain imports. Fixtures: Greptile's example as a case, a glob array with a forbidden later pattern, and a generated test of every v0 -> v1 form (denied and allowed). The v0 scan reports nothing on the current tree. --- frontend/scripts/check-v1.mjs | 137 +++++++++++++----- frontend/src/v1/README.md | 2 +- .../v1-boundary/boundary.fixtures.mjs | 43 +++++- .../v0-imports-v1-template/src/pages/Bad.jsx | 1 + 4 files changed, 148 insertions(+), 35 deletions(-) create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-template/src/pages/Bad.jsx diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index 0866dec496..3e842480c4 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -7,15 +7,17 @@ * file inside the v1 folder taxonomy. Run by `typecheck:v1`. * --code Imports: nothing outside src/v1 and the folder import contract * (which v1 folder may import which). No PropTypes, no antd `ant-` - * class names. Run by `lint:v1`. + * class names. Also scans v0 (the rest of src/) for imports that + * reach into v1 other than through `@v1`. Run by `lint:v1`. * * With neither flag, both run. It always checks the src/v1 beside this * script; the fixtures in test-fixtures/v1-boundary/ copy it into each * throwaway tree for that reason. * * The v0 <-> v1 boundary is also a Biome rule (biome.json `overrides`), so CI - * and editors report it. The script repeats the v1 -> outside half because it - * also reads template-literal imports, which Biome does not. Biome overrides + * and editors report it. The script repeats both halves because it also reads + * template-literal imports, Vite globs, require() and CSS @import, which + * Biome does not. Biome overrides * replace rule options rather than merge them, which is why the per-folder * contract lives here and not as one override per folder. * @@ -25,7 +27,8 @@ import fs from "node:fs"; import path from "node:path"; -const ROOT = path.join(import.meta.dirname, "..", "src", "v1"); +const SRC = path.join(import.meta.dirname, "..", "src"); +const ROOT = path.join(SRC, "v1"); const args = new Set(process.argv.slice(2)); const runFiles = args.has("--files") || !args.has("--code"); const runCode = args.has("--code") || !args.has("--files"); @@ -229,12 +232,28 @@ const IMPORT_RES = [ ), new RegExp(String.raw`\bimport\s*\(\s*${SPEC}\s*[,)]`, "g"), new RegExp(String.raw`\brequire\s*\(\s*${SPEC}\s*\)`, "g"), - // Vite globs; the first pattern of an array is checked. - new RegExp( - String.raw`\bimport\.meta\.glob\s*(?:<[^>]*>)?\(\s*\[?\s*${SPEC}`, - "g", - ), ]; +// Vite globs: one pattern or an array of them. Every pattern is checked. +const GLOB_RE = new RegExp( + String.raw`\bimport\.meta\.glob\s*(?:<[^>]*>)?\(\s*(\[[^\]]*\]|${SPEC})`, + "g", +); +const SPEC_RE = new RegExp(SPEC, "g"); + +/* Every import specifier in comment-blanked source, with its offset. */ +function* importSpecs(text) { + for (const re of IMPORT_RES) { + for (const m of text.matchAll(re)) { + yield { spec: m[1], index: m.index }; + } + } + for (const m of text.matchAll(GLOB_RE)) { + const start = m.index + m[0].indexOf(m[1]); + for (const s of m[1].matchAll(SPEC_RE)) { + yield { spec: s[1], index: start + s.index }; + } + } +} // CSS `@import` and `url()` targets, for the files in styles/. const CSS_IMPORT_RE = /(?:@import\s+(?:url\(\s*)?|url\(\s*)["']?([^"')\s;]+)/g; @@ -280,10 +299,10 @@ function walk(dir) { .filter((d) => (d.isFile() || d.isSymbolicLink()) && !IGNORED.has(d.name)) .map((d) => path - .relative(ROOT, path.join(d.parentPath, d.name)) + .relative(dir, path.join(d.parentPath, d.name)) .replaceAll(path.sep, "/"), ) - .sort(); + .toSorted((a, b) => a.localeCompare(b)); } const problems = []; @@ -347,28 +366,26 @@ for (const rel of walk(ROOT)) { const text = blankComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); const isTestFile = TEST_FILE.test(name); - for (const re of IMPORT_RES) { - for (const m of text.matchAll(re)) { - const to = targetFolder(m[1], rel); - const line = lineAt(text, m.index); - if (to === undefined) { - continue; - } - if (to === "outside") { - report( - rel, - line, - `'${m[1]}' is outside src/v1; v1 imports nothing outside it.`, - ); - } else if (to === null) { - report(rel, line, `'${m[1]}' is outside the v1 folder taxonomy.`); - } else if (!mayImport(folder, to, isTestFile)) { - report( - rel, - line, - `${folder}/ may not import from ${to}/ ('${m[1]}'); see the folder contract in src/v1/README.md.`, - ); - } + for (const { spec, index } of importSpecs(text)) { + const to = targetFolder(spec, rel); + const line = lineAt(text, index); + if (to === undefined) { + continue; + } + if (to === "outside") { + report( + rel, + line, + `'${spec}' is outside src/v1; v1 imports nothing outside it.`, + ); + } else if (to === null) { + report(rel, line, `'${spec}' is outside the v1 folder taxonomy.`); + } else if (!mayImport(folder, to, isTestFile)) { + report( + rel, + line, + `${folder}/ may not import from ${to}/ ('${spec}'); see the folder contract in src/v1/README.md.`, + ); } } for (const m of text.matchAll(/\.propTypes\b|\bpropTypes\s*[:=]/g)) { @@ -384,6 +401,60 @@ for (const rel of walk(ROOT)) { } } +/* + * The v0 -> v1 half: v0 (everything in src/ outside v1, including the cloud + * src/plugins/ overlay) reaches v1 only through the `@v1` entry. + */ +const V0_CODE_EXT = /\.(js|jsx|mjs|cjs|ts|tsx|mts|cts)$/; + +/* Whether a v0 specifier (fileRel is relative to src/) reaches into v1. */ +function intoV1(spec, fileRel) { + if (spec === "@v1" || /^@v1\/index(\.ts)?$/.test(spec)) { + return false; + } + if (spec.startsWith("@v1/") || /^(@\/|src\/)v1(\/|$)/.test(spec)) { + return true; + } + if (!spec.startsWith(".")) { + return false; + } + const rel = path.posix.join(path.posix.dirname(fileRel), spec); + return rel === "v1" || rel.startsWith("v1/"); +} + +function v0Specs(rel, text) { + if (!STYLE_EXT.test(rel)) { + return importSpecs(text); + } + return [...text.matchAll(CSS_IMPORT_RE)].map((m) => ({ + spec: m[1], + index: m.index, + })); +} + +function checkV0() { + for (const rel of walk(SRC)) { + const full = path.join(SRC, rel); + const inV1 = rel.startsWith("v1/"); + const scanned = STYLE_EXT.test(rel) || V0_CODE_EXT.test(rel); + if (inV1 || !scanned || fs.lstatSync(full).isSymbolicLink()) { + continue; + } + const text = blankComments(fs.readFileSync(full, "utf8")); + for (const { spec, index } of v0Specs(rel, text)) { + if (intoV1(spec, rel)) { + problems.push( + `src/${rel}:${lineAt(text, index)}: '${spec}' reaches into src/v1; v0 may import only the v1 public entry, as '@v1'.`, + ); + } + } + } +} + +if (runCode) { + checkV0(); +} + if (problems.length > 0) { console.error(problems.join("\n")); console.error(`\ncheck-v1: ${problems.length} problem(s).`); diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index aeb7dfb32f..61fa459ac3 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -42,7 +42,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | Script | Checks | | --- | --- | | `bun run typecheck:v1` | TypeScript-only files, CSS only in `styles/`, no symlinks, folder taxonomy; then `tsc` with `src/v1/tsconfig.json` | -| `bun run lint:v1` | Biome on `src/v1/`, then imports (nothing outside v1, folder contract), PropTypes and `ant-` checks | +| `bun run lint:v1` | Biome on `src/v1/`, then imports (nothing outside v1, folder contract, and v0 reaching v1 only through `@v1`), PropTypes and `ant-` checks | | `bun run test:v1` | v1 unit tests, then the guardrail fixtures in `test-fixtures/v1-boundary/` | The v0 ↔ v1 boundary and the banned imports are Biome rules in `biome.json`, diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index 6b2aeaa56d..c530319d89 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -50,6 +50,7 @@ const CASES = parseTable(` # Import boundary, v0 -> v1 (Biome). v0-imports-v1-internal | biome lint src/ | v0 may import only the v1 public entry v0-imports-v1-relative | biome lint src/ | v0 may import only the v1 public entry + v0-imports-v1-template | lint:v1 | v0 may import only the v1 public entry v0-imports-v1-entry | biome lint src/ | - # Banned under v1. v1-prop-types | lint:v1 | PropTypes are not used @@ -168,7 +169,7 @@ function checkCode(files) { writeFile(dir, rel, content); } const { output } = runAll(dir, ["node scripts/check-v1.mjs --code"]); - return output.split("\n").filter((l) => l.startsWith("src/v1/")); + return output.split("\n").filter((l) => l.startsWith("src/")); } finally { removeTree(dir); } @@ -320,6 +321,8 @@ test("folder contract: every import form is read", () => { afterDoubleSlashInString: 'const u = "a//b"; export { a } from "@v1/services/a";', glob: 'export const a = import.meta.glob("@v1/services/*.ts");', + globArrayLater: + 'export const a = import.meta.glob(["@v1/components/ui/*.tsx", "@v1/services/*.ts"]);', afterTemplateExpression: // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. 'const t = `${"}"}/*`;\nimport { a } from "@v1/services/a";\n/** doc */', @@ -379,3 +382,41 @@ test("v1 imports nothing outside src/v1, in any spelling", () => { ); }); }); + +test("v0 reaches v1 only through @v1, in any form", () => { + const denied = { + "static.jsx": 'import x from "@v1/lib/x";', + "template.jsx": "export const a = () => import(`@v1/plugins/manifest`);", + "glob.jsx": 'export const a = import.meta.glob("@v1/pages/*/index.ts");', + "globArrayLater.jsx": + 'export const a = import.meta.glob(["./*.jsx", "../v1/lib/*.ts"]);', + "require.jsx": 'export const a = require("../v1/lib/x");', + "aliasV0.jsx": 'export const a = () => import("@/v1/lib/x");', + "srcPath.jsx": 'export const a = () => import("src/v1/lib/x");', + "relativeEntry.jsx": 'import "../v1";', + "styles.css": '@import "../v1/styles/tokens.css";', + }; + const allowed = { + "entry.jsx": 'import "@v1";', + "entryTemplate.jsx": "export const a = () => import(`@v1`);", + "v0Only.jsx": 'import x from "../helpers/x";\nexport const y = x;', + }; + const files = {}; + for (const [name, code] of Object.entries({ ...denied, ...allowed })) { + files[`src/pages/${name}`] = `${code}\n`; + } + const reported = checkCode(files); + const v0Lines = reported.filter((l) => l.startsWith("src/pages/")); + for (const name of Object.keys(denied)) { + assert.ok( + v0Lines.some((l) => l.startsWith(`src/pages/${name}:`)), + `${name}: not reported\n${reported.join("\n")}`, + ); + } + for (const name of Object.keys(allowed)) { + assert.ok( + !v0Lines.some((l) => l.startsWith(`src/pages/${name}:`)), + `${name}: reported but allowed\n${reported.join("\n")}`, + ); + } +}); diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-template/src/pages/Bad.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-template/src/pages/Bad.jsx new file mode 100644 index 0000000000..278db55314 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-template/src/pages/Bad.jsx @@ -0,0 +1 @@ +export const loadManifest = () => import(`@v1/plugins/manifest`); From 7e66cc59c633e493d07eab8a1e96c96beef76159 Mon Sep 17 00:00:00 2001 From: Athul Date: Sat, 24 Oct 2026 19:54:37 +0530 Subject: [PATCH 07/26] UN-4286 [FEAT] Add the v1 plugin manifest, slots and build check v1 finds plugins at build time from src/v1/plugins//manifest.ts, which the cloud overlay copies in. An absent plugin has no manifest, so its slots show their fallbacks with no error; any other load or render error is reported, and each slot has its own error boundary. - lib/plugins: the typed slot contract, buildRegistry (distribution filter plus conflict checks for every distribution), and pluginRoutes(). VITE_DISTRIBUTION picks oss, cloud or onprem. - scripts/v1-plugins.mjs runs in every Vite build: a slot filled twice, a route defined twice or a plugin with no distribution fails the build, and the distribution matrix is published as plugin-matrix.json. - v0's optional-plugin stub no longer applies under src/v1, where it turned a missing module into an empty object instead of an error. Co-Authored-By: Claude Opus 5.5 --- frontend/package.json | 2 +- frontend/scripts/v1-plugins.fixtures.mjs | 89 ++++++++++++++ frontend/scripts/v1-plugins.mjs | 85 +++++++++++++ frontend/src/v1/README.md | 11 ++ frontend/src/v1/lib/plugins/Slot.test.tsx | 105 ++++++++++++++++ frontend/src/v1/lib/plugins/Slot.tsx | 87 +++++++++++++ frontend/src/v1/lib/plugins/index.ts | 45 +++++++ .../src/v1/lib/plugins/missing-import.test.ts | 13 ++ frontend/src/v1/lib/plugins/registry.test.ts | 97 +++++++++++++++ frontend/src/v1/lib/plugins/registry.ts | 115 ++++++++++++++++++ frontend/src/v1/lib/plugins/types.ts | 45 +++++++ frontend/src/v1/plugins/.gitkeep | 0 frontend/src/v1/plugins/manifest.ts | 6 - frontend/src/v1/test/jest-dom.d.ts | 2 + frontend/vite.config.js | 28 +++++ frontend/vitest.config.mjs | 7 +- 16 files changed, 729 insertions(+), 8 deletions(-) create mode 100644 frontend/scripts/v1-plugins.fixtures.mjs create mode 100755 frontend/scripts/v1-plugins.mjs create mode 100644 frontend/src/v1/lib/plugins/Slot.test.tsx create mode 100644 frontend/src/v1/lib/plugins/Slot.tsx create mode 100644 frontend/src/v1/lib/plugins/index.ts create mode 100644 frontend/src/v1/lib/plugins/missing-import.test.ts create mode 100644 frontend/src/v1/lib/plugins/registry.test.ts create mode 100644 frontend/src/v1/lib/plugins/registry.ts create mode 100644 frontend/src/v1/lib/plugins/types.ts create mode 100644 frontend/src/v1/plugins/.gitkeep delete mode 100644 frontend/src/v1/plugins/manifest.ts create mode 100644 frontend/src/v1/test/jest-dom.d.ts diff --git a/frontend/package.json b/frontend/package.json index 3d7f579cc9..cc01b5c9db 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -80,7 +80,7 @@ "typecheck": "tsc --noEmit", "typecheck:v1": "node scripts/check-v1.mjs --files && tsc -p src/v1/tsconfig.json", "lint:v1": "biome lint src/v1/ && node scripts/check-v1.mjs --code", - "test:v1": "vitest run --dir src/v1 --passWithNoTests && node --test test-fixtures/v1-boundary/boundary.fixtures.mjs" + "test:v1": "vitest run --dir src/v1 --passWithNoTests && node --test test-fixtures/v1-boundary/boundary.fixtures.mjs scripts/v1-plugins.fixtures.mjs" }, "browserslist": { "production": [ diff --git a/frontend/scripts/v1-plugins.fixtures.mjs b/frontend/scripts/v1-plugins.fixtures.mjs new file mode 100644 index 0000000000..4261a2d196 --- /dev/null +++ b/frontend/scripts/v1-plugins.fixtures.mjs @@ -0,0 +1,89 @@ +// Fixtures for v1-plugins.mjs. Run with `node --test` (bun run test:v1). +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { test } from "node:test"; +import { check } from "./v1-plugins.mjs"; + +function plugins(manifests) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "v1-plugins-")); + for (const [id, body] of Object.entries(manifests)) { + fs.mkdirSync(path.join(dir, id)); + fs.writeFileSync(path.join(dir, id, "manifest.ts"), body); + } + return dir; +} + +const manifest = (id, distributions, extra = "") => ` +import type { PluginManifest } from "@v1/lib/plugins/types"; +export const manifest = { + id: "${id}", + distributions: ${JSON.stringify(distributions)}, + ${extra} +} satisfies PluginManifest; +`; + +test("the matrix lists each plugin per distribution", async () => { + const dir = plugins({ + billing: manifest( + "billing", + ["cloud"], + `slots: { "account-menu.plan-billing": () => import("./Billing") }, + routes: [{ path: "/billing", load: () => import("./Billing") }],`, + ), + switcher: manifest("switcher", ["cloud", "onprem"]), + }); + assert.deepEqual(await check(dir), { + distributions: { + oss: [], + cloud: ["billing", "switcher"], + onprem: ["switcher"], + }, + plugins: [ + { + id: "billing", + distributions: ["cloud"], + slots: ["account-menu.plan-billing"], + routes: ["/billing"], + }, + { + id: "switcher", + distributions: ["cloud", "onprem"], + slots: [], + routes: [], + }, + ], + }); +}); + +test("no plugins directory gives an empty matrix", async () => { + assert.deepEqual(await check(path.join(os.tmpdir(), "v1-plugins-none")), { + distributions: { oss: [], cloud: [], onprem: [] }, + plugins: [], + }); +}); + +test("a slot filled twice fails, naming both plugins", async () => { + const slot = `slots: { "shell.product-switcher": () => import("./X") },`; + const dir = plugins({ + a: manifest("a", ["cloud"], slot), + b: manifest("b", ["cloud"], slot), + }); + await assert.rejects(check(dir), /shell\.product-switcher.*\ba\b.*\bb\b/); +}); + +test("a manifest whose id differs from its folder fails", async () => { + const dir = plugins({ billing: manifest("payments", ["cloud"]) }); + await assert.rejects( + check(dir), + /billing\/manifest\.ts declares id payments/, + ); +}); + +test("a manifest with a runtime import fails with a clear message", async () => { + const dir = plugins({ + bad: `import { x } from "@v1/lib/plugins/registry";\nexport const manifest = x;\n`, + }); + await assert.rejects(check(dir), /bad\/manifest\.ts.*import type/); +}); diff --git a/frontend/scripts/v1-plugins.mjs b/frontend/scripts/v1-plugins.mjs new file mode 100755 index 0000000000..ff6559cb22 --- /dev/null +++ b/frontend/scripts/v1-plugins.mjs @@ -0,0 +1,85 @@ +#!/usr/bin/env node +/* + * Build check for v1 plugins (src/v1/plugins//manifest.ts, copied in by + * the cloud overlay). Loads every manifest with Node or Bun type stripping, + * runs the registry's conflict checks for every distribution, and returns the + * distribution matrix. vite.config.js runs it in every build and publishes + * the matrix as plugin-matrix.json. + * + * node scripts/v1-plugins.mjs [out.json] check, print or write the matrix + */ +import fs from "node:fs"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { buildRegistry } from "../src/v1/lib/plugins/registry.ts"; + +const DEFAULT_DIR = path.join(import.meta.dirname, "../src/v1/plugins"); + +async function load(dir, id) { + const file = path.join(dir, id, "manifest.ts"); + const rel = `${id}/manifest.ts`; + let mod; + try { + mod = await import(pathToFileURL(file).href); + } catch (err) { + throw new Error( + `${rel} could not be loaded (${err.message}). A manifest may only ` + + '`import type`; load components with `() => import("./X")`.', + ); + } + const { manifest } = mod; + if (!manifest) { + throw new Error(`${rel} does not export \`manifest\``); + } + if (manifest.id !== id) { + throw new Error( + `${rel} declares id ${manifest.id}; it must match its folder`, + ); + } + return manifest; +} + +export async function check(dir = DEFAULT_DIR) { + const ids = fs.existsSync(dir) + ? fs + .readdirSync(dir, { withFileTypes: true }) + .filter((e) => fs.existsSync(path.join(dir, e.name, "manifest.ts"))) + .map((e) => e.name) + .sort() + : []; + const manifests = await Promise.all(ids.map((id) => load(dir, id))); + // Throws on any conflict in any distribution. + const offered = (d) => [...buildRegistry(manifests, d).plugins]; + return { + distributions: { + oss: offered("oss"), + cloud: offered("cloud"), + onprem: offered("onprem"), + }, + plugins: manifests.map((m) => ({ + id: m.id, + distributions: [...m.distributions], + slots: Object.keys(m.slots ?? {}), + routes: (m.routes ?? []).map((r) => r.path), + })), + }; +} + +// No top-level await: vite.config.js imports this file and Vite bundles the +// config as CommonJS. +if (import.meta.url === pathToFileURL(process.argv[1]).href) { + check() + .then((matrix) => { + const json = `${JSON.stringify(matrix, null, 2)}\n`; + const out = process.argv[2]; + if (out) { + fs.writeFileSync(out, json); + } else { + process.stdout.write(json); + } + }) + .catch((err) => { + console.error(`v1-plugins: ${err.message}`); + process.exit(1); + }); +} diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 61fa459ac3..0620b604da 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -37,6 +37,17 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | `plugins/` | Cloud overlay target; empty in the public repository | everything | | `test/` | Render helpers, fixtures, network mocks | everything | +## Plugins + +`lib/plugins/` is the extension-point contract. Pages and the shell import `Slot` and `pluginRoutes` from `@v1/lib/plugins`; they never import a plugin. + +- `types.ts` names every slot and its props. Adding a slot is one line there. +- A plugin is `plugins//manifest.ts` exporting `manifest` (`satisfies PluginManifest`), with `distributions` (`cloud`, `onprem`) and lazy `slots` and `routes`. The folder name is the id. A manifest may only `import type`: the build check loads it without bundling. +- `` shows the fallback when no plugin fills the slot, silently. A filler that fails to load or render shows the fallback and is reported; each slot has its own error boundary. +- `VITE_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) picks the plugins the build offers. +- Every build runs `scripts/v1-plugins.mjs`. It fails on a slot filled twice or a route defined twice in any distribution, or a plugin with no distribution, and writes `plugin-matrix.json` to the build output. `node scripts/v1-plugins.mjs` prints the matrix. +- v0's optional-plugin stub skips `src/v1`, so a missing module in v1 fails the build. + ## Scripts | Script | Checks | diff --git a/frontend/src/v1/lib/plugins/Slot.test.tsx b/frontend/src/v1/lib/plugins/Slot.test.tsx new file mode 100644 index 0000000000..8cbddbfa8d --- /dev/null +++ b/frontend/src/v1/lib/plugins/Slot.test.tsx @@ -0,0 +1,105 @@ +import { render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { buildRegistry } from "./registry"; +import { makeSlot } from "./Slot"; +import type { PluginManifest } from "./types"; + +const fallback = fallback; + +function slotFor(manifests: PluginManifest[]) { + const report = vi.fn(); + const Slot = makeSlot(buildRegistry(manifests, "cloud"), report); + return { Slot, report }; +} + +describe("Slot", () => { + beforeEach(() => { + // React logs caught render errors; the report spy is the assertion. + vi.spyOn(console, "error").mockImplementation(() => undefined); + }); + afterEach(() => vi.restoreAllMocks()); + + it("renders the fallback silently when no plugin fills it", () => { + const { Slot, report } = slotFor([]); + render(); + expect(screen.getByText("fallback")).toBeInTheDocument(); + expect(report).not.toHaveBeenCalled(); + }); + + it("renders the filler when a plugin fills it", async () => { + const { Slot, report } = slotFor([ + { + id: "switcher", + distributions: ["cloud"], + slots: { + "shell.product-switcher": async () => ({ + default: () => switcher, + }), + }, + }, + ]); + render(); + expect(await screen.findByText("switcher")).toBeInTheDocument(); + expect(report).not.toHaveBeenCalled(); + }); + + it("reports a filler that fails to load and shows the fallback", async () => { + const { Slot, report } = slotFor([ + { + id: "broken", + distributions: ["cloud"], + slots: { + "shell.product-switcher": () => Promise.reject(new Error("chunk")), + }, + }, + ]); + render(); + expect(await screen.findByText("fallback")).toBeInTheDocument(); + await vi.waitFor(() => + expect(report).toHaveBeenCalledWith( + "broken", + "shell.product-switcher", + expect.objectContaining({ message: "chunk" }), + ), + ); + }); + + it("keeps a failing plugin from taking down another slot", async () => { + const { Slot, report } = slotFor([ + { + id: "broken", + distributions: ["cloud"], + slots: { + "account-menu.plan-billing": async () => ({ + default: () => { + throw new Error("render"); + }, + }), + }, + }, + { + id: "switcher", + distributions: ["cloud"], + slots: { + "shell.product-switcher": async () => ({ + default: () => switcher, + }), + }, + }, + ]); + render( + <> + + + , + ); + expect(await screen.findByText("switcher")).toBeInTheDocument(); + expect(await screen.findByText("fallback")).toBeInTheDocument(); + expect(report).toHaveBeenCalledTimes(1); + expect(report).toHaveBeenCalledWith( + "broken", + "account-menu.plan-billing", + expect.objectContaining({ message: "render" }), + ); + }); +}); diff --git a/frontend/src/v1/lib/plugins/Slot.tsx b/frontend/src/v1/lib/plugins/Slot.tsx new file mode 100644 index 0000000000..0295ced42e --- /dev/null +++ b/frontend/src/v1/lib/plugins/Slot.tsx @@ -0,0 +1,87 @@ +import { + Component, + type ComponentType, + type ErrorInfo, + lazy, + type ReactNode, + Suspense, +} from "react"; +import type { PluginRegistry } from "./registry"; +import type { SlotId, SlotProps } from "./types"; + +export type PluginErrorReporter = ( + owner: string, + slot: SlotId, + error: unknown, +) => void; + +export interface SlotComponentProps { + id: K; + props: SlotProps[K]; + /** Shown when no plugin fills the slot, while it loads, and if it fails. */ + fallback: ReactNode; +} + +interface BoundaryProps { + fallback: ReactNode; + onError: (error: unknown) => void; + children: ReactNode; +} + +class SlotBoundary extends Component { + override state = { failed: false }; + + static getDerivedStateFromError(): { failed: boolean } { + return { failed: true }; + } + + override componentDidCatch(error: unknown, _info: ErrorInfo): void { + this.props.onError(error); + } + + override render(): ReactNode { + return this.state.failed ? this.props.fallback : this.props.children; + } +} + +/** + * The `` component for one registry. Each slot gets its own error + * boundary, so a plugin that fails to load or render shows that slot's + * fallback and is reported, and every other slot keeps working. + */ +export function makeSlot( + registry: PluginRegistry, + report: PluginErrorReporter, +): (p: SlotComponentProps) => ReactNode { + const fillers = new Map>(); + + function filler(id: K): ComponentType | null { + const entry = registry.slot(id); + if (!entry) { + return null; + } + if (!fillers.has(id)) { + fillers.set(id, lazy(entry.load) as ComponentType); + } + return fillers.get(id) as ComponentType; + } + + return function Slot({ + id, + props, + fallback, + }: SlotComponentProps): ReactNode { + const Filler = filler(id); + if (!Filler) { + return fallback; + } + const owner = registry.slot(id)?.owner ?? "unknown"; + return ( + report(owner, id, e)}> + + + + + ); + }; +} diff --git a/frontend/src/v1/lib/plugins/index.ts b/frontend/src/v1/lib/plugins/index.ts new file mode 100644 index 0000000000..86b52a2403 --- /dev/null +++ b/frontend/src/v1/lib/plugins/index.ts @@ -0,0 +1,45 @@ +/* + * v1's extension points. Plugins are found at build time from + * src/v1/plugins//manifest.ts (copied in by the cloud overlay), so an + * absent plugin has no manifest and its slots show their fallbacks. Every + * import error left is a real failure and is reported. + */ +import { buildRegistry, type RouteEntry } from "./registry"; +import { makeSlot, type PluginErrorReporter } from "./Slot"; +import type { Distribution, PluginManifest } from "./types"; + +export type { SlotComponentProps } from "./Slot"; +export type * from "./types"; + +const DISTRIBUTIONS: readonly Distribution[] = ["oss", "cloud", "onprem"]; + +function currentDistribution(): Distribution { + const value = import.meta.env.VITE_DISTRIBUTION ?? "oss"; + if (!DISTRIBUTIONS.includes(value)) { + throw new Error( + `VITE_DISTRIBUTION must be one of ${DISTRIBUTIONS.join(", ")}; got ${value}`, + ); + } + return value; +} + +const manifests = Object.values( + import.meta.glob("../../plugins/*/manifest.ts", { + eager: true, + import: "manifest", + }), +); + +export const registry = buildRegistry(manifests, currentDistribution()); + +// ponytail: logs only; the v1 notification contract adds the toast here. +const report: PluginErrorReporter = (owner, slot, error) => { + console.error(`v1 plugin ${owner} failed in slot ${slot}`, error); +}; + +export const Slot = makeSlot(registry, report); + +/** Routes contributed by plugins in this distribution, for the v1 router. */ +export function pluginRoutes(): readonly RouteEntry[] { + return registry.routes; +} diff --git a/frontend/src/v1/lib/plugins/missing-import.test.ts b/frontend/src/v1/lib/plugins/missing-import.test.ts new file mode 100644 index 0000000000..32704726b6 --- /dev/null +++ b/frontend/src/v1/lib/plugins/missing-import.test.ts @@ -0,0 +1,13 @@ +import { expect, it } from "vitest"; + +/* + * v0's optionalPluginImports (vite.config.js, vitest.config.mjs) turns any + * missing import under a `/plugins/` path into a stub. v1 has no optional + * imports: an absent plugin has no manifest, so a missing module is a bug. + * The specifier is a variable so the failure surfaces at run time, where the + * test can see it; a literal one fails the transform, which is the same rule. + */ +it("a missing module under v1 fails to import instead of being stubbed", async () => { + const missing = "./does-not-exist"; + await expect(import(/* @vite-ignore */ missing)).rejects.toThrow(); +}); diff --git a/frontend/src/v1/lib/plugins/registry.test.ts b/frontend/src/v1/lib/plugins/registry.test.ts new file mode 100644 index 0000000000..cc8eb1ef6d --- /dev/null +++ b/frontend/src/v1/lib/plugins/registry.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from "vitest"; +import { buildRegistry, PluginConflictError } from "./registry"; +import type { PluginManifest } from "./types"; + +const load = () => Promise.resolve({ default: () => null }); + +const billing: PluginManifest = { + id: "billing", + distributions: ["cloud"], + slots: { "account-menu.plan-billing": load }, + routes: [{ path: "/billing", load }], +}; +const switcher: PluginManifest = { + id: "switcher", + distributions: ["cloud", "onprem"], + slots: { "shell.product-switcher": load }, +}; + +describe("buildRegistry", () => { + it("keeps only plugins that list the current distribution", () => { + const onprem = buildRegistry([billing, switcher], "onprem"); + expect(onprem.plugins).toEqual(["switcher"]); + expect(onprem.slot("account-menu.plan-billing")).toBeUndefined(); + expect(onprem.slot("shell.product-switcher")?.owner).toBe("switcher"); + expect(onprem.routes).toEqual([]); + + const cloud = buildRegistry([billing, switcher], "cloud"); + expect(cloud.plugins).toEqual(["billing", "switcher"]); + expect(cloud.routes.map((r) => r.path)).toEqual(["/billing"]); + }); + + it("offers nothing in the OSS build", () => { + const oss = buildRegistry([billing, switcher], "oss"); + expect(oss.plugins).toEqual([]); + expect(oss.slot("shell.product-switcher")).toBeUndefined(); + }); + + it("fails on a slot filled twice, naming both plugins", () => { + const twin = { ...switcher, id: "twin" }; + expect(() => buildRegistry([switcher, twin], "cloud")).toThrow( + PluginConflictError, + ); + expect(() => buildRegistry([switcher, twin], "cloud")).toThrow( + /shell\.product-switcher.*switcher.*twin/, + ); + }); + + it("fails on a route two plugins contribute, naming both", () => { + const other: PluginManifest = { + id: "other", + distributions: ["cloud"], + routes: [{ path: "/billing", load }], + }; + expect(() => buildRegistry([billing, other], "cloud")).toThrow( + /\/billing.*billing.*other/, + ); + }); + + it("fails on a route OSS also defines, naming both owners", () => { + expect(() => buildRegistry([billing], "cloud", ["/billing"])).toThrow( + /\/billing.*OSS.*billing/, + ); + }); + + it("checks conflicts in every distribution, not only the current one", () => { + const twin = { ...billing, id: "twin", routes: [] }; + expect(() => buildRegistry([billing, twin], "oss")).toThrow( + /account-menu\.plan-billing/, + ); + }); + + it("lets plugins in disjoint distributions fill the same slot", () => { + const cloudOnly = { ...switcher, distributions: ["cloud"] as const }; + const onpremOnly = { + ...switcher, + id: "switcher-onprem", + distributions: ["onprem"] as const, + }; + const onprem = buildRegistry([cloudOnly, onpremOnly], "onprem"); + expect(onprem.slot("shell.product-switcher")?.owner).toBe( + "switcher-onprem", + ); + }); + + it("fails on a plugin that declares no distribution", () => { + const undeclared = { ...switcher, distributions: [] }; + expect(() => buildRegistry([undeclared], "cloud")).toThrow( + /switcher declares no distributions/, + ); + }); + + it("fails on two plugins with the same id", () => { + expect(() => buildRegistry([switcher, switcher], "cloud")).toThrow( + /plugin id switcher is used twice/, + ); + }); +}); diff --git a/frontend/src/v1/lib/plugins/registry.ts b/frontend/src/v1/lib/plugins/registry.ts new file mode 100644 index 0000000000..3241f78406 --- /dev/null +++ b/frontend/src/v1/lib/plugins/registry.ts @@ -0,0 +1,115 @@ +import type { + ComponentLoader, + Distribution, + PluginDistribution, + PluginManifest, + PluginRoute, + SlotId, + SlotProps, +} from "./types"; + +/* + * Runtime imports stay at zero: scripts/v1-plugins.mjs loads this file with + * Node or Bun type stripping, so it may only `import type`. + */ + +export class PluginConflictError extends Error { + override name = "PluginConflictError"; +} + +export interface SlotEntry { + owner: string; + load: ComponentLoader; +} + +export interface RouteEntry extends PluginRoute { + owner: string; +} + +export interface PluginRegistry { + distribution: Distribution; + plugins: readonly string[]; + routes: readonly RouteEntry[]; + slot(id: K): SlotEntry | undefined; +} + +const PLUGIN_DISTRIBUTIONS: readonly PluginDistribution[] = ["cloud", "onprem"]; + +function assertDeclared(manifests: readonly PluginManifest[]): void { + const seen = new Set(); + for (const m of manifests) { + if (seen.has(m.id)) { + throw new PluginConflictError(`plugin id ${m.id} is used twice`); + } + seen.add(m.id); + if (m.distributions.length === 0) { + throw new PluginConflictError(`${m.id} declares no distributions`); + } + } +} + +function collect( + manifests: readonly PluginManifest[], + ossRoutes: readonly string[], +): { slots: Map>; routes: RouteEntry[] } { + const slots = new Map>(); + const routeOwners = new Map( + ossRoutes.map((path) => [path, "OSS"]), + ); + const routes: RouteEntry[] = []; + for (const m of manifests) { + for (const [id, load] of Object.entries(m.slots ?? {})) { + const slotId = id as SlotId; + const taken = slots.get(slotId); + if (taken) { + throw new PluginConflictError( + `slot ${slotId} is filled by both ${taken.owner} and ${m.id}`, + ); + } + slots.set(slotId, { + owner: m.id, + load: load as ComponentLoader, + }); + } + for (const route of m.routes ?? []) { + const taken = routeOwners.get(route.path); + if (taken) { + throw new PluginConflictError( + `route ${route.path} is defined by both ${taken} and ${m.id}`, + ); + } + routeOwners.set(route.path, m.id); + routes.push({ ...route, owner: m.id }); + } + } + return { slots, routes }; +} + +/** + * The plugins, slots and routes one distribution offers. Throws + * PluginConflictError when any distribution would have a slot filled twice, + * a route defined twice, or a plugin that declares nowhere to ship. + */ +export function buildRegistry( + manifests: readonly PluginManifest[], + distribution: Distribution, + ossRoutes: readonly string[] = [], +): PluginRegistry { + assertDeclared(manifests); + const inDistribution = (d: Distribution) => + manifests.filter((m) => + (m.distributions as readonly Distribution[]).includes(d), + ); + for (const d of PLUGIN_DISTRIBUTIONS) { + collect(inDistribution(d), ossRoutes); + } + const offered = inDistribution(distribution); + const { slots, routes } = collect(offered, ossRoutes); + return { + distribution, + plugins: offered.map((m) => m.id), + routes, + slot: (id: K) => + slots.get(id) as SlotEntry | undefined, + }; +} diff --git a/frontend/src/v1/lib/plugins/types.ts b/frontend/src/v1/lib/plugins/types.ts new file mode 100644 index 0000000000..e10fa67c46 --- /dev/null +++ b/frontend/src/v1/lib/plugins/types.ts @@ -0,0 +1,45 @@ +import type { ComponentType } from "react"; + +/* + * Type-only module. Plugin manifests import from here with `import type`, so + * Node and Bun can load a manifest without bundling it (the build check in + * scripts/v1-plugins.mjs does exactly that). + */ + +/** Builds a cloud plugin can ship in. The public OSS build has no plugins. */ +export type PluginDistribution = "cloud" | "onprem"; +export type Distribution = "oss" | PluginDistribution; + +/* + * Every extension point and its props. A filler receives these props; the + * host passes them. Adding a slot is one line here. + * ponytail: props are empty until the host page that renders each slot is + * built; that task adds the fields it passes. + */ +export interface SlotProps { + "prompt-studio.lookups": Record; + "prompt-studio.cloud-tools": Record; + "workflows.human-review": Record; + "account-menu.plan-billing": Record; + "shell.product-switcher": Record; + "platform.hitl-keys": Record; + "platform.hitl-settings": Record; + "dashboard.subscription-usage": Record; + "dashboard.attention-items": Record; +} + +export type SlotId = keyof SlotProps; + +export type ComponentLoader

= () => Promise<{ default: ComponentType

}>; + +export interface PluginRoute { + path: string; + load: ComponentLoader>; +} + +export interface PluginManifest { + id: string; + distributions: readonly PluginDistribution[]; + slots?: { [K in SlotId]?: ComponentLoader }; + routes?: readonly PluginRoute[]; +} diff --git a/frontend/src/v1/plugins/.gitkeep b/frontend/src/v1/plugins/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/v1/plugins/manifest.ts b/frontend/src/v1/plugins/manifest.ts deleted file mode 100644 index c139d37fbf..0000000000 --- a/frontend/src/v1/plugins/manifest.ts +++ /dev/null @@ -1,6 +0,0 @@ -/* - * Cloud overlay target. Empty in the public repository: the cloud build - * copies its v1 plugins into this folder. The manifest's shape and contents - * belong to UN-4285 and UN-4286. - */ -export const pluginManifest = [] as const; diff --git a/frontend/src/v1/test/jest-dom.d.ts b/frontend/src/v1/test/jest-dom.d.ts new file mode 100644 index 0000000000..0d0fff9d8f --- /dev/null +++ b/frontend/src/v1/test/jest-dom.d.ts @@ -0,0 +1,2 @@ +// jest-dom matchers for v1 tests; src/setupTests.js registers them at run time. +/// diff --git a/frontend/vite.config.js b/frontend/vite.config.js index fecc6b40f5..79eba7b1a6 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -35,6 +35,10 @@ function optionalPluginImports() { resolveId(source, importer) { if (!importer) return null; + // v1 has no optional imports: an absent v1 plugin has no manifest, so a + // missing module under src/v1 is a bug and must fail the build. + if (importer.includes("/src/v1/")) return null; + // Only handle relative imports if (!source.startsWith(".")) return null; @@ -134,6 +138,29 @@ function jsxInJs() { }; } +/* + * v1 plugin check (scripts/v1-plugins.mjs): fails the build on a slot filled + * twice, a route defined twice or a plugin with no distribution, and + * publishes the distribution matrix as plugin-matrix.json. + */ +function v1PluginMatrix() { + let matrix; + return { + name: "v1-plugin-matrix", + async buildStart() { + const { check } = await import("./scripts/v1-plugins.mjs"); + matrix = await check(); + }, + generateBundle() { + this.emitFile({ + type: "asset", + fileName: "plugin-matrix.json", + source: `${JSON.stringify(matrix, null, 2)}\n`, + }); + }, + }; +} + // https://vite.dev/config/ export default defineConfig(({ mode }) => { // Load env file based on `mode` in the current working directory. @@ -155,6 +182,7 @@ export default defineConfig(({ mode }) => { }), // SVG as React component support (for `import Logo from './logo.svg?react'`) svgr(), + v1PluginMatrix(), ], /* diff --git a/frontend/vitest.config.mjs b/frontend/vitest.config.mjs index 25cbe07f68..824d21e982 100644 --- a/frontend/vitest.config.mjs +++ b/frontend/vitest.config.mjs @@ -51,7 +51,12 @@ function optionalPluginImports() { return { name: "optional-plugin-imports", resolveId(source, importer) { - if (!importer || !source.startsWith(".")) { + // v1 has no optional imports; a missing module there must fail. + if ( + !importer || + !source.startsWith(".") || + importer.includes("/src/v1/") + ) { return null; } const sourcePath = source.split("?")[0].split("#")[0]; From 1f426f9991913df0af88f82457701f6441693cfc Mon Sep 17 00:00:00 2001 From: Athul Date: Sat, 24 Oct 2026 19:56:16 +0530 Subject: [PATCH 08/26] UN-4286 [FEAT] Pass VITE_DISTRIBUTION through the frontend Docker build The cloud workflows set it to cloud or onprem; the default, oss, keeps the public image unchanged. Co-Authored-By: Claude Opus 5.5 --- docker/dockerfiles/frontend.Dockerfile | 6 ++++++ frontend/src/v1/README.md | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docker/dockerfiles/frontend.Dockerfile b/docker/dockerfiles/frontend.Dockerfile index 0c3fdcc92a..54843defba 100644 --- a/docker/dockerfiles/frontend.Dockerfile +++ b/docker/dockerfiles/frontend.Dockerfile @@ -46,6 +46,12 @@ RUN bun install --frozen-lockfile --ignore-scripts # Copy the rest of the application files COPY ${BUILD_CONTEXT_PATH}/ . +# Which v1 plugins the build offers (oss, cloud or onprem); the cloud +# workflows set it. Declared after install so a different value keeps the +# dependency layer cached. See frontend/src/v1/README.md. +ARG VITE_DISTRIBUTION=oss +ENV VITE_DISTRIBUTION=${VITE_DISTRIBUTION} + # Build with Vite RUN bun run build diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 0620b604da..83a64d769e 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -44,7 +44,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, - `types.ts` names every slot and its props. Adding a slot is one line there. - A plugin is `plugins//manifest.ts` exporting `manifest` (`satisfies PluginManifest`), with `distributions` (`cloud`, `onprem`) and lazy `slots` and `routes`. The folder name is the id. A manifest may only `import type`: the build check loads it without bundling. - `` shows the fallback when no plugin fills the slot, silently. A filler that fails to load or render shows the fallback and is reported; each slot has its own error boundary. -- `VITE_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) picks the plugins the build offers. +- `VITE_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) picks the plugins the build offers. The frontend Dockerfile takes it as a build arg. - Every build runs `scripts/v1-plugins.mjs`. It fails on a slot filled twice or a route defined twice in any distribution, or a plugin with no distribution, and writes `plugin-matrix.json` to the build output. `node scripts/v1-plugins.mjs` prints the matrix. - v0's optional-plugin stub skips `src/v1`, so a missing module in v1 fails the build. From 7ae6de57242d3a5935d360941177dc6249cfa833 Mon Sep 17 00:00:00 2001 From: Athul Date: Sat, 24 Oct 2026 19:59:30 +0530 Subject: [PATCH 09/26] UN-4286 [FEAT] Keep overlaid v1 plugins out of OSS commits The cloud overlay copies plugins into frontend/src/v1/plugins/; ignore them as frontend/src/plugins/* already is, keeping only the .gitkeep. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 4817bafa05..eb141a5dbe 100644 --- a/.gitignore +++ b/.gitignore @@ -639,6 +639,9 @@ workers/pluggable_worker/* # FE Plugins frontend/src/plugins/* frontend/public/llm-whisperer/ +# v1 plugins come from the cloud overlay; only the placeholder file is OSS. +frontend/src/v1/plugins/* +!frontend/src/v1/plugins/.gitkeep # TODO: Ensure its made generic to abstract subfolder and file names From cbfd33244b16e06d27029e960c4e24f32915691d Mon Sep 17 00:00:00 2001 From: Athul Date: Tue, 27 Oct 2026 18:05:22 +0530 Subject: [PATCH 10/26] UN-4286 [FEAT] Build the plugin registry in plugins/ and hand Slot to lib through context The scaffold allows lib/ to import only lib/, and only plugins/ may see the overlaid manifests. plugins/index.ts now builds the registry and exports PluginSlot and pluginRoutes(); lib/plugins keeps the contract and a Slot that reads PluginSlot from PluginProvider, which app/ will mount. With no provider every slot shows its fallback. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 3 +- frontend/src/v1/README.md | 2 +- frontend/src/v1/lib/plugins/context.test.tsx | 33 ++++++++++++++++ frontend/src/v1/lib/plugins/index.tsx | 41 ++++++++++++++++++++ frontend/src/v1/{lib => }/plugins/index.ts | 27 +++++++------ 5 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 frontend/src/v1/lib/plugins/context.test.tsx create mode 100644 frontend/src/v1/lib/plugins/index.tsx rename frontend/src/v1/{lib => }/plugins/index.ts (57%) diff --git a/.gitignore b/.gitignore index eb141a5dbe..b820925e1e 100644 --- a/.gitignore +++ b/.gitignore @@ -639,9 +639,10 @@ workers/pluggable_worker/* # FE Plugins frontend/src/plugins/* frontend/public/llm-whisperer/ -# v1 plugins come from the cloud overlay; only the placeholder file is OSS. +# v1 plugins come from the cloud overlay; only the placeholder and the loader are OSS. frontend/src/v1/plugins/* !frontend/src/v1/plugins/.gitkeep +!frontend/src/v1/plugins/index.ts # TODO: Ensure its made generic to abstract subfolder and file names diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 83a64d769e..5b2af77e4b 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -39,7 +39,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, ## Plugins -`lib/plugins/` is the extension-point contract. Pages and the shell import `Slot` and `pluginRoutes` from `@v1/lib/plugins`; they never import a plugin. +`lib/plugins/` is the extension-point contract. Pages and the shell import `Slot` from `@v1/lib/plugins`; they never import a plugin. `plugins/index.ts` builds the registry from the manifests and exports `PluginSlot` and `pluginRoutes()`; `app/` passes `PluginSlot` to `PluginProvider` and the routes to the router. With no provider, every slot shows its fallback. - `types.ts` names every slot and its props. Adding a slot is one line there. - A plugin is `plugins//manifest.ts` exporting `manifest` (`satisfies PluginManifest`), with `distributions` (`cloud`, `onprem`) and lazy `slots` and `routes`. The folder name is the id. A manifest may only `import type`: the build check loads it without bundling. diff --git a/frontend/src/v1/lib/plugins/context.test.tsx b/frontend/src/v1/lib/plugins/context.test.tsx new file mode 100644 index 0000000000..cd1bdd8c77 --- /dev/null +++ b/frontend/src/v1/lib/plugins/context.test.tsx @@ -0,0 +1,33 @@ +import { render, screen } from "@testing-library/react"; +import { expect, it } from "vitest"; +import { buildRegistry, makeSlot, PluginProvider, Slot } from "./index"; + +const fallback = fallback; + +it("a slot with no provider shows its fallback", () => { + render(); + expect(screen.getByText("fallback")).toBeInTheDocument(); +}); + +it("a slot renders the provider's filler", async () => { + const registry = buildRegistry( + [ + { + id: "switcher", + distributions: ["cloud"], + slots: { + "shell.product-switcher": async () => ({ + default: () => switcher, + }), + }, + }, + ], + "cloud", + ); + render( + undefined)}> + + , + ); + expect(await screen.findByText("switcher")).toBeInTheDocument(); +}); diff --git a/frontend/src/v1/lib/plugins/index.tsx b/frontend/src/v1/lib/plugins/index.tsx new file mode 100644 index 0000000000..85c6f2c0ca --- /dev/null +++ b/frontend/src/v1/lib/plugins/index.tsx @@ -0,0 +1,41 @@ +/* + * v1's extension-point contract. Shell and pages render `` from here; + * they never import a plugin. The registry itself is built in + * `@v1/plugins` (the only folder that may see plugin manifests) and handed + * down by `app/` through `PluginProvider`. With no provider, as in OSS tests, + * every slot shows its fallback. + */ +import { createContext, type ReactNode, useContext } from "react"; +import type { makeSlot, SlotComponentProps } from "./Slot"; +import type { SlotId } from "./types"; + +export { + buildRegistry, + PluginConflictError, + type RouteEntry, +} from "./registry"; +export { + makeSlot, + type PluginErrorReporter, + type SlotComponentProps, +} from "./Slot"; +export type * from "./types"; + +type SlotRenderer = ReturnType; + +const SlotContext = createContext(null); + +export function PluginProvider({ + slot, + children, +}: { + slot: SlotRenderer; + children: ReactNode; +}): ReactNode { + return {children}; +} + +export function Slot(p: SlotComponentProps): ReactNode { + const Render = useContext(SlotContext); + return Render ? : p.fallback; +} diff --git a/frontend/src/v1/lib/plugins/index.ts b/frontend/src/v1/plugins/index.ts similarity index 57% rename from frontend/src/v1/lib/plugins/index.ts rename to frontend/src/v1/plugins/index.ts index 86b52a2403..f8e7e25a75 100644 --- a/frontend/src/v1/lib/plugins/index.ts +++ b/frontend/src/v1/plugins/index.ts @@ -1,15 +1,18 @@ /* - * v1's extension points. Plugins are found at build time from - * src/v1/plugins//manifest.ts (copied in by the cloud overlay), so an - * absent plugin has no manifest and its slots show their fallbacks. Every - * import error left is a real failure and is reported. + * Builds the plugin registry from the manifests the cloud overlay copies in + * as `plugins//manifest.ts`. An absent plugin has no manifest, so its + * slots show their fallbacks; every import error left is a real failure and + * is reported. `app/` passes `PluginSlot` to `PluginProvider` and + * `pluginRoutes()` to the router. */ -import { buildRegistry, type RouteEntry } from "./registry"; -import { makeSlot, type PluginErrorReporter } from "./Slot"; -import type { Distribution, PluginManifest } from "./types"; - -export type { SlotComponentProps } from "./Slot"; -export type * from "./types"; +import { + buildRegistry, + type Distribution, + makeSlot, + type PluginErrorReporter, + type PluginManifest, + type RouteEntry, +} from "@v1/lib/plugins"; const DISTRIBUTIONS: readonly Distribution[] = ["oss", "cloud", "onprem"]; @@ -24,7 +27,7 @@ function currentDistribution(): Distribution { } const manifests = Object.values( - import.meta.glob("../../plugins/*/manifest.ts", { + import.meta.glob("./*/manifest.ts", { eager: true, import: "manifest", }), @@ -37,7 +40,7 @@ const report: PluginErrorReporter = (owner, slot, error) => { console.error(`v1 plugin ${owner} failed in slot ${slot}`, error); }; -export const Slot = makeSlot(registry, report); +export const PluginSlot = makeSlot(registry, report); /** Routes contributed by plugins in this distribution, for the v1 router. */ export function pluginRoutes(): readonly RouteEntry[] { From 1babd39e6215d36d593de6233c9cd795b07bece0 Mon Sep 17 00:00:00 2001 From: Athul Date: Tue, 27 Oct 2026 18:05:35 +0530 Subject: [PATCH 11/26] UN-4286 [FIX] Let v1 tests typecheck until the scaffold settles the chai clash TEMPORARY. With skipLibCheck off, @types/chai 5.2.3 and vitest 3.2.6 both declare Chai containSubset, so typecheck:v1 fails once any v1 test imports vitest. Drop this commit when #2380 settles it (pin @types/chai or turn the option on). Co-Authored-By: Claude Opus 5.5 --- frontend/src/v1/tsconfig.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/v1/tsconfig.json b/frontend/src/v1/tsconfig.json index 0a10f9dbdb..a2c4d69282 100644 --- a/frontend/src/v1/tsconfig.json +++ b/frontend/src/v1/tsconfig.json @@ -17,9 +17,10 @@ "allowJs": false, "noEmit": true, - /* Off so hand-written .d.ts under v1 are checked too; with it on, an - * unresolved name in one silently becomes `any`. */ - "skipLibCheck": false, + /* TEMPORARY (UN-4286): on, because with it off @types/chai 5.2.3 and + * vitest 3.2.6 both declare Chai `containSubset`, so typecheck fails as + * soon as a v1 test imports vitest. Drop once the scaffold settles it. */ + "skipLibCheck": true, "esModuleInterop": true, "resolveJsonModule": true, "isolatedModules": true, From 861c7b6637c090e9d4c3dc9e314f284994472c70 Mon Sep 17 00:00:00 2001 From: Athul Date: Wed, 28 Oct 2026 12:11:30 +0530 Subject: [PATCH 12/26] UN-4286 [FIX] Retry a slot's import after a failed load Co-Authored-By: Claude Opus 5.5 --- frontend/src/v1/lib/plugins/Slot.test.tsx | 44 +++++++++++++++++++++++ frontend/src/v1/lib/plugins/Slot.tsx | 43 +++++++++++++++++++--- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/frontend/src/v1/lib/plugins/Slot.test.tsx b/frontend/src/v1/lib/plugins/Slot.test.tsx index 8cbddbfa8d..9a39f11be5 100644 --- a/frontend/src/v1/lib/plugins/Slot.test.tsx +++ b/frontend/src/v1/lib/plugins/Slot.test.tsx @@ -102,4 +102,48 @@ describe("Slot", () => { expect.objectContaining({ message: "render" }), ); }); + + it("loads again on a later mount after a failed load", async () => { + const load = vi + .fn() + .mockRejectedValueOnce(new Error("chunk")) + .mockResolvedValue({ default: () => switcher }); + const { Slot, report } = slotFor([ + { + id: "flaky", + distributions: ["cloud"], + slots: { "shell.product-switcher": load }, + }, + ]); + const first = render( + , + ); + await vi.waitFor(() => expect(report).toHaveBeenCalledTimes(1)); + first.unmount(); + render(); + expect(await screen.findByText("switcher")).toBeInTheDocument(); + expect(load).toHaveBeenCalledTimes(2); + }); + + it("recovers in place on the next render after a failed load", async () => { + const load = vi + .fn() + .mockRejectedValueOnce(new Error("chunk")) + .mockResolvedValue({ default: () => switcher }); + const { Slot, report } = slotFor([ + { + id: "flaky", + distributions: ["cloud"], + slots: { "shell.product-switcher": load }, + }, + ]); + const tree = ( + + ); + const { rerender } = render(tree); + await vi.waitFor(() => expect(report).toHaveBeenCalledTimes(1)); + rerender(

{tree}
); + rerender(tree); + expect(await screen.findByText("switcher")).toBeInTheDocument(); + }); }); diff --git a/frontend/src/v1/lib/plugins/Slot.tsx b/frontend/src/v1/lib/plugins/Slot.tsx index 0295ced42e..367f7ef91f 100644 --- a/frontend/src/v1/lib/plugins/Slot.tsx +++ b/frontend/src/v1/lib/plugins/Slot.tsx @@ -23,18 +23,37 @@ export interface SlotComponentProps { } interface BoundaryProps { + /** A new value clears an earlier failure, so a fresh filler gets a try. */ + resetKey: unknown; fallback: ReactNode; onError: (error: unknown) => void; children: ReactNode; } -class SlotBoundary extends Component { - override state = { failed: false }; +interface BoundaryState { + failed: boolean; + resetKey: unknown; +} + +class SlotBoundary extends Component { + override state: BoundaryState = { + failed: false, + resetKey: this.props.resetKey, + }; - static getDerivedStateFromError(): { failed: boolean } { + static getDerivedStateFromError(): Partial { return { failed: true }; } + static getDerivedStateFromProps( + props: BoundaryProps, + state: BoundaryState, + ): Partial | null { + return props.resetKey === state.resetKey + ? null + : { failed: false, resetKey: props.resetKey }; + } + override componentDidCatch(error: unknown, _info: ErrorInfo): void { this.props.onError(error); } @@ -61,7 +80,17 @@ export function makeSlot( return null; } if (!fillers.has(id)) { - fillers.set(id, lazy(entry.load) as ComponentType); + // React caches a rejected lazy promise, so a failed load drops the + // filler and the next render imports the chunk again. + const lazyFiller = lazy(() => + entry.load().catch((err: unknown) => { + if (fillers.get(id) === lazyFiller) { + fillers.delete(id); + } + throw err; + }), + ) as ComponentType; + fillers.set(id, lazyFiller); } return fillers.get(id) as ComponentType; } @@ -77,7 +106,11 @@ export function makeSlot( } const owner = registry.slot(id)?.owner ?? "unknown"; return ( - report(owner, id, e)}> + report(owner, id, e)} + > From 6c31314878aaf59b34936cb210c9945f5c668bdb Mon Sep 17 00:00:00 2001 From: Athul Date: Wed, 28 Oct 2026 12:13:01 +0530 Subject: [PATCH 13/26] UN-4286 [FIX] Reject unknown slots and distributions and load manifests with esbuild The build check now transpiles the registry and each manifest with esbuild, so it runs on the Node versions the frontend supports. It also fails on a folder with no manifest.ts, resolves argv[1] before the main-guard compare, and has CLI tests. The README notes the distribution matrix. Co-Authored-By: Claude Opus 5.5 --- frontend/scripts/v1-plugins.fixtures.mjs | 48 ++++++++++++++++ frontend/scripts/v1-plugins.mjs | 59 +++++++++++++++++--- frontend/src/v1/README.md | 2 + frontend/src/v1/lib/plugins/registry.test.ts | 21 +++++++ frontend/src/v1/lib/plugins/registry.ts | 39 +++++++------ frontend/src/v1/lib/plugins/types.ts | 40 ++++++------- 6 files changed, 165 insertions(+), 44 deletions(-) diff --git a/frontend/scripts/v1-plugins.fixtures.mjs b/frontend/scripts/v1-plugins.fixtures.mjs index 4261a2d196..4b982fbe9b 100644 --- a/frontend/scripts/v1-plugins.fixtures.mjs +++ b/frontend/scripts/v1-plugins.fixtures.mjs @@ -1,5 +1,6 @@ // Fixtures for v1-plugins.mjs. Run with `node --test` (bun run test:v1). import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -87,3 +88,50 @@ test("a manifest with a runtime import fails with a clear message", async () => }); await assert.rejects(check(dir), /bad\/manifest\.ts.*import type/); }); + +test("a slot id the contract does not define fails, naming the plugin", async () => { + const dir = plugins({ + typo: manifest( + "typo", + ["cloud"], + `slots: { "shell.product-swicher": () => import("./X") },`, + ), + }); + await assert.rejects(check(dir), /typo.*shell\.product-swicher/); +}); + +test("a distribution no build offers fails, naming the plugin", async () => { + const dir = plugins({ stray: manifest("stray", ["cloud", "onprem-x"]) }); + await assert.rejects(check(dir), /stray.*onprem-x/); +}); + +test("a folder without a manifest fails, but dotfiles are ignored", async () => { + const dir = plugins({ billing: manifest("billing", ["cloud"]) }); + fs.mkdirSync(path.join(dir, ".cache")); + fs.writeFileSync(path.join(dir, "index.ts"), "export {};\n"); + assert.deepEqual((await check(dir)).distributions.cloud, ["billing"]); + fs.mkdirSync(path.join(dir, "empty")); + await assert.rejects(check(dir), /empty\/ has no manifest\.ts/); +}); + +const script = path.join(import.meta.dirname, "v1-plugins.mjs"); +const cli = (dir) => + spawnSync(process.execPath, [script, "--dir", dir], { encoding: "utf8" }); + +test("the CLI exits 1 on a conflict, naming both plugins", () => { + const slot = `slots: { "shell.product-switcher": () => import("./X") },`; + const result = cli( + plugins({ + a: manifest("a", ["cloud"], slot), + b: manifest("b", ["cloud"], slot), + }), + ); + assert.equal(result.status, 1); + assert.match(result.stderr, /shell\.product-switcher.*\ba\b.*\bb\b/); +}); + +test("the CLI exits 0 and prints the matrix on a clean directory", () => { + const result = cli(plugins({ billing: manifest("billing", ["cloud"]) })); + assert.equal(result.status, 0); + assert.deepEqual(JSON.parse(result.stdout).distributions.cloud, ["billing"]); +}); diff --git a/frontend/scripts/v1-plugins.mjs b/frontend/scripts/v1-plugins.mjs index ff6559cb22..f535ffc032 100755 --- a/frontend/scripts/v1-plugins.mjs +++ b/frontend/scripts/v1-plugins.mjs @@ -1,26 +1,54 @@ #!/usr/bin/env node /* * Build check for v1 plugins (src/v1/plugins//manifest.ts, copied in by - * the cloud overlay). Loads every manifest with Node or Bun type stripping, - * runs the registry's conflict checks for every distribution, and returns the + * the cloud overlay). Transpiles every manifest and the registry with esbuild + * (so it runs on any Node the frontend supports), runs the registry's conflict checks for every distribution, and returns the * distribution matrix. vite.config.js runs it in every build and publishes * the matrix as plugin-matrix.json. * - * node scripts/v1-plugins.mjs [out.json] check, print or write the matrix + * node scripts/v1-plugins.mjs [out.json] [--dir plugins] check, print or write the matrix */ import fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; -import { buildRegistry } from "../src/v1/lib/plugins/registry.ts"; +import { build, transform } from "esbuild"; const DEFAULT_DIR = path.join(import.meta.dirname, "../src/v1/plugins"); +const REGISTRY = path.join( + import.meta.dirname, + "../src/v1/lib/plugins/registry.ts", +); + +const dataUrl = (code) => + `data:text/javascript;base64,${Buffer.from(code).toString("base64")}`; + +let registry; +function loadRegistry() { + registry ??= build({ + entryPoints: [REGISTRY], + bundle: true, + write: false, + format: "esm", + platform: "node", + }).then((r) => import(dataUrl(r.outputFiles[0].text))); + return registry; +} + +async function importManifest(file) { + const { code } = await transform(fs.readFileSync(file, "utf8"), { + loader: "ts", + format: "esm", + sourcefile: file, + }); + return import(dataUrl(code)); +} async function load(dir, id) { const file = path.join(dir, id, "manifest.ts"); const rel = `${id}/manifest.ts`; let mod; try { - mod = await import(pathToFileURL(file).href); + mod = await importManifest(file); } catch (err) { throw new Error( `${rel} could not be loaded (${err.message}). A manifest may only ` + @@ -43,11 +71,17 @@ export async function check(dir = DEFAULT_DIR) { const ids = fs.existsSync(dir) ? fs .readdirSync(dir, { withFileTypes: true }) - .filter((e) => fs.existsSync(path.join(dir, e.name, "manifest.ts"))) + .filter((e) => e.isDirectory() && !e.name.startsWith(".")) .map((e) => e.name) .sort() : []; + for (const id of ids) { + if (!fs.existsSync(path.join(dir, id, "manifest.ts"))) { + throw new Error(`${id}/ has no manifest.ts`); + } + } const manifests = await Promise.all(ids.map((id) => load(dir, id))); + const { buildRegistry } = await loadRegistry(); // Throws on any conflict in any distribution. const offered = (d) => [...buildRegistry(manifests, d).plugins]; return { @@ -67,11 +101,18 @@ export async function check(dir = DEFAULT_DIR) { // No top-level await: vite.config.js imports this file and Vite bundles the // config as CommonJS. -if (import.meta.url === pathToFileURL(process.argv[1]).href) { - check() +const isMain = + process.argv[1] !== undefined && + import.meta.url === pathToFileURL(fs.realpathSync(process.argv[1])).href; + +if (isMain) { + const args = process.argv.slice(2); + const flag = args.indexOf("--dir"); + const dir = flag === -1 ? DEFAULT_DIR : path.resolve(args[flag + 1]); + const out = args.find((a, i) => !a.startsWith("--") && i !== flag + 1); + check(dir) .then((matrix) => { const json = `${JSON.stringify(matrix, null, 2)}\n`; - const out = process.argv[2]; if (out) { fs.writeFileSync(out, json); } else { diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 5b2af77e4b..c52150185e 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -46,6 +46,8 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, - `` shows the fallback when no plugin fills the slot, silently. A filler that fails to load or render shows the fallback and is reported; each slot has its own error boundary. - `VITE_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) picks the plugins the build offers. The frontend Dockerfile takes it as a build arg. - Every build runs `scripts/v1-plugins.mjs`. It fails on a slot filled twice or a route defined twice in any distribution, or a plugin with no distribution, and writes `plugin-matrix.json` to the build output. `node scripts/v1-plugins.mjs` prints the matrix. +- `plugin-matrix.json` lists, for `oss`, `cloud` and `onprem`, the plugin ids each build offers, and for every plugin its distributions, slots and routes. Release tooling reads it to see which plugins ship where. +- The check also fails on a slot id or distribution the contract does not define, and on a `plugins/` folder with no `manifest.ts`. - v0's optional-plugin stub skips `src/v1`, so a missing module in v1 fails the build. ## Scripts diff --git a/frontend/src/v1/lib/plugins/registry.test.ts b/frontend/src/v1/lib/plugins/registry.test.ts index cc8eb1ef6d..9b0babd318 100644 --- a/frontend/src/v1/lib/plugins/registry.test.ts +++ b/frontend/src/v1/lib/plugins/registry.test.ts @@ -94,4 +94,25 @@ describe("buildRegistry", () => { /plugin id switcher is used twice/, ); }); + + it("fails on a slot id no plugin contract defines, naming the plugin", () => { + const typo = { + id: "typo", + distributions: ["cloud"], + slots: { "shell.product-swicher": load }, + } as unknown as PluginManifest; + expect(() => buildRegistry([typo], "cloud")).toThrow(PluginConflictError); + expect(() => buildRegistry([typo], "cloud")).toThrow( + /typo.*shell\.product-swicher/, + ); + }); + + it("fails on a distribution no build offers, naming the plugin", () => { + const stray = { + id: "stray", + distributions: ["cloud", "onprm"], + } as unknown as PluginManifest; + expect(() => buildRegistry([stray], "cloud")).toThrow(PluginConflictError); + expect(() => buildRegistry([stray], "cloud")).toThrow(/stray.*onprm/); + }); }); diff --git a/frontend/src/v1/lib/plugins/registry.ts b/frontend/src/v1/lib/plugins/registry.ts index 3241f78406..fc7f0c5e46 100644 --- a/frontend/src/v1/lib/plugins/registry.ts +++ b/frontend/src/v1/lib/plugins/registry.ts @@ -1,18 +1,14 @@ -import type { - ComponentLoader, - Distribution, - PluginDistribution, - PluginManifest, - PluginRoute, - SlotId, - SlotProps, +import { + type ComponentLoader, + type Distribution, + PLUGIN_DISTRIBUTIONS, + type PluginManifest, + type PluginRoute, + SLOT_IDS, + type SlotId, + type SlotProps, } from "./types"; -/* - * Runtime imports stay at zero: scripts/v1-plugins.mjs loads this file with - * Node or Bun type stripping, so it may only `import type`. - */ - export class PluginConflictError extends Error { override name = "PluginConflictError"; } @@ -33,8 +29,6 @@ export interface PluginRegistry { slot(id: K): SlotEntry | undefined; } -const PLUGIN_DISTRIBUTIONS: readonly PluginDistribution[] = ["cloud", "onprem"]; - function assertDeclared(manifests: readonly PluginManifest[]): void { const seen = new Set(); for (const m of manifests) { @@ -45,6 +39,18 @@ function assertDeclared(manifests: readonly PluginManifest[]): void { if (m.distributions.length === 0) { throw new PluginConflictError(`${m.id} declares no distributions`); } + for (const d of m.distributions as readonly string[]) { + if (!(PLUGIN_DISTRIBUTIONS as readonly string[]).includes(d)) { + throw new PluginConflictError( + `${m.id} declares unknown distribution ${d}`, + ); + } + } + for (const id of Object.keys(m.slots ?? {})) { + if (!(SLOT_IDS as readonly string[]).includes(id)) { + throw new PluginConflictError(`${m.id} fills unknown slot ${id}`); + } + } } } @@ -88,7 +94,8 @@ function collect( /** * The plugins, slots and routes one distribution offers. Throws * PluginConflictError when any distribution would have a slot filled twice, - * a route defined twice, or a plugin that declares nowhere to ship. + * a route defined twice, a plugin that declares nowhere to ship, or a + * manifest naming a slot or distribution that does not exist. */ export function buildRegistry( manifests: readonly PluginManifest[], diff --git a/frontend/src/v1/lib/plugins/types.ts b/frontend/src/v1/lib/plugins/types.ts index e10fa67c46..0181973ae3 100644 --- a/frontend/src/v1/lib/plugins/types.ts +++ b/frontend/src/v1/lib/plugins/types.ts @@ -1,34 +1,36 @@ import type { ComponentType } from "react"; /* - * Type-only module. Plugin manifests import from here with `import type`, so - * Node and Bun can load a manifest without bundling it (the build check in - * scripts/v1-plugins.mjs does exactly that). + * Plugin manifests import from here with `import type`, so the build check in + * scripts/v1-plugins.mjs can load a manifest without bundling it. The runtime + * lists below let the registry reject a value tsc never saw. */ /** Builds a cloud plugin can ship in. The public OSS build has no plugins. */ -export type PluginDistribution = "cloud" | "onprem"; +export const PLUGIN_DISTRIBUTIONS = ["cloud", "onprem"] as const; +export type PluginDistribution = (typeof PLUGIN_DISTRIBUTIONS)[number]; export type Distribution = "oss" | PluginDistribution; +/** Every extension point. Adding a slot is one line here. */ +export const SLOT_IDS = [ + "prompt-studio.lookups", + "prompt-studio.cloud-tools", + "workflows.human-review", + "account-menu.plan-billing", + "shell.product-switcher", + "platform.hitl-keys", + "platform.hitl-settings", + "dashboard.subscription-usage", + "dashboard.attention-items", +] as const; +export type SlotId = (typeof SLOT_IDS)[number]; + /* - * Every extension point and its props. A filler receives these props; the - * host passes them. Adding a slot is one line here. + * The props a filler receives; the host passes them. * ponytail: props are empty until the host page that renders each slot is * built; that task adds the fields it passes. */ -export interface SlotProps { - "prompt-studio.lookups": Record; - "prompt-studio.cloud-tools": Record; - "workflows.human-review": Record; - "account-menu.plan-billing": Record; - "shell.product-switcher": Record; - "platform.hitl-keys": Record; - "platform.hitl-settings": Record; - "dashboard.subscription-usage": Record; - "dashboard.attention-items": Record; -} - -export type SlotId = keyof SlotProps; +export type SlotProps = { [K in SlotId]: Record }; export type ComponentLoader

= () => Promise<{ default: ComponentType

}>; From 927f4e00a9b46c3a23bbde4aecbbf1d0f192274e Mon Sep 17 00:00:00 2001 From: Athul Date: Wed, 28 Oct 2026 12:27:44 +0530 Subject: [PATCH 14/26] UN-4286 [FIX] Load esbuild only under Node, so the cloud overlay runs the check before installing dependencies Co-Authored-By: Claude Opus 5.5 --- frontend/scripts/v1-plugins.mjs | 36 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/frontend/scripts/v1-plugins.mjs b/frontend/scripts/v1-plugins.mjs index f535ffc032..bc717ea41b 100755 --- a/frontend/scripts/v1-plugins.mjs +++ b/frontend/scripts/v1-plugins.mjs @@ -1,9 +1,10 @@ #!/usr/bin/env node /* * Build check for v1 plugins (src/v1/plugins//manifest.ts, copied in by - * the cloud overlay). Transpiles every manifest and the registry with esbuild - * (so it runs on any Node the frontend supports), runs the registry's conflict checks for every distribution, and returns the - * distribution matrix. vite.config.js runs it in every build and publishes + * the cloud overlay). Under Bun it imports the TypeScript directly; under Node + * it transpiles with esbuild, loaded only then, because the cloud overlay runs + * this with Bun before any dependency is installed. Runs the registry's + * conflict checks for every distribution and returns the distribution matrix. vite.config.js runs it in every build and publishes * the matrix as plugin-matrix.json. * * node scripts/v1-plugins.mjs [out.json] [--dir plugins] check, print or write the matrix @@ -11,7 +12,6 @@ import fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; -import { build, transform } from "esbuild"; const DEFAULT_DIR = path.join(import.meta.dirname, "../src/v1/plugins"); const REGISTRY = path.join( @@ -22,19 +22,33 @@ const REGISTRY = path.join( const dataUrl = (code) => `data:text/javascript;base64,${Buffer.from(code).toString("base64")}`; +const BUN = Boolean(process.versions.bun); + let registry; function loadRegistry() { - registry ??= build({ - entryPoints: [REGISTRY], - bundle: true, - write: false, - format: "esm", - platform: "node", - }).then((r) => import(dataUrl(r.outputFiles[0].text))); + if (BUN) { + registry ??= import(pathToFileURL(REGISTRY).href); + return registry; + } + registry ??= import("esbuild") + .then(({ build }) => + build({ + entryPoints: [REGISTRY], + bundle: true, + write: false, + format: "esm", + platform: "node", + }), + ) + .then((r) => import(dataUrl(r.outputFiles[0].text))); return registry; } async function importManifest(file) { + if (BUN) { + return import(pathToFileURL(file).href); + } + const { transform } = await import("esbuild"); const { code } = await transform(fs.readFileSync(file, "utf8"), { loader: "ts", format: "esm", From 664d1cfedffbd0a1230bb37f875bf281c442ee68 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Thu, 29 Oct 2026 10:38:43 +0530 Subject: [PATCH 15/26] fix(frontend): typecheck v1 tests that import vitest; skip cloud plugins in the ignore check (UN-4270) Reported by Athul while building #2394/#2395 on this branch: - src/v1/tsconfig.json: skipLibCheck back on, as in tsconfig.json. Off, tsc also checked third-party typings, and @types/chai 5.2.3 and vitest 3.2.6 both declare `containSubset`, so any v1 test importing vitest failed typecheck:v1. A fixture test now imports vitest to keep this covered. - The "no v1 file is git-ignored" check skips src/v1/plugins//: cloud plugins copied there are git-ignored on purpose (#2394). --- frontend/src/v1/tsconfig.json | 8 +++++--- frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs | 5 ++++- .../v1-allowed-imports/src/v1/components/ui/Label.test.ts | 6 +++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/frontend/src/v1/tsconfig.json b/frontend/src/v1/tsconfig.json index 0a10f9dbdb..89bfe6e386 100644 --- a/frontend/src/v1/tsconfig.json +++ b/frontend/src/v1/tsconfig.json @@ -17,9 +17,11 @@ "allowJs": false, "noEmit": true, - /* Off so hand-written .d.ts under v1 are checked too; with it on, an - * unresolved name in one silently becomes `any`. */ - "skipLibCheck": false, + /* On, as in tsconfig.json. Off also type-checks third-party typings, and + * those conflict: @types/chai 5.2.3 and vitest 3.2.6 both declare + * `containSubset`, so any v1 test importing vitest failed. The cost is + * that hand-written .d.ts files under v1 are not checked either. */ + "skipLibCheck": true, "esModuleInterop": true, "resolveJsonModule": true, "isolatedModules": true, diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index c530319d89..62c9d58f6b 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -188,7 +188,10 @@ test("no v1 or fixture file is git-ignored", () => { withFileTypes: true, }) .filter((d) => d.isFile() && d.name !== ".DS_Store") - .map((d) => path.relative(FRONTEND, path.join(d.parentPath, d.name))), + .map((d) => path.relative(FRONTEND, path.join(d.parentPath, d.name))) + // Cloud plugins copied into src/v1/plugins// are git-ignored on + // purpose; they belong to the cloud repository. + .filter((rel) => !/^src\/v1\/plugins\/[^/]+\//.test(rel)), ); // A lib/ folder anywhere under v1, not only the top-level one. files.push("src/v1/pages/projects/lib/x.ts", "src/v1/lib/sub/lib/x.ts"); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts index 1cb7e9a430..feb52d7f4f 100644 --- a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Label.test.ts @@ -1,4 +1,8 @@ import { renderLabel } from "@v1/test/render"; +import { expect, it } from "vitest"; import { label } from "./Label"; -export const rendered = renderLabel(label("ok")); +// Importing vitest here keeps typecheck:v1 honest about test typings. +it("renders", () => { + expect(renderLabel(label("ok"))).toBe("OK"); +}); From 8ede7eccef88e6a3c28f0d20823539c6c0a6de40 Mon Sep 17 00:00:00 2001 From: Athul Date: Thu, 29 Oct 2026 10:48:55 +0530 Subject: [PATCH 16/26] UN-4286 [FIX] Write the plugin matrix to a file given without --dir, and test slot recovery in place - v1-plugins.mjs: with no --dir, `flag + 1` was 0, so the first argument (the output file) was skipped and the matrix went to stdout. A fixture now covers the file argument with and without --dir. - Slot.test.tsx: the "recovers in place" test wrapped the slot in a

, which remounted it, so the boundary's resetKey reset never ran. It now re-renders the same slot; with resetKey={id} the test fails. Co-Authored-By: Claude Opus 5.5 --- frontend/scripts/v1-plugins.fixtures.mjs | 19 +++++++++++++++++++ frontend/scripts/v1-plugins.mjs | 4 +++- frontend/src/v1/lib/plugins/Slot.test.tsx | 10 ++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/frontend/scripts/v1-plugins.fixtures.mjs b/frontend/scripts/v1-plugins.fixtures.mjs index 4b982fbe9b..b58812f5ff 100644 --- a/frontend/scripts/v1-plugins.fixtures.mjs +++ b/frontend/scripts/v1-plugins.fixtures.mjs @@ -135,3 +135,22 @@ test("the CLI exits 0 and prints the matrix on a clean directory", () => { assert.equal(result.status, 0); assert.deepEqual(JSON.parse(result.stdout).distributions.cloud, ["billing"]); }); + +test("the CLI writes the matrix to the file argument, with or without --dir", () => { + const out = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), "v1-out-")), + "m.json", + ); + const bare = spawnSync(process.execPath, [script, out], { encoding: "utf8" }); + assert.equal(bare.status, 0, bare.stderr); + assert.equal(bare.stdout, ""); + assert.ok(JSON.parse(fs.readFileSync(out, "utf8")).distributions); + + const dir = plugins({ billing: manifest("billing", ["cloud"]) }); + const before = spawnSync(process.execPath, [script, out, "--dir", dir], { + encoding: "utf8", + }); + assert.equal(before.status, 0, before.stderr); + const matrix = JSON.parse(fs.readFileSync(out, "utf8")); + assert.deepEqual(matrix.distributions.cloud, ["billing"]); +}); diff --git a/frontend/scripts/v1-plugins.mjs b/frontend/scripts/v1-plugins.mjs index bc717ea41b..7009b4bcdc 100755 --- a/frontend/scripts/v1-plugins.mjs +++ b/frontend/scripts/v1-plugins.mjs @@ -123,7 +123,9 @@ if (isMain) { const args = process.argv.slice(2); const flag = args.indexOf("--dir"); const dir = flag === -1 ? DEFAULT_DIR : path.resolve(args[flag + 1]); - const out = args.find((a, i) => !a.startsWith("--") && i !== flag + 1); + const out = args.find( + (a, i) => !a.startsWith("--") && (flag === -1 || i !== flag + 1), + ); check(dir) .then((matrix) => { const json = `${JSON.stringify(matrix, null, 2)}\n`; diff --git a/frontend/src/v1/lib/plugins/Slot.test.tsx b/frontend/src/v1/lib/plugins/Slot.test.tsx index 9a39f11be5..b5f3bf94fb 100644 --- a/frontend/src/v1/lib/plugins/Slot.test.tsx +++ b/frontend/src/v1/lib/plugins/Slot.test.tsx @@ -137,13 +137,15 @@ describe("Slot", () => { slots: { "shell.product-switcher": load }, }, ]); - const tree = ( + // A fresh element each time, in the same position: React re-renders the + // mounted slot and its boundary instead of mounting new ones. + const tree = () => ( ); - const { rerender } = render(tree); + const { rerender } = render(tree()); await vi.waitFor(() => expect(report).toHaveBeenCalledTimes(1)); - rerender(
{tree}
); - rerender(tree); + rerender(tree()); expect(await screen.findByText("switcher")).toBeInTheDocument(); + expect(load).toHaveBeenCalledTimes(2); }); }); From 787bf8be4e2a11c5a96b620a0ea515c1b0599142 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Thu, 29 Oct 2026 10:53:12 +0530 Subject: [PATCH 17/26] fix(frontend): close v1 guardrail gaps from team review (UN-4270) From Ali's, Athul's and Vishnu's reviews on #2380, each reproduced first: - tsconfig/jsconfig map `@v1/*` too: once v0 imports the entry, tsc follows it into src/v1, where cross-folder `@v1/...` imports must resolve (would break `bun run typecheck` at UN-4271). Lint keeps v0 on `@v1`. - check-v1 scanner tracks regex literals, so a `/*` inside one no longer hides the rest of the file; CSS blanks only block comments (`//` is URL). - Imports are read by argument: literals, templates by their fixed prefix, every glob pattern, and new URL(..., import.meta.url). In v1 an import the guard cannot read fails closed; a computed v1 path is allowed only in folders that may import everything (the lazy route registry in app/). - v0 specifiers are normalized before the check (/src/v1, @/./v1, ...). - v1 styles may not @import antd packages. - Biome's v0 rule matches only alias spellings of src/v1, so `uuid/v1` or `@/api/v1` no longer fail; relative spellings are check-v1's (in CI). - CI runs `test:v1` (fixtures) on Node 22; setup-biome pinned to a SHA (SonarCloud S7637). Fixtures assert the exact message and count per form, cover each fix, and check that comments, JSX and look-alike class names stay quiet. Against the previous guard, 9 of the new tests fail. --- .github/workflows/ci-frontend-lint.yaml | 21 +- frontend/biome.json | 11 +- frontend/jsconfig.json | 3 +- frontend/scripts/check-v1.mjs | 377 +++++++++++++----- frontend/src/v1/README.md | 10 +- .../v1-boundary/boundary.fixtures.mjs | 145 +++++-- .../src/pages/Lookalike.jsx | 4 + .../v0-tsx-imports-entry/src/V1Switch.tsx | 3 + .../v0-tsx-imports-entry/src/v1/app/mount.ts | 1 + .../v0-tsx-imports-entry/src/v1/index.ts | 1 + .../src/v1/styles/global.css | 1 + .../src/v1/styles/global.css | 1 + .../v1-unanalysable-import/src/v1/lib/load.ts | 1 + frontend/tsconfig.json | 7 +- frontend/vite.config.js | 4 +- 15 files changed, 448 insertions(+), 142 deletions(-) create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-lookalike/src/pages/Lookalike.jsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/V1Switch.tsx create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/app/mount.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/index.ts create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-after-url/src/v1/styles/global.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-styles-import-antd/src/v1/styles/global.css create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-unanalysable-import/src/v1/lib/load.ts diff --git a/.github/workflows/ci-frontend-lint.yaml b/.github/workflows/ci-frontend-lint.yaml index 2b5fbac9c0..cc65d2c073 100644 --- a/.github/workflows/ci-frontend-lint.yaml +++ b/.github/workflows/ci-frontend-lint.yaml @@ -50,7 +50,7 @@ jobs: uses: actions/checkout@v6 - name: Setup Biome - uses: biomejs/setup-biome@v2 + uses: biomejs/setup-biome@4c91541eaada48f67d7dbd7833600ce162b68f51 # v2.7.1 with: version: "2.3.13" @@ -122,17 +122,30 @@ jobs: run: bun run typecheck # src/v1 is excluded from the step above and checked under its own - # stricter tsconfig, after scripts/check-v1.mjs rejects non-TS files. + # stricter tsconfig, after scripts/check-v1.mjs checks file types, + # placement, symlinks and the folder taxonomy. - name: Run v1 typecheck working-directory: frontend run: bun run typecheck:v1 - # Biome on src/v1, then the folder import contract, PropTypes and `ant-` - # checks in scripts/check-v1.mjs, which `biome ci` cannot express. + # Biome on src/v1, then scripts/check-v1.mjs: the folder import + # contract, v0 reaching v1 only through `@v1`, PropTypes and `ant-`. - name: Run v1 lint working-directory: frontend run: bun run lint:v1 + # The guardrail fixtures run under `node --test` (Node 22 APIs). + - name: Setup Node + uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: "22" + + # v1 unit tests, then the fixtures proving each guard above fails and + # passes when it should. + - name: Run v1 tests + working-directory: frontend + run: bun run test:v1 + # ci-test.yaml runs the same suite as the rig's `frontend` group, but its # tiers skip frontend-only PRs; this job is what gates those. vitest: diff --git a/frontend/biome.json b/frontend/biome.json index 6799918af6..8d9f1d1ff3 100644 --- a/frontend/biome.json +++ b/frontend/biome.json @@ -187,7 +187,16 @@ "options": { "patterns": [ { - "group": ["@v1/**", "**/v1", "**/v1/**", "!@v1"], + "group": [ + "@v1/**", + "@/v1", + "@/v1/**", + "src/v1", + "src/v1/**", + "/src/v1", + "/src/v1/**", + "!@v1" + ], "message": "v0 may import only the v1 public entry, as '@v1'." } ] diff --git a/frontend/jsconfig.json b/frontend/jsconfig.json index 63ea90da3d..d2e095ab6b 100644 --- a/frontend/jsconfig.json +++ b/frontend/jsconfig.json @@ -3,7 +3,8 @@ "baseUrl": ".", "paths": { "@/*": ["./src/*"], - "@v1": ["./src/v1/index.ts"] + "@v1": ["./src/v1/index.ts"], + "@v1/*": ["./src/v1/*"] } }, "include": ["src/**/*"], diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index 3e842480c4..649f8307a3 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -3,23 +3,28 @@ * Guard for the v1 frontend (src/v1/): the rules Biome cannot express. * * --files TypeScript only (a .js under v1 is skipped by tsc, not failed, - * because allowJs is off), style files only in styles/, and every - * file inside the v1 folder taxonomy. Run by `typecheck:v1`. + * because allowJs is off), style files only in styles/, no + * symlinks, and every file inside the v1 folder taxonomy. Run by + * `typecheck:v1`. * --code Imports: nothing outside src/v1 and the folder import contract * (which v1 folder may import which). No PropTypes, no antd `ant-` * class names. Also scans v0 (the rest of src/) for imports that * reach into v1 other than through `@v1`. Run by `lint:v1`. * - * With neither flag, both run. It always checks the src/v1 beside this - * script; the fixtures in test-fixtures/v1-boundary/ copy it into each - * throwaway tree for that reason. + * With neither flag, both run. It checks the src/ beside this script; the + * fixtures in test-fixtures/v1-boundary/ copy it into each throwaway tree for + * that reason. + * + * Imports are read from static import/export statements and from the calls + * Vite resolves: import(), require(), import.meta.glob() and + * new URL(..., import.meta.url). A template argument is checked by its fixed + * prefix. In v1, an argument that is neither (a variable, say) is reported as + * unanalysable rather than passed. * * The v0 <-> v1 boundary is also a Biome rule (biome.json `overrides`), so CI - * and editors report it. The script repeats both halves because it also reads - * template-literal imports, Vite globs, require() and CSS @import, which - * Biome does not. Biome overrides - * replace rule options rather than merge them, which is why the per-folder - * contract lives here and not as one override per folder. + * and editors report the plain forms. Biome overrides replace rule options + * rather than merge them, which is why the per-folder contract lives here and + * not as one override per folder. * * The `ant-` check exists only while antd markup survives in v0; drop it * once the shadcn migration is complete. @@ -97,6 +102,10 @@ const STYLE_EXT = /\.(css|scss|sass|less)$/; const TEST_FILE = /\.(test|spec)\.(ts|tsx|mts|cts)$/; // Specifiers that leave src/v1: root-absolute, parent, the v0 alias, src/. const OUTSIDE = /^(\/|\.\.(\/|$)|@\/|src\/)/; +// The v1 public entry, in the spellings that resolve to src/v1/index.ts. +const ENTRY_SPEC = /^@v1(\/index(\.ts)?)?$/; +// antd packages, which v1 styles may not pull in either. +const ANTD = /^(antd|@ant-design)(\/|$)/; /* * The v1 folder a path (relative to src/v1, `/`-separated) belongs to, e.g. @@ -144,10 +153,9 @@ function mayImport(from, to, isTestFile) { /* * Blanks comments, keeping line numbers so matches can be reported. Tracks - * strings and template literals (including `${}` expressions), so a `/*` or - * `//` inside a string does not swallow the code after it. Regex literals are - * not tracked; a quote inside one is read as a string to the end of the line - * at most, so it can hide that line only. + * strings, template literals (including `${}` expressions) and regex + * literals, so a `/*` or `//` inside any of them does not swallow the code + * after it. A regex literal is told from division by the token before it. */ function blankComments(src) { let out = ""; @@ -155,26 +163,100 @@ function blankComments(src) { // "`" = inside a template literal; "{" = inside a `${}` or a brace in one. const stack = []; while (i < src.length) { - let end; if (stack.at(-1) === "`") { - end = i + templateStep(src, i, stack); - } else if (src[i] === "/" && (src[i + 1] === "/" || src[i + 1] === "*")) { - end = commentEnd(src, i); - out += src.slice(i, end).replace(/[^\n]/g, " "); + const end = i + templateStep(src, i, stack); + out += src.slice(i, end); i = end; continue; - } else if (src[i] === '"' || src[i] === "'") { - end = stringEnd(src, i); - } else { - codeStep(src[i], stack); - end = i + 1; } - out += src.slice(i, end); + const end = tokenEnd(src, i, out, stack); + const token = src.slice(i, end); + out += isComment(src, i) ? token.replace(/[^\n]/g, " ") : token; i = end; } return out; } +/* CSS has block comments only; a `//` there is part of a URL. */ +function blankCssComments(src) { + return src.replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " ")); +} + +function isComment(src, i) { + return src[i] === "/" && (src[i + 1] === "/" || src[i + 1] === "*"); +} + +/* Index just past the code token that starts at i. */ +function tokenEnd(src, i, out, stack) { + if (isComment(src, i)) { + return commentEnd(src, i); + } + if (src[i] === '"' || src[i] === "'") { + return stringEnd(src, i); + } + if (src[i] === "/" && regexMayStart(out)) { + const end = regexEnd(src, i); + if (end !== -1) { + return end; + } + } + codeStep(src[i], stack); + return i + 1; +} + +// A `/` after one of these (or a keyword below) starts a regex, not division. +// `<` is left out so JSX closing tags (`
`) stay code. +const REGEX_AFTER = new Set("(,=:[!&|?{};+-*%>~^"); +const REGEX_KEYWORDS = new Set([ + "return", + "typeof", + "case", + "do", + "else", + "in", + "of", + "void", + "yield", + "await", + "delete", + "new", +]); + +function regexMayStart(out) { + const before = out.trimEnd(); + if (before === "") { + return true; + } + const last = before.at(-1); + if (REGEX_AFTER.has(last)) { + return true; + } + const word = /[\w$]+$/.exec(before); + return word !== null && REGEX_KEYWORDS.has(word[0]); +} + +/* Index just past the regex literal at i, or -1 if it does not close on its + * line (then it was division after all). */ +function regexEnd(src, i) { + let inClass = false; + for (let j = i + 1; j < src.length; j += 1) { + const c = src[j]; + if (c === "\n") { + return -1; + } + if (c === "\\") { + j += 1; + } else if (c === "[") { + inClass = true; + } else if (c === "]") { + inClass = false; + } else if (c === "/" && !inClass) { + return j + 1 + (/^[a-z]*/i.exec(src.slice(j + 1))?.[0].length ?? 0); + } + } + return -1; +} + /* Index just past the line or block comment that starts at i. */ function commentEnd(src, i) { if (src[i + 1] === "/") { @@ -223,39 +305,83 @@ function codeStep(c, stack) { } } -// A specifier: a quoted or backtick string with no interpolation. -const SPEC = "[\"'`]([^\"'`$]+)[\"'`]"; -const IMPORT_RES = [ - new RegExp( - String.raw`\b(?:import|export)\s+(?:type\s+)?(?:[\w$*{}\s,]+?\s+from\s+)?${SPEC}`, - "g", - ), - new RegExp(String.raw`\bimport\s*\(\s*${SPEC}\s*[,)]`, "g"), - new RegExp(String.raw`\brequire\s*\(\s*${SPEC}\s*\)`, "g"), -]; -// Vite globs: one pattern or an array of them. Every pattern is checked. -const GLOB_RE = new RegExp( - String.raw`\bimport\.meta\.glob\s*(?:<[^>]*>)?\(\s*(\[[^\]]*\]|${SPEC})`, - "g", -); -const SPEC_RE = new RegExp(SPEC, "g"); - -/* Every import specifier in comment-blanked source, with its offset. */ +// Static import/export statements; these take a plain string literal. +const STATIC_RE = + /\b(?:import|export)\s+(?:type\s+)?(?:[\w$*{}\s,]+?\s+from\s+)?["']([^"']+)["']/g; +// Calls whose first argument Vite resolves as a module or asset. +const CALL_RE = + /\b(import\.meta\.glob|import|require|new\s+URL)\s*(?:<[^>]*>)?\(\s*/g; +// CSS `@import` and `url()` targets. +const CSS_IMPORT_RE = /(?:@import\s+(?:url\(\s*)?|url\(\s*)["']?([^"')\s;]+)/g; + +/* + * The argument at j: { kind: "literal", spec } for a plain string; + * { kind: "prefix", spec } for a template with `${}`, spec being the fixed + * part before it; { kind: "opaque" } for anything else. `end` is the index + * just past a string argument. + */ +function readArg(text, j) { + const quote = text[j]; + if (quote !== '"' && quote !== "'" && quote !== "`") { + return { kind: "opaque" }; + } + const end = text.indexOf(quote, j + 1); + if (end === -1) { + return { kind: "opaque" }; + } + const body = text.slice(j + 1, end); + const hole = quote === "`" ? body.indexOf("${") : -1; + return hole === -1 + ? { kind: "literal", spec: body, end: end + 1 } + : { kind: "prefix", spec: body.slice(0, hole), end: end + 1 }; +} + +/* Every import in comment-blanked source: { kind, spec, index }. */ function* importSpecs(text) { - for (const re of IMPORT_RES) { - for (const m of text.matchAll(re)) { - yield { spec: m[1], index: m.index }; + for (const m of text.matchAll(STATIC_RE)) { + yield { kind: "literal", spec: m[1], index: m.index }; + } + for (const m of text.matchAll(CALL_RE)) { + yield* callSpecs(text, m); + } +} + +function* callSpecs(text, m) { + const j = m.index + m[0].length; + if (m[1] === "import.meta.glob" && text[j] === "[") { + yield* globArraySpecs(text, j + 1); + return; + } + const arg = readArg(text, j); + if (m[1].startsWith("new")) { + // Only `new URL(x, import.meta.url)` is a module reference. + const base = arg.end === undefined ? "" : text.slice(arg.end, arg.end + 40); + if (!/^\s*,\s*import\.meta\.url\b/.test(base)) { + return; } } - for (const m of text.matchAll(GLOB_RE)) { - const start = m.index + m[0].indexOf(m[1]); - for (const s of m[1].matchAll(SPEC_RE)) { - yield { spec: s[1], index: start + s.index }; + yield { ...arg, index: m.index }; +} + +/* Every pattern of an `import.meta.glob([...])` array; `!` patterns exclude + * files rather than import them, so they are skipped. */ +function* globArraySpecs(text, j) { + let k = j; + while (k < text.length) { + k += /^[\s,]*/.exec(text.slice(k))[0].length; + if (text[k] === "]") { + return; } + const arg = readArg(text, k); + if (arg.kind !== "literal" || !arg.spec.startsWith("!")) { + yield { ...arg, index: k }; + } + if (arg.end === undefined) { + return; + } + k = arg.end; } } -// CSS `@import` and `url()` targets, for the files in styles/. -const CSS_IMPORT_RE = /(?:@import\s+(?:url\(\s*)?|url\(\s*)["']?([^"')\s;]+)/g; /* * Classifies a specifier: "outside" when it leaves src/v1; the v1 folder it @@ -265,7 +391,7 @@ function targetFolder(spec, fileRel) { if (OUTSIDE.test(spec)) { return "outside"; } - if (spec === "@v1" || /^@v1\/index(\.ts)?$/.test(spec)) { + if (ENTRY_SPEC.test(spec)) { return "entry"; } let rel; @@ -281,7 +407,57 @@ function targetFolder(spec, fileRel) { } // No extension on the last segment -> it may name a folder ("@v1/lib"). const isDir = !path.posix.extname(rel); - return folderOf(rel, isDir) ?? folderOf(rel) ?? null; + return folderOf(rel, isDir) ?? folderOf(rel); +} + +/* + * Classifies a template's fixed prefix like targetFolder, by the directory it + * fixes (up to its last "/"). "any" when that leaves the v1 folder open, as in + * `@v1/pages/${name}`; only folders that may import everything may do that. + */ +function prefixFolder(prefix, fileRel) { + if (OUTSIDE.test(prefix)) { + return "outside"; + } + if (!prefix.startsWith("@v1") && !prefix.startsWith("./")) { + return undefined; + } + const dir = prefix.slice(0, prefix.lastIndexOf("/") + 1); + const rel = dir.startsWith("@v1/") + ? path.posix.normalize(dir.slice(4)) + : path.posix.join(path.posix.dirname(fileRel), dir); + if (rel.startsWith("..")) { + return "outside"; + } + const isRoot = rel === "" || rel === "." || rel === "./"; + return (isRoot ? null : folderOf(rel, true)) ?? "any"; +} + +/* The problem with one v1 import, or null. */ +function v1ImportProblem(folder, rel, arg, isTestFile) { + if (arg.kind === "opaque" || (arg.kind === "prefix" && arg.spec === "")) { + return "unanalysable import; use a string literal, or a template whose fixed prefix names the folder."; + } + const { spec } = arg; + const to = + arg.kind === "prefix" ? prefixFolder(spec, rel) : targetFolder(spec, rel); + if (to === undefined) { + return null; + } + if (to === "outside") { + return `'${spec}' is outside src/v1; v1 imports nothing outside it.`; + } + if (to === null) { + return `'${spec}' is outside the v1 folder taxonomy.`; + } + if (to === "any") { + return CONTRACT[contractKey(folder)] === ALL + ? null + : `${folder}/ may not import a computed v1 path ('${spec}...'); only app/, plugins/, test/ and the entry may.`; + } + return mayImport(folder, to, isTestFile) + ? null + : `${folder}/ may not import from ${to}/ ('${spec}'); see the folder contract in src/v1/README.md.`; } function lineAt(text, index) { @@ -291,8 +467,8 @@ function lineAt(text, index) { // Finder litter; never committed (.gitignore), so not worth reporting. const IGNORED = new Set([".DS_Store"]); -/* Every file under dir, plus symlinks, which are reported rather than - * followed: a linked file would bring outside code in unchecked. */ +/* Every file under dir, plus symlinks, which are listed rather than + * followed; callers decide what to do with them. */ function walk(dir) { return fs .readdirSync(dir, { recursive: true, withFileTypes: true }) @@ -345,9 +521,11 @@ for (const rel of walk(ROOT)) { } if (runCode && STYLE_EXT.test(name) && folder === "styles") { - const css = blankComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); + const css = blankCssComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); for (const m of css.matchAll(CSS_IMPORT_RE)) { - if (targetFolder(m[1], rel) === "outside") { + if (ANTD.test(m[1])) { + report(rel, lineAt(css, m.index), "antd is not used under src/v1."); + } else if (targetFolder(m[1], rel) === "outside") { report( rel, lineAt(css, m.index), @@ -366,26 +544,10 @@ for (const rel of walk(ROOT)) { const text = blankComments(fs.readFileSync(path.join(ROOT, rel), "utf8")); const isTestFile = TEST_FILE.test(name); - for (const { spec, index } of importSpecs(text)) { - const to = targetFolder(spec, rel); - const line = lineAt(text, index); - if (to === undefined) { - continue; - } - if (to === "outside") { - report( - rel, - line, - `'${spec}' is outside src/v1; v1 imports nothing outside it.`, - ); - } else if (to === null) { - report(rel, line, `'${spec}' is outside the v1 folder taxonomy.`); - } else if (!mayImport(folder, to, isTestFile)) { - report( - rel, - line, - `${folder}/ may not import from ${to}/ ('${spec}'); see the folder contract in src/v1/README.md.`, - ); + for (const arg of importSpecs(text)) { + const problem = v1ImportProblem(folder, rel, arg, isTestFile); + if (problem) { + report(rel, lineAt(text, arg.index), problem); } } for (const m of text.matchAll(/\.propTypes\b|\bpropTypes\s*[:=]/g)) { @@ -407,29 +569,58 @@ for (const rel of walk(ROOT)) { */ const V0_CODE_EXT = /\.(js|jsx|mjs|cjs|ts|tsx|mts|cts)$/; -/* Whether a v0 specifier (fileRel is relative to src/) reaches into v1. */ -function intoV1(spec, fileRel) { - if (spec === "@v1" || /^@v1\/index(\.ts)?$/.test(spec)) { +/* A v0 specifier as a path from the project root, or undefined for a + * package. fileRel is relative to src/. */ +function v0Target(spec, fileRel) { + if (spec.startsWith("@v1/")) { + return `src/v1/${spec.slice(4)}`; + } + if (spec.startsWith("@/")) { + return `src/${spec.slice(2)}`; + } + if (spec.startsWith("/")) { + return spec.slice(1); + } + if (spec.startsWith("src/")) { + return spec; + } + if (spec.startsWith(".")) { + return path.posix.join("src", path.posix.dirname(fileRel), spec); + } + return undefined; +} + +/* Whether a v0 import reaches into v1 other than through the `@v1` entry. + * A template is judged by the directory its fixed prefix names. */ +function intoV1(arg, fileRel) { + if (arg.kind === "opaque") { return false; } - if (spec.startsWith("@v1/") || /^(@\/|src\/)v1(\/|$)/.test(spec)) { - return true; + if (arg.kind === "literal" && ENTRY_SPEC.test(arg.spec)) { + return false; } - if (!spec.startsWith(".")) { + const target = v0Target(arg.spec, fileRel); + if (target === undefined) { return false; } - const rel = path.posix.join(path.posix.dirname(fileRel), spec); - return rel === "v1" || rel.startsWith("v1/"); + const norm = path.posix.normalize(target); + const fixed = + arg.kind === "prefix" ? norm.slice(0, norm.lastIndexOf("/") + 1) : norm; + return fixed === "src/v1" || fixed.startsWith("src/v1/"); } -function v0Specs(rel, text) { +function v0Specs(rel, raw) { if (!STYLE_EXT.test(rel)) { - return importSpecs(text); + const text = blankComments(raw); + return { text, specs: importSpecs(text) }; } - return [...text.matchAll(CSS_IMPORT_RE)].map((m) => ({ + const text = blankCssComments(raw); + const specs = [...text.matchAll(CSS_IMPORT_RE)].map((m) => ({ + kind: "literal", spec: m[1], index: m.index, })); + return { text, specs }; } function checkV0() { @@ -440,11 +631,11 @@ function checkV0() { if (inV1 || !scanned || fs.lstatSync(full).isSymbolicLink()) { continue; } - const text = blankComments(fs.readFileSync(full, "utf8")); - for (const { spec, index } of v0Specs(rel, text)) { - if (intoV1(spec, rel)) { + const { text, specs } = v0Specs(rel, fs.readFileSync(full, "utf8")); + for (const arg of specs) { + if (intoV1(arg, rel)) { problems.push( - `src/${rel}:${lineAt(text, index)}: '${spec}' reaches into src/v1; v0 may import only the v1 public entry, as '@v1'.`, + `src/${rel}:${lineAt(text, arg.index)}: '${arg.spec}' reaches into src/v1; v0 may import only the v1 public entry, as '@v1'.`, ); } } diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 61fa459ac3..be49afa0c7 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -1,8 +1,9 @@ # Frontend v1 The v1 frontend, built alongside v0 (everything else under `src/`) until v0 is -retired. The full guide is the Frontend Developer Guide for v1 (UN-4378); this -file holds only the rules that tooling enforces. +retired. The full guide is the Frontend Developer Guide for v1 (UN-4378). The +rules and the "May import from" column below are enforced by tooling; the +"Holds" column is convention. ## Rules @@ -12,6 +13,9 @@ file holds only the rules that tooling enforces. shim, and no symlinks. Import with `./` within a folder and `@v1/...` across folders; `../`, `/src/...` and `src/...` are never used. - **v0 imports only the public entry**, `@v1` (`src/v1/index.ts`). +- **Imports must be readable.** Use a string literal, or a template whose fixed + prefix names the folder (`` `@v1/pages/${name}/index.tsx` ``, allowed only in + folders that may import everything). An import of a variable is rejected. - **No PropTypes, no component `.css`, no antd** (package, shim or `ant-` class). Global v1 styles live in `styles/` and are imported as `@v1/styles/...`, including from inside `styles/`. @@ -34,7 +38,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | `hooks/` | Shared hooks | `lib/` | | `lib/` | Pure helpers | — | | `styles/` | Tokens and global v1 styles | `lib/` | -| `plugins/` | Cloud overlay target; empty in the public repository | everything | +| `plugins/` | Cloud overlay target; only a placeholder in the public repository | everything | | `test/` | Render helpers, fixtures, network mocks | everything | ## Scripts diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index 62c9d58f6b..e687300241 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -4,8 +4,8 @@ * is not a vitest test (vitest.config.mjs excludes test-fixtures/). * * Each case builds a minimal throwaway copy of the frontend (biome.json, - * package.json, vite-env.d.ts, scripts/, the real src/v1 scaffold, a - * node_modules symlink and a git root), overlays the files under + * package.json, tsconfig.json, vite-env.d.ts, scripts/, the real src/v1 + * scaffold, a node_modules symlink and a .gitignore), overlays the files under * cases// and runs the real package.json scripts, or a raw command, * inside it. The known-bad files live here, outside src/, so CI lint, the * pre-commit hook and typecheck never see them. @@ -47,11 +47,15 @@ const CASES = parseTable(` v1-imports-v0-alias | lint:v1 | v1 imports nothing outside src/v1 v1-imports-v0-relative | lint:v1 | v1 imports nothing outside src/v1 v1-imports-root-absolute | lint:v1 | v1 imports nothing outside src/v1 - # Import boundary, v0 -> v1 (Biome). + # Import boundary, v0 -> v1: Biome for aliases, check-v1 for the rest. v0-imports-v1-internal | biome lint src/ | v0 may import only the v1 public entry - v0-imports-v1-relative | biome lint src/ | v0 may import only the v1 public entry + v0-imports-v1-relative | lint:v1 | v0 may import only the v1 public entry v0-imports-v1-template | lint:v1 | v0 may import only the v1 public entry v0-imports-v1-entry | biome lint src/ | - + # A v1-looking path that is not src/v1 (a package subpath, a v0 folder). + v0-imports-v1-lookalike | biome lint src/ + lint:v1 | - + # v0 typecheck follows the entry into v1, where @v1/... must resolve. + v0-tsx-imports-entry | typecheck + typecheck:v1 + lint:v1 | - # Banned under v1. v1-prop-types | lint:v1 | PropTypes are not used v1-prop-types-subpath | lint:v1 | PropTypes are not used @@ -60,6 +64,9 @@ const CASES = parseTable(` v1-antd-subpath | lint:v1 | antd is not used under src/v1 v1-styles-import-outside | lint:v1 | v1 styles import nothing outside it v1-styles-ant-class | lint:v1 | No antd 'ant-' class names + v1-styles-import-antd | lint:v1 | antd is not used under src/v1 + v1-styles-ant-after-url | lint:v1 | No antd 'ant-' class names + v1-unanalysable-import | lint:v1 | unanalysable import v1-component-css-import | lint:v1 | No component .css under src/v1 v1-component-css-file | typecheck:v1 | No component style files v1-ant-class-prefix | lint:v1 | No antd 'ant-' class names @@ -90,7 +97,12 @@ const CASES = parseTable(` function makeTree(caseName) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "v1-boundary-")); - for (const file of ["biome.json", "package.json", "vite-env.d.ts"]) { + for (const file of [ + "biome.json", + "package.json", + "tsconfig.json", + "vite-env.d.ts", + ]) { fs.copyFileSync(path.join(FRONTEND, file), path.join(dir, file)); } // Copied, not linked: check-v1.mjs finds src/v1 relative to itself. @@ -301,8 +313,12 @@ test("folder contract: every folder pair, by file and by folder", () => { assert.deepEqual(sorted(reported), sorted(expected)); }); +/* Reported lines for one generated file. */ +const linesFor = (reported, file) => + reported.filter((l) => l.startsWith(`${file}:`)); + test("folder contract: every import form is read", () => { - // Each file breaks the contract once (components/ui -> services). + // Each file breaks the contract exactly once (components/ui -> services). const forms = { static: 'import { a } from "@v1/services/a";', multiline: 'import {\n a,\n b,\n} from "@v1/services/a";', @@ -315,12 +331,18 @@ test("folder contract: every import form is read", () => { exportStarAs: 'export * as a from "@v1/services/a";', dynamic: 'export const a = () => import("@v1/services/a");', dynamicTemplate: "export const a = () => import(`@v1/services/a`);", + templatePrefix: + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "export const a = (n: string) => import(`@v1/services/${n}`);", require: 'export const a = require("@v1/services/a");', + newUrl: 'export const a = new URL("@v1/services/a.ts", import.meta.url);', relative: 'import { a } from "./../../services/a";', afterSlashStarInString: 'export const route = "/projects/*";\nimport { a } from "@v1/services/a";\n/** doc */', afterSlashStarInComment: '// matches /api/*\nimport { a } from "@v1/services/a";\n/** doc */', + afterSlashStarInRegex: + 'export const strip = (p: string) => p.replace(/\\/*$/, "");\nimport { a } from "@v1/services/a";\n/** doc */', afterDoubleSlashInString: 'const u = "a//b"; export { a } from "@v1/services/a";', glob: 'export const a = import.meta.glob("@v1/services/*.ts");', @@ -330,18 +352,33 @@ test("folder contract: every import form is read", () => { // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. 'const t = `${"}"}/*`;\nimport { a } from "@v1/services/a";\n/** doc */', }; + // None of these may be reported: comments, JSX, look-alike class names. + const quiet = { + "commentedOut.ts": + '// import "@v1/services/a";\n/* import("@/helpers/x"); */\nexport const v = 1;', + "lookalikeClasses.ts": 'export const c = "variant-outline tenant-id";', + "jsx.tsx": + 'export const T = () =>
a / b
;\nexport const n = 2 / 1;', + }; const files = {}; for (const [form, code] of Object.entries(forms)) { files[`src/v1/components/ui/${form}.ts`] = `${code}\n`; } + for (const [name, code] of Object.entries(quiet)) { + files[`src/v1/components/ui/${name}`] = `${code}\n`; + } const reported = checkCode(files); for (const form of Object.keys(forms)) { - const file = `src/v1/components/ui/${form}.ts:`; + const lines = linesFor(reported, `src/v1/components/ui/${form}.ts`); + assert.equal(lines.length, 1, `${form}:\n${reported.join("\n")}`); assert.ok( - reported.some((l) => l.startsWith(file)), - `${form}: not reported\n${reported.join("\n")}`, + lines[0].includes("components/ui/ may not import from services/"), + `${form}: ${lines[0]}`, ); } + for (const name of Object.keys(quiet)) { + assert.deepEqual(linesFor(reported, `src/v1/components/ui/${name}`), []); + } }); test("a symlinked file under v1 fails typecheck:v1", () => { @@ -360,66 +397,102 @@ test("a symlinked file under v1 fails typecheck:v1", () => { }); test("v1 imports nothing outside src/v1, in any spelling", () => { - const specs = [ - "@/helpers/x", - "../helpers/x", - "..", - "./../../helpers/x", - "/src/assets/logo.svg", - "src/helpers/x", + const imports = [ + "import(`@/helpers/x`)", + "import(`../helpers/x`)", + "import(`..`)", + "import(`./../../helpers/x`)", + "import(`/src/assets/logo.svg`)", + "import(`src/helpers/x`)", + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "import(`../../helpers/${name}`)", + 'new URL("../../assets/logo.svg", import.meta.url)', ]; const files = {}; - specs.forEach((spec, i) => { + imports.forEach((code, i) => { files[`src/v1/lib/outside-${i}.ts`] = - `export const a = () => import(\`${spec}\`);\n`; + `export const a = (name: string) => ${code};\n`; }); const reported = checkCode(files); - specs.forEach((spec, i) => { - assert.ok( - reported.some( - (l) => - l.startsWith(`src/v1/lib/outside-${i}.ts:`) && - l.includes("is outside src/v1"), - ), - `'${spec}': not reported\n${reported.join("\n")}`, - ); + imports.forEach((code, i) => { + const lines = linesFor(reported, `src/v1/lib/outside-${i}.ts`); + assert.equal(lines.length, 1, `${code}:\n${reported.join("\n")}`); + assert.ok(lines[0].includes("is outside src/v1"), `${code}: ${lines[0]}`); }); }); +test("v1: an import the guard cannot read fails closed", () => { + const files = { + "src/v1/lib/variable.ts": "export const a = (n: string) => import(n);\n", + "src/v1/lib/fullyComputed.ts": + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "export const a = (n: string) => import(`${n}`);\n", + "src/v1/components/ui/computedPage.ts": + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "export const a = (n: string) => import(`@v1/pages/${n}/index.tsx`);\n", + // The lazy route registry: app/ may import any v1 folder. + "src/v1/app/routes.ts": + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "export const a = (n: string) => import(`@v1/pages/${n}/index.tsx`);\n", + // A non-module `new URL` is not an import. + "src/v1/lib/link.ts": + 'export const u = (p: string) => new URL(p, "https://example.com");\n', + }; + const reported = checkCode(files); + for (const file of ["variable", "fullyComputed"]) { + const lines = linesFor(reported, `src/v1/lib/${file}.ts`); + assert.equal(lines.length, 1, reported.join("\n")); + assert.ok(lines[0].includes("unanalysable import"), lines[0]); + } + const page = linesFor(reported, "src/v1/components/ui/computedPage.ts"); + assert.equal(page.length, 1, reported.join("\n")); + assert.ok(page[0].includes("may not import a computed v1 path"), page[0]); + assert.deepEqual(linesFor(reported, "src/v1/app/routes.ts"), []); + assert.deepEqual(linesFor(reported, "src/v1/lib/link.ts"), []); +}); + test("v0 reaches v1 only through @v1, in any form", () => { const denied = { "static.jsx": 'import x from "@v1/lib/x";', "template.jsx": "export const a = () => import(`@v1/plugins/manifest`);", + "templatePrefix.jsx": + // biome-ignore lint/suspicious/noTemplateCurlyInString: source text containing a template literal. + "export const a = (n) => import(`../v1/lib/${n}`);", "glob.jsx": 'export const a = import.meta.glob("@v1/pages/*/index.ts");', + "globRootAbsolute.jsx": + 'export const a = import.meta.glob("/src/v1/lib/*.ts");', "globArrayLater.jsx": 'export const a = import.meta.glob(["./*.jsx", "../v1/lib/*.ts"]);', "require.jsx": 'export const a = require("../v1/lib/x");', + "newUrl.jsx": + 'export const a = new URL("../v1/lib/x.ts", import.meta.url);', "aliasV0.jsx": 'export const a = () => import("@/v1/lib/x");', + "aliasV0Dot.jsx": "export const a = () => import(`@/./v1/lib/x`);", "srcPath.jsx": 'export const a = () => import("src/v1/lib/x");', "relativeEntry.jsx": 'import "../v1";', + "typed.tsx": 'import x from "../v1/lib/x";\nexport const y = x;', "styles.css": '@import "../v1/styles/tokens.css";', + "stylesRootAbsolute.css": ".a { background: url(/src/v1/styles/a.png); }", }; const allowed = { "entry.jsx": 'import "@v1";', "entryTemplate.jsx": "export const a = () => import(`@v1`);", + "entryIndex.jsx": 'import "@v1/index";', "v0Only.jsx": 'import x from "../helpers/x";\nexport const y = x;', + "lookalike.jsx": + 'import a from "@/api/v1";\nimport { v1 } from "uuid/v1";\nexport const b = [a, v1];', }; const files = {}; for (const [name, code] of Object.entries({ ...denied, ...allowed })) { files[`src/pages/${name}`] = `${code}\n`; } const reported = checkCode(files); - const v0Lines = reported.filter((l) => l.startsWith("src/pages/")); for (const name of Object.keys(denied)) { - assert.ok( - v0Lines.some((l) => l.startsWith(`src/pages/${name}:`)), - `${name}: not reported\n${reported.join("\n")}`, - ); + const lines = linesFor(reported, `src/pages/${name}`); + assert.equal(lines.length, 1, `${name}:\n${reported.join("\n")}`); + assert.ok(lines[0].includes("reaches into src/v1"), `${name}: ${lines[0]}`); } for (const name of Object.keys(allowed)) { - assert.ok( - !v0Lines.some((l) => l.startsWith(`src/pages/${name}:`)), - `${name}: reported but allowed\n${reported.join("\n")}`, - ); + assert.deepEqual(linesFor(reported, `src/pages/${name}`), [], name); } }); diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-lookalike/src/pages/Lookalike.jsx b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-lookalike/src/pages/Lookalike.jsx new file mode 100644 index 0000000000..04f214831c --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-imports-v1-lookalike/src/pages/Lookalike.jsx @@ -0,0 +1,4 @@ +import api from "@/api/v1"; +import { v1 as uuid } from "uuid/v1"; + +export const ids = () => [api, uuid]; diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/V1Switch.tsx b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/V1Switch.tsx new file mode 100644 index 0000000000..33ec55123a --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/V1Switch.tsx @@ -0,0 +1,3 @@ +import { mountV1 } from "@v1"; + +export const mounted: string = mountV1(); diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/app/mount.ts b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/app/mount.ts new file mode 100644 index 0000000000..4487c798fb --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/app/mount.ts @@ -0,0 +1 @@ +export const mountV1 = (): string => "v1"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/index.ts b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/index.ts new file mode 100644 index 0000000000..f54562d67c --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v0-tsx-imports-entry/src/v1/index.ts @@ -0,0 +1 @@ +export { mountV1 } from "@v1/app/mount"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-after-url/src/v1/styles/global.css b/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-after-url/src/v1/styles/global.css new file mode 100644 index 0000000000..a3a94eac65 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-styles-ant-after-url/src/v1/styles/global.css @@ -0,0 +1 @@ +@import url(//cdn.example.com/x.css); .ant-btn { color: red; } diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-antd/src/v1/styles/global.css b/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-antd/src/v1/styles/global.css new file mode 100644 index 0000000000..fab1ed8535 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-styles-import-antd/src/v1/styles/global.css @@ -0,0 +1 @@ +@import "antd/dist/reset.css"; diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-unanalysable-import/src/v1/lib/load.ts b/frontend/test-fixtures/v1-boundary/cases/v1-unanalysable-import/src/v1/lib/load.ts new file mode 100644 index 0000000000..019411da36 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-unanalysable-import/src/v1/lib/load.ts @@ -0,0 +1 @@ +export const load = (name: string): Promise => import(name); diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index 779791063e..415126d1ef 100644 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -41,8 +41,11 @@ * this file's directory. */ "paths": { "@/*": ["./src/*"], - /* v0 may import only the v1 public entry, so only that is mapped. */ - "@v1": ["./src/v1/index.ts"] + /* v1 code imports across its folders with `@v1/...`, and tsc follows + * the entry into src/v1 once v0 imports it, so the whole alias is + * mapped. That v0 itself imports only `@v1` is enforced by lint. */ + "@v1": ["./src/v1/index.ts"], + "@v1/*": ["./src/v1/*"] } }, "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"], diff --git a/frontend/vite.config.js b/frontend/vite.config.js index fecc6b40f5..fc4f72eb42 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -184,8 +184,8 @@ export default defineConfig(({ mode }) => { resolve: { alias: { "@": path.resolve(import.meta.dirname, "./src"), - // v1 frontend. Also in src/v1/tsconfig.json and vitest.config.mjs; - // tsconfig.json/jsconfig.json map only the entry, for v0. + // v1 frontend. Also in vitest.config.mjs and the tsconfigs. Only lint + // keeps v0 to the `@v1` entry; the alias itself resolves any path. "@v1": path.resolve(import.meta.dirname, "./src/v1"), }, }, From b9bed0dcee75c138c801665eef639212f52f07a3 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Thu, 29 Oct 2026 11:01:14 +0530 Subject: [PATCH 18/26] feat(frontend): add an assets/ folder to the v1 taxonomy (UN-4270) Static files (icons, logos, images) get one home, src/v1/assets/, imported as @v1/assets/... (e.g. logo.svg?react). v1 cannot reach v0's src/assets/. Raised by Ali on #2380: Lane I's P1-T22 needs SVG icons and provider logos, and a .svg outside the taxonomy failed --files. components/ui, components/patterns, shell, pages and styles may import assets/; assets/ imports nothing but itself (room for a generated icon map). README, contract table and the fixture matrix updated together; v1-allowed-imports imports an SVG as a React component under typecheck:v1. --- frontend/scripts/check-v1.mjs | 10 ++++++++-- frontend/src/v1/README.md | 13 ++++++++----- frontend/src/v1/assets/.gitkeep | 0 .../test-fixtures/v1-boundary/boundary.fixtures.mjs | 12 +++++++----- .../cases/v1-allowed-imports/src/v1/assets/logo.svg | 1 + .../src/v1/components/ui/Logo.tsx | 5 +++++ 6 files changed, 29 insertions(+), 12 deletions(-) create mode 100644 frontend/src/v1/assets/.gitkeep create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/assets/logo.svg create mode 100644 frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Logo.tsx diff --git a/frontend/scripts/check-v1.mjs b/frontend/scripts/check-v1.mjs index 649f8307a3..268661725c 100755 --- a/frontend/scripts/check-v1.mjs +++ b/frontend/scripts/check-v1.mjs @@ -59,15 +59,17 @@ const CONTRACT = { "hooks", "lib", "styles", + "assets", "plugins", ], - "components/ui": ["lib", "styles"], + "components/ui": ["lib", "styles", "assets"], "components/patterns": [ "components/ui", "lib", "services", "stores", "hooks", + "assets", ], shell: [ "components/ui", @@ -76,12 +78,15 @@ const CONTRACT = { "stores", "hooks", "lib", + "assets", ], services: ["lib"], stores: ["lib"], hooks: ["lib"], lib: [], - styles: ["lib"], + styles: ["lib", "assets"], + // Static files (icons, logos, images); a generated icon map may live here. + assets: [], }; const TOP_LEVEL = new Set([ "app", @@ -91,6 +96,7 @@ const TOP_LEVEL = new Set([ "hooks", "lib", "styles", + "assets", "plugins", "test", ]); diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index be49afa0c7..b9c2092eef 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -19,6 +19,8 @@ rules and the "May import from" column below are enforced by tooling; the - **No PropTypes, no component `.css`, no antd** (package, shim or `ant-` class). Global v1 styles live in `styles/` and are imported as `@v1/styles/...`, including from inside `styles/`. +- **Static assets live in `assets/`**, imported as `@v1/assets/...` (e.g. + `@v1/assets/logo.svg?react`). v1 never uses v0's `src/assets/`. ## Folders and what each may import @@ -29,15 +31,16 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | --- | --- | --- | | `index.ts` | Public entry; the version switch (UN-4271) will mount v1 through it | everything | | `app/` | Entry, providers, router, route registry | everything | -| `components/ui/` | Core components | `lib/`, `styles/` | -| `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/` | -| `shell/` | Top bar, rail, context bar, tab bar | `components/ui/`, `components/patterns/`, `services/`, `stores/`, `hooks/`, `lib/` | -| `pages//` | One folder per target page | `components/ui/`, `components/patterns/`, `shell/`, `services/`, `stores/`, `hooks/`, `lib/`, `styles/`, `plugins/` — not other pages, `app/`, `test/` or the entry | +| `components/ui/` | Core components | `lib/`, `styles/`, `assets/` | +| `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/`, `assets/` | +| `shell/` | Top bar, rail, context bar, tab bar | `components/ui/`, `components/patterns/`, `services/`, `stores/`, `hooks/`, `lib/`, `assets/` | +| `pages//` | One folder per target page | `components/ui/`, `components/patterns/`, `shell/`, `services/`, `stores/`, `hooks/`, `lib/`, `styles/`, `assets/`, `plugins/` — not other pages, `app/`, `test/` or the entry | | `services/` | Typed service modules; the only place a URL appears | `lib/` | | `stores/` | Zustand stores | `lib/` | | `hooks/` | Shared hooks | `lib/` | | `lib/` | Pure helpers | — | -| `styles/` | Tokens and global v1 styles | `lib/` | +| `styles/` | Tokens and global v1 styles | `lib/`, `assets/` | +| `assets/` | Static files: icons, logos, images (and a generated icon map) | — | | `plugins/` | Cloud overlay target; only a placeholder in the public repository | everything | | `test/` | Render helpers, fixtures, network mocks | everything | diff --git a/frontend/src/v1/assets/.gitkeep b/frontend/src/v1/assets/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs index e687300241..d210b414a7 100644 --- a/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs +++ b/frontend/test-fixtures/v1-boundary/boundary.fixtures.mjs @@ -257,6 +257,7 @@ const FOLDERS = { hooks: "hooks", lib: "lib", styles: "styles", + assets: "assets", plugins: "plugins", test: "test", }; @@ -266,15 +267,16 @@ const MAY_IMPORT = Object.fromEntries( app | * plugins | * test | * - pages/a | components/ui components/patterns shell services stores hooks lib styles plugins - components/ui | lib styles - components/patterns | components/ui lib services stores hooks - shell | components/ui components/patterns services stores hooks lib + pages/a | components/ui components/patterns shell services stores hooks lib styles assets plugins + components/ui | lib styles assets + components/patterns | components/ui lib services stores hooks assets + shell | components/ui components/patterns services stores hooks lib assets services | lib stores | lib hooks | lib lib | - styles | lib + styles | lib assets + assets | `).map(([from, to]) => [from, to === "*" ? EVERYTHING : to.split(/\s+/)]), ); diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/assets/logo.svg b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/assets/logo.svg new file mode 100644 index 0000000000..625fd367af --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/assets/logo.svg @@ -0,0 +1 @@ + diff --git a/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Logo.tsx b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Logo.tsx new file mode 100644 index 0000000000..9ca41d3948 --- /dev/null +++ b/frontend/test-fixtures/v1-boundary/cases/v1-allowed-imports/src/v1/components/ui/Logo.tsx @@ -0,0 +1,5 @@ +import LogoIcon from "@v1/assets/logo.svg?react"; + +export function Logo() { + return