Skip to content

Commit 0f751ba

Browse files
thymikeeclaude
andcommitted
refactor(layering): let test support resolve private siblings through devDependencies
R11 now reads a private package's devDependencies as test-only siblings: tests, specs, fixtures files and test trees may import through them, production source still needs dependencies, and the package tsconfig references must name both. The daemon moves test-kit to devDependencies. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent b962458 commit 0f751ba

5 files changed

Lines changed: 144 additions & 10 deletions

File tree

‎packages/daemon/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222
"@agent-device/replay-port": "workspace:*",
2323
"@agent-device/replay-test": "workspace:*",
2424
"@agent-device/selectors": "workspace:*",
25-
"@agent-device/session-journal": "workspace:*",
26-
"@agent-device/test-kit": "workspace:*"
25+
"@agent-device/session-journal": "workspace:*"
2726
},
2827
"devDependencies": {
28+
"@agent-device/test-kit": "workspace:*",
2929
"fast-check": "^4.9.0",
3030
"oxc-parser": "^0.146.0",
3131
"vitest": "5.0.4",

‎pnpm-lock.yaml‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// R11 and devDependencies: a private package declares siblings in `dependencies`, and only test
2+
// support (tests, `*.fixtures.*` files, test trees) may also resolve through its `devDependencies`,
3+
// so a test-only sibling such as test-kit never becomes a runtime dependency. Production source
4+
// still needs the production field. Compact sibling of package-boundaries.test.ts, which sits at
5+
// its size ratchet.
6+
7+
import assert from 'node:assert/strict';
8+
import { test } from 'node:test';
9+
import {
10+
checkPackageInternalSites,
11+
specifierSites,
12+
workspacePackagesFromManifests,
13+
} from './package-boundaries.ts';
14+
15+
function manifests(owner: Record<string, unknown>) {
16+
return new Map([
17+
[
18+
'packages/helper/package.json',
19+
JSON.stringify({
20+
name: '@agent-device/helper',
21+
private: true,
22+
exports: { '.': './src/index.ts' },
23+
}),
24+
],
25+
[
26+
'packages/owner/package.json',
27+
JSON.stringify({ name: '@agent-device/owner', exports: { '.': './src/index.ts' }, ...owner }),
28+
],
29+
]);
30+
}
31+
32+
function violations(owner: Record<string, unknown>, file: string) {
33+
const packages = workspacePackagesFromManifests(manifests(owner));
34+
const pkg = packages.find(({ name }) => name === '@agent-device/owner')!;
35+
return checkPackageInternalSites(
36+
pkg,
37+
specifierSites(file, "import { helper } from '@agent-device/helper';"),
38+
packages,
39+
true,
40+
);
41+
}
42+
43+
const DEV_ONLY = { private: true, devDependencies: { '@agent-device/helper': 'workspace:*' } };
44+
45+
test('a private package reads its devDependencies as test-only siblings', () => {
46+
const [, owner] = workspacePackagesFromManifests(manifests(DEV_ONLY));
47+
assert.deepEqual([...owner!.workspaceDependencies], []);
48+
assert.deepEqual([...owner!.workspaceDevDependencies!], ['@agent-device/helper']);
49+
});
50+
51+
test('test support may import a sibling declared only in devDependencies', () => {
52+
for (const file of [
53+
'packages/owner/src/thing.test.ts',
54+
'packages/owner/src/thing.spec.ts',
55+
'packages/owner/src/__tests__/thing.ts',
56+
'packages/owner/src/thing.fixtures.ts',
57+
'packages/owner/test/harness.ts',
58+
]) {
59+
assert.deepEqual(violations(DEV_ONLY, file), [], file);
60+
}
61+
});
62+
63+
test('production source needs the production field and is told so', () => {
64+
const found = violations(DEV_ONLY, 'packages/owner/src/thing.ts');
65+
assert.equal(found.length, 1);
66+
assert.match(found[0]!.message, /declares "@agent-device\/helper" only in devDependencies/);
67+
assert.match(found[0]!.message, /declare it in dependencies/);
68+
});
69+
70+
test('an undeclared sibling is refused for tests and production alike', () => {
71+
for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) {
72+
const found = violations({ private: true }, file);
73+
assert.equal(found.length, 1, file);
74+
assert.match(found[0]!.message, /without declaring "@agent-device\/helper"/);
75+
}
76+
});
77+
78+
test('a production declaration serves tests and production', () => {
79+
const declared = { private: true, dependencies: { '@agent-device/helper': 'workspace:*' } };
80+
for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) {
81+
assert.deepEqual(violations(declared, file), [], file);
82+
}
83+
});
84+
85+
test('a published package keeps its siblings in devDependencies for every file', () => {
86+
const published = { devDependencies: { '@agent-device/helper': 'workspace:*' } };
87+
for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) {
88+
assert.deepEqual(violations(published, file), [], file);
89+
}
90+
});

‎scripts/layering/package-boundaries.ts‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ import {
3737
resolveAllConditionTargets,
3838
testOnlyImportMessage,
3939
} from './package-export-surface.ts';
40-
import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts';
40+
import {
41+
isProductionSourceFile,
42+
listTrackedPackageManifests,
43+
listTrackedProductionSources,
44+
} from './tracked-sources.ts';
4145

