test: guard runtime-agnostic dist entries against node-builtin imports#15
Merged
Conversation
Walks the transitive import graph of devframe/client and the runtime-agnostic devframe/utils/* dist entries and fails on any node:*/bare-builtin import or require() call. Surfaced that tsdown's CJS-interop shim imports node:module#createRequire, tainting any agnostic entry that transitively bundled a CJS dep. Vendor human-id (pure CJS) inline and replace ansis (ESM that re-exports from .cjs) with a small inline ANSI palette so the shim is no longer pulled in.
✅ Deploy Preview for devfra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Adds a runtime-agnostic dist-entry guard for devframe/client and selected devframe/utils/* exports, and removes Node-builtin taint by inlining small replacements for human-id and ansis.
Changes:
- Added a Vitest guard that scans built dist entries for Node builtin imports/requires.
- Vendored
humanId()word-list generation and replacedansiswith a minimal inline ANSI color palette. - Removed
human-id/ansispackage wiring and updated API snapshots.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/devframe/test/runtime-agnostic.test.ts |
Adds dist-entry scan for Node builtin imports. |
packages/devframe/src/utils/human-id.ts |
Replaces external human-id dependency with inline implementation. |
packages/devframe/src/utils/colors.ts |
Replaces ansis with inline color functions. |
packages/devframe/package.json |
Removes direct/inlined dependency declarations. |
packages/devframe/tsdown.config.ts |
Removes deleted deps from bundling allowlist. |
pnpm-workspace.yaml |
Removes deleted deps from catalogs. |
pnpm-lock.yaml |
Updates lockfile after dependency removals. |
tests/__snapshots__/tsnapi/devframe/utils/human-id.snapshot.js |
Updates public API snapshot shape. |
tests/__snapshots__/tsnapi/devframe/utils/colors.snapshot.js |
Updates public API snapshot shape. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Generate a human-readable, lowercase, dash-separated random ID | ||
| * (e.g. `bright-orange-tiger`). | ||
| * (e.g. `bright-orange-tigers-jump`). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/devframe/test/runtime-agnostic.test.ts, which walks the transitive import graph ofdevframe/clientand the 10 runtime-agnosticdevframe/utils/*entries and fails on anynode:*/ bare-builtin import orrequire()call.node:module#createRequire, so every agnostic entry that transitively bundled a CJS dep was tainted;human-id(pure CJS) andansis(ESM that re-exports from.cjs) were the offenders pulling the shim intoutils/human-id,utils/colors, and (viahuman-id)client/index.human-id@4.1.3inline insrc/utils/human-id.ts(~30 LOC + MIT-licensed word lists) and replacesansis@4.3.0with a small inline ANSI palette insrc/utils/colors.ts; drops both deps frompackage.json, the tsdownonlyBundlelist, and the pnpm workspace catalogs.tsnapisnapshots updated to reflect the now-directexport function/export constshapes; full pre-PR run (pnpm lint && pnpm test && pnpm typecheck && pnpm build) is green with 317/317 tests passing.Test plan
pnpm --filter devframe build && pnpm exec vitest run --project devframe runtime-agnostic— 11/11 entries greenimport { readFileSync } from 'node:fs'intosrc/utils/nanoid.ts, rebuild — guard reports 4 entries red (nanoid + 3 transitive consumers); revertpnpm lint && pnpm test && pnpm typecheck && pnpm build— all clean