fix(deps): replace @package-json/types with an inline minimal type#476
fix(deps): replace @package-json/types with an inline minimal type#476nbouvrette wants to merge 10 commits intoun-ts:masterfrom
Conversation
@package-json/types was added in un-ts#434 as a replacement for type-fest, after type-fest leaked into published .d.ts files from devDependencies. However the swap introduced a new transitive dependency (~148k weekly downloads, low activity) for what is effectively three type annotations touching only 8 package.json fields. This commit removes the dependency entirely and replaces it with a small inline PackageJson interface in src/utils/package-json.ts that covers exactly the fields this plugin accesses: - name, version (read-pkg-up, no-duplicates) - dependencies, devDependencies, optionalDependencies, peerDependencies, bundleDependencies, bundledDependencies (no-extraneous-dependencies) Benefits: - Zero runtime/type dependencies added to consumers - No more exposure to upstream type breakages (e.g. TS2411 strict errors in @package-json/types that affect consumers with strict: true) - The interface is trivial to maintain; package.json dep fields are stable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved the external Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
|
FYI @mshima (re: #434 (comment)) |
Add `private`, `main`, `bin`, and `browser` fields to the inline PackageJson interface, which are accessed in `no-unused-modules.ts` but were omitted from the initial audit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Good catch @SukkaW! The initial audit only covered files explicitly mentioned in the PR description ( Fixed in the latest commit by adding those four missing fields to the inline interface. I've verified all |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/package-json.ts`:
- Around line 16-18: Change the types for bundleDependencies and
bundledDependencies from "string[] | Record<string,string>" to the correct
npm-supported "string[] | boolean", and update the preceding comment to state
they support either an array of package names or a boolean (remove the incorrect
"object form (supported by npm)" phrasing); if your code still intentionally
accepts non-standard object forms via arrayOrKeys(), add a separate inline note
next to arrayOrKeys() (or near these fields) clarifying that object-form support
is an internal/lenient parse behavior and not part of the npm spec.
The comment implied npm supports an object form for bundleDependencies, which is inaccurate. Only string[] is in the npm spec. The Record<string,string> type is kept because arrayOrKeys() in no-extraneous-dependencies.ts handles object form leniently, but boolean would break that call site. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- bundleDependencies/bundledDependencies: string[] | boolean (per npm spec and original @package-json/types), not Record<string,string>. Update no-extraneous-dependencies.ts call site to guard against boolean values. - browser: add | false to record values (Record<string, string | false>) to support the common pattern of excluding modules (e.g. "fs": false). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rules/no-extraneous-dependencies.ts`:
- Around line 44-52: The code uses `const bundle = pkg.bundleDependencies ||
pkg.bundledDependencies` which treats an explicit falsey value (e.g., false) as
missing; update this to use the nullish coalescing operator so explicit booleans
are preserved: replace the `||` with `??` when assigning `bundle` in the
`no-extraneous-dependencies.ts` code that computes `bundle` and then later uses
`arrayOrKeys(bundle)` for `bundledDependencies`, ensuring the boolean `false` is
not overwritten by the fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 078841c2-c04d-4a4b-8622-3b70cd76fa69
📒 Files selected for processing (2)
src/rules/no-extraneous-dependencies.tssrc/utils/package-json.ts
The object form of bundleDependencies (Record<string, string>) is non-standard but intentionally supported by no-extraneous-dependencies and has been tested since the original eslint-plugin-import (2019). Revert the type back to string[] | Record<string, string> and restore the original call site. Also fix the comment in no-extraneous-dependencies.ts: object form is non-standard, not "supported by npm" as the old comment claimed. The browser fix (Record<string, string | false>) is kept — false values are a real pattern for excluding modules in browser bundles. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
commit: |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Since utils/read-pkg-up.js is part of the public API (exported via the ./utils package export), PackageJson appears in its return type and is effectively public. Moving it to src/types.ts makes this explicit and consistent with the rest of the public type surface. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Removes the @package-json/types dependency and replaces it with an internal minimal PackageJson interface so published .d.ts files no longer rely on a low-maintenance transitive type package and strict TS consumers avoid upstream type issues.
Changes:
- Drop
@package-json/typesfromdependencies(and lockfile). - Add an internal minimal
PackageJsoninterface and switch type imports to use it. - Update documentation comment around
bundleDependencies/bundledDependencieshandling.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes the resolved @package-json/types entry after dependency removal. |
| package.json | Removes @package-json/types from dependencies. |
| src/types.ts | Adds internal PackageJson interface for the subset of package.json fields used. |
| src/utils/read-pkg-up.ts | Switches PackageJson type import to the internal type. |
| src/rules/no-duplicates.ts | Switches PackageJson type import to the internal type. |
| src/rules/no-extraneous-dependencies.ts | Switches PackageJson type import and adjusts the bundled deps comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nicolas Bouvrette <bouvrette.nicolas@gmail.com>
Problem
@package-json/typeswas added in #434 to replacetype-festafter it leaked into published.d.tsfiles (from being indevDependencies). While that fixed the immediate issue, it introduced a new transitive dependency with low maintenance activity — all for type annotations covering a handful ofpackage.jsonfields.Additionally,
@package-json/typesitself has a bug under strict TypeScript (strict: true) that affects consumers:(A fix has been submitted upstream: importantimport/package-json#1, but it depends on that maintainer acting.)
Solution
Remove
@package-json/typesfromdependenciesentirely and replace it with a small inlinePackageJsoninterface added tosrc/types.ts, covering exactly the fields this plugin uses:name,version— used inread-pkg-up.tsandno-duplicates.tsprivate,main,bin,browser— used inno-unused-modules.tsdependencies,devDependencies,optionalDependencies,peerDependencies,bundleDependencies,bundledDependencies— used inno-extraneous-dependencies.tsPackageJsonlives insrc/types.ts(not a new utility file) becauseutils/read-pkg-up.jsis part of the public API (exported via the./utilspackage export) and referencesPackageJsonin its return type — making it effectively public. This is also why@package-json/typeshad to be independenciesrather thandevDependenciesin the first place.Field types are aligned with authoritative sources (
type-fest,pkg-types, npm spec):browserusesRecord<string, string | false>—falseis a real pattern for excluding modules in browser bundles (e.g."fs": false)bundleDependencies/bundledDependenciesusestring[] | Record<string, string>— array per npm spec; object form is non-standard but intentionally supported by this rule (tested since the originaleslint-plugin-importin 2019)Benefits
.d.tsfiles@package-json/typesno longer affects consumers of this pluginpackage.jsonfieldsChanges
src/types.ts— newPackageJsoninterface added to the existing public types filesrc/utils/read-pkg-up.ts,src/rules/no-duplicates.ts,src/rules/no-extraneous-dependencies.ts,src/rules/no-unused-modules.ts— update imports to use local typesrc/rules/no-extraneous-dependencies.ts— fix comment: object form ofbundleDependenciesis supported by this rule, not by npmpackage.json— remove@package-json/typesfromdependenciesyarn.lock— updated after dependency removal