4246
export type PackageBoundaryViolation = {
4347
rule: string;
@@ -56,6 +60,13 @@ export type WorkspacePackage = {
5660
exportConditionTargets?: ReadonlyMap<string, readonly string[]>;
5761
/** Declared `workspace:*` dependencies on sibling internal packages. */
5862
workspaceDependencies: ReadonlySet<string>;
63+
/**
64+
* Workspace siblings a private package declares in `devDependencies`. Only test support (tests,
65+
* fixtures, test trees) may import through them; production source still needs
66+
* `workspaceDependencyField`. A published package keeps every sibling in `devDependencies`
67+
* already, so this is empty there.
68+
*/
69+
workspaceDevDependencies?: ReadonlySet<string>;
5970
/** Where siblings are declared: a published package bundles them, so it lists them as dev-only. */
6071
workspaceDependencyField: 'dependencies' | 'devDependencies';
6172
/** Non-workspace dependencies that the root build must externalize. */
@@ -167,6 +178,13 @@ export function workspacePackagesFromManifests(
167178
.filter(([, range]) => range.startsWith('workspace:'))
168179
.map(([name]) => name),
169180
);
181+
const workspaceDevDependencies = new Set(
182+
manifest.private === true
183+
? Object.entries(manifest.devDependencies ?? {})
184+
.filter(([, range]) => range.startsWith('workspace:'))
185+
.map(([name]) => name)
186+
: [],
187+
);
170188
const externalDependencies = new Map(
171189
Object.entries(manifest.dependencies ?? {}).filter(
172190
([, range]) => !range.startsWith('workspace:'),
@@ -178,6 +196,7 @@ export function workspacePackagesFromManifests(
178196
exportTargets,
179197
exportConditionTargets: conditionTargets,
180198
workspaceDependencies,
199+
workspaceDevDependencies,
181200
workspaceDependencyField,
182201
externalDependencies,
183202
});
@@ -252,6 +271,29 @@ export function insideCompiledSources(file: string): boolean {
252271
return /^packages\/[^/]+\/src\//.test(file) || /^src\//.test(file) || /^test\//.test(file);
253272
}
254273

274+
/**
275+
* A sibling import must be declared `workspace:*` in the package manifest. Production source needs
276+
* the production field (`dependencies`, or `devDependencies` for a published package, which
277+
* bundles its siblings); test support — tests, `*.fixtures.*` files, test trees, test-kit — may
278+
* also resolve through a private package's `devDependencies`, so a test-only sibling such as
279+
* test-kit never ships as a runtime dependency.
280+
*/
281+
function undeclaredSiblingMessage(
282+
pkg: WorkspacePackage,
283+
name: string,
284+
site: SpecifierSite,
285+
): string | undefined {
286+
if (name === pkg.name || pkg.workspaceDependencies.has(name)) return undefined;
287+
const declaredForTests = pkg.workspaceDevDependencies?.has(name) ?? false;
288+
if (declaredForTests && !isProductionSourceFile(site.file)) return undefined;
289+
return declaredForTests
290+
? `${pkg.name} imports '${site.specifier}' from production source, but ${pkg.dir}/package.json ` +
291+
`declares "${name}" only in devDependencies: declare it in ${pkg.workspaceDependencyField} ` +
292+
`or import it from test support only.`
293+
: `${pkg.name} imports '${site.specifier}' without declaring "${name}": "workspace:*" ` +
294+
`in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`;
295+
}
296+
255297
/**
256298
* Rules for files INSIDE a package. An undeclared sibling import is R11's alone: pnpm links
257299
* every workspace package under the root `node_modules`, so the import resolves, compiles, and
@@ -312,14 +354,13 @@ export function checkPackageInternalSites(
312354
}
313355
continue;
314356
}
315-
if (name !== pkg.name && !pkg.workspaceDependencies.has(name)) {
357+
const undeclared = undeclaredSiblingMessage(pkg, name, site);
358+
if (undeclared) {
316359
violations.push({
317360
rule: 'R11 package-boundaries',
318361
file: site.file,
319362
line: site.line,
320-
message:
321-
`${pkg.name} imports '${site.specifier}' without declaring "${name}": "workspace:*" ` +
322-
`in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`,
363+
message: undeclared,
323364
});
324365
}
325366
const testDataOnly = checkTestOnlyImportSite(site, target);

‎scripts/layering/project-references.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,11 @@ test('every package tsconfig references exactly its manifest workspace dependenc
165165
}
166166
assert.deepEqual(
167167
referencedDirs.map((dir) => nameByDir.get(dir)),
168-
[...pkg.workspaceDependencies].filter((name) => name !== rootPackageName).sort(),
169-
`${pkg.dir}/tsconfig.json references must equal its manifest workspace:* dependencies`,
168+
[...new Set([...pkg.workspaceDependencies, ...(pkg.workspaceDevDependencies ?? [])])]
169+
.filter((name) => name !== rootPackageName)
170+
.sort(),
171+
`${pkg.dir}/tsconfig.json references must equal its manifest workspace:* dependencies and ` +
172+
`devDependencies`,
170173
);
171174
if (pkg.workspaceDependencies.has(rootPackageName)) {
172175
const config = readTsconfig(`${pkg.dir}/tsconfig.json`);

0 commit comments

Comments
 (0)