Repository navigation
Enforce import boundaries with Biome - #1491
cristianrgreco wants to merge 2 commits into
Conversation
Turn on suspicious/noImportCycles and, for packages/modules, style/noRestrictedImports so modules only import core through the testcontainers entry point (the core test helper stays allowed). Suppress the existing GenericContainer <-> reaper/port-forwarder/builder cycle per import, since Ryuk and sshd are GenericContainers and every edge is only used at runtime. Fix the two deep imports in the weaviate and mongodb tests without changing the public API.
cristianrgreco
added
maintenance
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The update tightens lint enforcement and switches module tests to the public API without changing runtime behavior; no actionable merge-blocking risk is evident. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: enforcing import boundaries with Biome lint rules. |
| Description check | |
The description is directly related to the changeset and explains the new Biome rules, existing cycle suppressions, fixed deep imports, and verification results. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 7… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…-boundaries # Conflicts: # biome.json
This branch has not been deployed
1 participant
Summary
Enforces import boundaries with Biome rules that ship with the Biome version we already use (2.5.14). This is the Biome-native alternative to adding dependency-cruiser. The dependency graph is shallow (modules only depend on
testcontainers, never on each other), so we only need two rules:suspicious/noImportCycles:error. Applies to the whole repo.style/noRestrictedImports:error. Scoped topackages/modules/**with anoverride. Modules may only import core through thetestcontainersentry point. The rule blocks:testcontainers/**(package deep imports such astestcontainers/src/typesortestcontainers/build/...)**/testcontainers/src/**(relative reaches into core source)**/testcontainers/src/utils/test-helper, which is explicitly allowed. 54 module tests importgetImagefrom it on purpose. It is a test-only, cross-package import, and making it public would be a separate change.AGENTS.mdgets a short note on both rules.Findings and fixes
Import cycles (10 sites, one loop): suppressed, not refactored
All 10 reports come from one loop in
packages/testcontainers/src:generic-container.ts↔reaper/reaper.ts↔port-forwarder/port-forwarder.ts↔generic-container-builder.ts(plusstarted-generic-container.ts)The loop is inherent. Ryuk and sshd are themselves
GenericContainers, andfromDockerfile()/build()construct each other. testcontainers-java has the same shape (GenericContainer↔ResourceReaper/RyukContainer,PortForwardingContainer). The loop is harmless today because every edge is only used inside functions, never while the module loads.None of the edges is type-only, so
import typewould not break the loop. MovingSSHD_IMAGE/getReaperImage()into a leaf module would not break it either: the same files still needPortForwarderInstance,getReaperandGenericContainerat runtime. Breaking it properly would need dependency injection or a registry. That is too big a refactor for a guardrail PR.So each existing edge gets its own
// biome-ignore lint/suspicious/noImportCycles: <reason>. The suppressions are per import, not per file, so a new cycle still fails, even one that goes through these same files (see the guard proof below).Deep imports (2): fixed without changing the public API
weaviate-container.test.tsimportedtype { Environment } from "testcontainers/src/types".Environmentis not exported fromindex.ts. I removed the annotation, so TypeScript infers the type andwithEnvironment()still type-checks it. This snippet appears indocs/modules/weaviate.md, so the rendered example no longer references a type that users cannot import.mongodb-atlas-local-container.test.tsimportedIntervalRetryfrom../../../testcontainers/src/common. It is already public, so it now comes from"testcontainers".Possible follow-up (not done here because it changes the public API and needs docs plus a
minorlabel): exportEnvironment, and maybe other types intypes.ts, fromtestcontainers'index.ts.Verification
Red, on
mainwith the rules enabled (npx biome lint packages):Green, on this branch:
npm cipackage-lock.jsonunchangednpm run formatnpm run lintnpx biome ci --error-on-warnings .npm run check-compilesnpx vitest run packages/modules/weaviate/src/weaviate-container.test.ts packages/modules/mongodb/src/mongodb-atlas-local-container.test.tsGuard proof. I made these temporary edits, ran
npx biome lint packages, then reverted them:utils/labels.tsimportsGenericContainer. Fails, flagginglabels.ts:1and the unsuppressedlabelsimports in the five loop files.probe-cycle-a.ts↔probe-cycle-b.ts. Fails on both files.redis-container.ts(module runtime code) imports fromtestcontainers/build/commonand../../../testcontainers/src/common. Fails on both withnoRestrictedImports.Why this is not breaking
The runtime changes are comments only (
biome-ignore). The other edits are tobiome.json,AGENTS.md, and two test files. Nothing exported from any package changes, andcheck-compilespasses for core and every module.