docs: mirror the overrides rule into copilot-instructions.md
AGENTS.md requires review-relevant rules to be mirrored into .github/copilot-instructions.md in the same PR, and that file already carries a Dependency placement section. Add the distilled form of the new overrides guidance: use an `overrides` entry rather than `npm audit fix` when the advisory range has no upward escape inside the parent's declared range, gate such a pin on the build rather than the audit, expect the lockfile diff to touch only that package, and say when the pin can be dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
3be4230391
commit
012f3a062a
1 changed files with 1 additions and 0 deletions
1
.github/copilot-instructions.md
vendored
1
.github/copilot-instructions.md
vendored
|
|
@ -81,6 +81,7 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
|
|||
- **A dependency that renders React components must be bundled into the client that uses it, and is then not a root dependency.** An externalized package resolves its own `react` from wherever npm placed it in the _consumer's_ tree, beside a React satisfying _its_ peer range — looser than ours, which is all it takes to split React. `ink-form`/`ink-scroll-view` declare `">=18"`, so a consumer's React 18 satisfies them, the TUI ends up with two React instances, and it crashes on the first hook (#1952). Both are inlined via `noExternal` in `clients/tui/tsup.config.ts`. **`ink` is the one exemption, justified by cost (~1.4MB) — never by a peer range**: flag any claim that `">=19"` keeps npm from misplacing it, which is false and was in this repo once. What keeps it safe is the **root `react` range staying open to the whole major (`^19.0.0`)** so npm can dedupe with a consumer's pinned React 19; treat narrowing that range as reopening the bug. `clients/tui/__tests__/tsupConfig.test.ts` enforces the split, the root-declaration of exempt packages, and that range.
|
||||
- **Which section is a separate question from which manifest.** A package `core/` imports at runtime must be in root **`dependencies`**: client builds externalize npm packages, so a published install resolves them from the root manifest and devDependencies are absent there. Only test/build-only packages (`express`) belong in `devDependencies`. Flag a runtime `core/` import added to `devDependencies` — it passes every local check and breaks the published package.
|
||||
- **A root-declared package that `core/` imports at runtime must also be named in each client's bundler `external` list** — `clients/{cli,tui}/tsup.config.ts` and `clients/web/tsup.runner.config.ts`, all three. Bundlers externalize what the _client's_ manifest declares, and these packages are root-only by rule, so omitting them means they get bundled. For a CJS package inlined into an ESM bundle that is fatal: esbuild's `Dynamic require of "path" is not supported` shim throws at import time and the binary dies before parsing a flag (#2082, `proper-lockfile`). Flag a new root runtime dependency that is not added to all three. This has broken twice — `undici` slipped past #2082 because it _was_ declared in `clients/cli/package.json`, so the CLI looked fine while the web and TUI bundles inlined 1.05MB of it, unloadably (#2067). `npm run verify:bundle-externals` now guards it from the **built output**; flag any change that would weaken or skip that guard.
|
||||
- **Pin a transitive dependency past its parent's declared range with an `overrides` entry — never with `npm audit fix`.** When the advisory range has no upward escape inside the parent's range, `npm audit fix` resolves it by silently *downgrading* (esbuild 0.27.7 → 0.27.2 across three installs, #2058/#2062). Flag that in a diff. An override puts a bundler on a major it does not declare, so the gate for one is `npm run build`, not `npm audit`; a lockfile diff accompanying it should touch that package's entries only. Flag an override added without a note saying when it can be dropped.
|
||||
|
||||
## Tests and the coverage gate
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue