|
| 1 | +import { describe, expect, it } from 'vitest'; |
| 2 | +import fs from 'node:fs'; |
| 3 | +import path from 'node:path'; |
| 4 | +import { fileURLToPath } from 'node:url'; |
| 5 | + |
| 6 | +/** |
| 7 | + * objectui#3207: `turbo run type-check --filter=@object-ui/plugin-detail` failed |
| 8 | + * on a clean worktree with six `TS2307: Cannot find module '@object-ui/permissions'` |
| 9 | + * errors, plus the `TS7006 implicitly has an 'any' type` cascade that follows a |
| 10 | + * failed import. `@object-ui/plugin-list` had the identical break. |
| 11 | + * |
| 12 | + * The cause is a build-graph hole, not a missing install. Both packages listed |
| 13 | + * `@object-ui/permissions` in `peerDependencies` only. pnpm still symlinks such a |
| 14 | + * peer into `node_modules` (the lockfile recorded it under the importer's |
| 15 | + * `dependencies`), so the package *path* resolved fine — but turbo reads |
| 16 | + * `package.json`, not the lockfile, and the `type-check` task's `dependsOn: |
| 17 | + * ["^build"]` walks `dependencies`/`devDependencies` only. `permissions/dist` |
| 18 | + * therefore never got built, and `tsc` found a directory with no type |
| 19 | + * declarations in it. |
| 20 | + * |
| 21 | + * Why that is worth a guard rather than just a one-line fix: the full-repo |
| 22 | + * `pnpm type-check` was GREEN the whole time, because some other package's build |
| 23 | + * dragged `@object-ui/permissions` up as a side effect. CI could not see this. |
| 24 | + * The only command that could was a scoped, per-package typecheck — precisely the |
| 25 | + * command someone should run after touching one package — and it answered with a |
| 26 | + * screenful of red that had nothing to do with their change. That is the worst |
| 27 | + * possible failure mode: it trains people to ignore the output of the one check |
| 28 | + * that would have caught their real mistake. |
| 29 | + * |
| 30 | + * So the invariant is enforced structurally instead of being restored by hand: |
| 31 | + * a peer that lives in this workspace must also carry a `dependencies` or |
| 32 | + * `devDependencies` edge, so turbo's `^` traversal can actually reach it. At the |
| 33 | + * time of writing every one of the workspace's packages satisfies this, which is |
| 34 | + * why there is no exemption list here and should not become one — an exemption |
| 35 | + * would be indistinguishable from the bug this pins down. |
| 36 | + */ |
| 37 | +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); |
| 38 | + |
| 39 | +interface WorkspacePackage { |
| 40 | + name: string; |
| 41 | + dir: string; |
| 42 | + json: { |
| 43 | + dependencies?: Record<string, string>; |
| 44 | + devDependencies?: Record<string, string>; |
| 45 | + peerDependencies?: Record<string, string>; |
| 46 | + }; |
| 47 | +} |
| 48 | + |
| 49 | +/** |
| 50 | + * The `packages:` globs from `pnpm-workspace.yaml`, read rather than hardcoded so |
| 51 | + * a new workspace root is covered the day it is added. |
| 52 | + * |
| 53 | + * The parser deliberately understands only the two shapes the file actually uses |
| 54 | + * (`dir/*` and a bare `dir`). Anything else throws instead of being skipped: a |
| 55 | + * guard that silently stops looking at part of the workspace is worse than no |
| 56 | + * guard, because it keeps reporting success over a shrinking surface. |
| 57 | + */ |
| 58 | +function workspaceGlobs(): string[] { |
| 59 | + const yaml = fs.readFileSync(path.join(repoRoot, 'pnpm-workspace.yaml'), 'utf8'); |
| 60 | + const lines = yaml.split('\n'); |
| 61 | + const start = lines.findIndex((l) => /^packages:\s*$/.test(l)); |
| 62 | + expect(start, '`pnpm-workspace.yaml` must still declare a top-level `packages:` key').toBeGreaterThan(-1); |
| 63 | + |
| 64 | + const globs: string[] = []; |
| 65 | + for (const line of lines.slice(start + 1)) { |
| 66 | + if (/^\s*(#.*)?$/.test(line)) continue; |
| 67 | + // A non-indented line ends the `packages:` block. |
| 68 | + if (!/^\s/.test(line)) break; |
| 69 | + const match = line.match(/^\s*-\s*['"]?([^'"#\s]+)['"]?\s*(#.*)?$/); |
| 70 | + if (!match) throw new Error(`Unparsed entry in pnpm-workspace.yaml \`packages:\`: ${JSON.stringify(line)} — teach this guard the new syntax.`); |
| 71 | + globs.push(match[1]); |
| 72 | + } |
| 73 | + return globs; |
| 74 | +} |
| 75 | + |
| 76 | +function readWorkspacePackages(): WorkspacePackage[] { |
| 77 | + const found: WorkspacePackage[] = []; |
| 78 | + for (const glob of workspaceGlobs()) { |
| 79 | + let dirs: string[]; |
| 80 | + if (glob.endsWith('/*')) { |
| 81 | + const parent = path.join(repoRoot, glob.slice(0, -2)); |
| 82 | + dirs = fs.existsSync(parent) |
| 83 | + ? fs.readdirSync(parent).map((d) => path.join(parent, d)).filter((d) => fs.statSync(d).isDirectory()) |
| 84 | + : []; |
| 85 | + } else if (!glob.includes('*')) { |
| 86 | + dirs = [path.join(repoRoot, glob)]; |
| 87 | + } else { |
| 88 | + throw new Error(`Unsupported workspace glob ${JSON.stringify(glob)} — teach this guard how to expand it.`); |
| 89 | + } |
| 90 | + |
| 91 | + for (const dir of dirs) { |
| 92 | + const pkgPath = path.join(dir, 'package.json'); |
| 93 | + if (!fs.existsSync(pkgPath)) continue; |
| 94 | + const json = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); |
| 95 | + if (!json.name) continue; |
| 96 | + found.push({ name: json.name, dir: path.relative(repoRoot, dir), json }); |
| 97 | + } |
| 98 | + } |
| 99 | + return found; |
| 100 | +} |
| 101 | + |
| 102 | +const packages = readWorkspacePackages(); |
| 103 | +const workspaceNames = new Set(packages.map((p) => p.name)); |
| 104 | + |
| 105 | +describe('workspace peer dependencies are reachable by turbo `^build`', () => { |
| 106 | + it('discovers the workspace (guard cannot pass by finding nothing)', () => { |
| 107 | + // Without this, a broken parser or a moved directory would turn every |
| 108 | + // assertion below into a vacuous pass over an empty list. |
| 109 | + expect(packages.length).toBeGreaterThan(30); |
| 110 | + expect(workspaceNames.has('@object-ui/plugin-detail')).toBe(true); |
| 111 | + expect(workspaceNames.has('@object-ui/permissions')).toBe(true); |
| 112 | + }); |
| 113 | + |
| 114 | + it('every workspace package that is a peer is also a dependency or devDependency', () => { |
| 115 | + const violations: string[] = []; |
| 116 | + |
| 117 | + for (const pkg of packages) { |
| 118 | + const peers = Object.keys(pkg.json.peerDependencies ?? {}); |
| 119 | + const edges = new Set([ |
| 120 | + ...Object.keys(pkg.json.dependencies ?? {}), |
| 121 | + ...Object.keys(pkg.json.devDependencies ?? {}), |
| 122 | + ]); |
| 123 | + |
| 124 | + for (const peer of peers) { |
| 125 | + // Only workspace packages matter: turbo builds those, and only those |
| 126 | + // need an edge for `^build` to reach them. An external peer such as |
| 127 | + // `react` or `@objectstack/spec` comes from the registry already built. |
| 128 | + if (!workspaceNames.has(peer)) continue; |
| 129 | + if (edges.has(peer)) continue; |
| 130 | + violations.push(`${pkg.name} (${pkg.dir}/package.json) declares "${peer}" in peerDependencies but has no dependencies/devDependencies edge for it`); |
| 131 | + } |
| 132 | + } |
| 133 | + |
| 134 | + expect( |
| 135 | + violations, |
| 136 | + [ |
| 137 | + 'A workspace peer without a dependencies/devDependencies edge is invisible to turbo.', |
| 138 | + "The `type-check` task's `dependsOn: [\"^build\"]` walks dependencies/devDependencies only,", |
| 139 | + 'so that peer never gets built and a scoped `turbo run type-check --filter=<pkg>` fails with', |
| 140 | + "TS2307 \"Cannot find module\" errors that have nothing to do with the change being tested.", |
| 141 | + '', |
| 142 | + 'Fix: add the peer to devDependencies as "workspace:*" (see objectui#3207), then `pnpm install`.', |
| 143 | + '', |
| 144 | + ...violations, |
| 145 | + ].join('\n'), |
| 146 | + ).toEqual([]); |
| 147 | + }); |
| 148 | + |
| 149 | + it('pins the two packages fixed in objectui#3207', () => { |
| 150 | + // The general assertion above would catch a regression here too, but these |
| 151 | + // two are named so a revert points straight at the issue that explains why. |
| 152 | + for (const name of ['@object-ui/plugin-detail', '@object-ui/plugin-list']) { |
| 153 | + const pkg = packages.find((p) => p.name === name); |
| 154 | + expect(pkg, `${name} must exist in the workspace`).toBeDefined(); |
| 155 | + expect( |
| 156 | + pkg!.json.devDependencies?.['@object-ui/permissions'], |
| 157 | + `${name} imports @object-ui/permissions in its sources, so it needs the devDependencies edge that lets turbo build it`, |
| 158 | + ).toBe('workspace:*'); |
| 159 | + } |
| 160 | + }); |
| 161 | +}); |
0 commit comments