Skip to content

fix(no-extraneous-dependencies): merge deps from closest package.json #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
B4nan wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(no-extraneous-dependencies): merge deps from closest package.json #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f1468e
fix(no-extraneous-dependencies): merge deps from closest package.json
B4nan Apr 11, 2026
8047900
test(no-extraneous-dependencies): update tests for closest package.js…
B4nan Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 37 additions & 25 deletions src/rules/no-extraneous-dependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,45 +69,57 @@ function getDependencies(context: RuleContext, packageDir?: string | string[]) {
let paths: string[] = []

try {
let packageContent: PackageDeps = {
const packageContent: PackageDeps = {
dependencies: {},
devDependencies: {},
optionalDependencies: {},
peerDependencies: {},
bundledDependencies: [],
}

// Always merge in deps from the closest `package.json` relative to the
// linted file. Without this, setting `packageDir` to point at a root
// (shared) `package.json` in a monorepo makes the rule ignore the
// workspace package's own deps — every dep declared in the workspace
// package's `package.json` gets flagged as "should be listed in the
// project's dependencies". Checking the closest package.json first
// restores the expected behavior in lerna / npm-workspaces / pnpm-
// workspaces layouts.
const closestPackageJsonPath = pkgUp({
cwd: context.physicalFilename,
})

if (closestPackageJsonPath) {
const closestPackageContent = getPackageDepFields(
closestPackageJsonPath,
false,
)
if (closestPackageContent) {
for (const depsKey of Object.keys(packageContent)) {
const key = depsKey as keyof PackageDeps
Object.assign(packageContent[key], closestPackageContent[key])
}
Comment on lines +98 to +101

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Object.assign corrupts bundledDependencies array during merge.

bundledDependencies is an array, but Object.assign on arrays overwrites by numeric index rather than concatenating. For example:

Object.assign(['dep1', 'dep2'], ['dep3']) // → ['dep3', 'dep2']

This loses 'dep1' and will cause false positives when the closest package.json declares bundled deps that get overwritten by a shorter array from packageDir.

🐛 Proposed fix: Handle array and object merging separately 
       if (closestPackageContent) {
         for (const depsKey of Object.keys(packageContent)) {
           const key = depsKey as keyof PackageDeps
-          Object.assign(packageContent[key], closestPackageContent[key])
+          if (key === 'bundledDependencies') {
+            packageContent[key] = [
+              ...new Set([...packageContent[key], ...closestPackageContent[key]]),
+            ]
+          } else {
+            Object.assign(packageContent[key], closestPackageContent[key])
+          }
         }
       }

Apply the same pattern at lines 119-121 for the packageDir merge loop.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/rules/no-extraneous-dependencies.ts` around lines 98 - 101, The current
merge uses Object.assign(packageContent[key], closestPackageContent[key]) which
corrupts array fields like bundledDependencies by treating arrays as objects;
update the merge to handle arrays and objects separately: in the loop over
Object.keys(packageContent) (variables packageContent, closestPackageContent,
and type PackageDeps), check if both values are arrays via Array.isArray and, if
so, set packageContent[key] = Array.from(new Set([...closestPackageContent[key],
...packageContent[key]])) (or concatenate in the correct precedence) instead of
Object.assign; otherwise, for plain objects keep using Object.assign or
deep-merge as needed. Apply the same array-aware fix to the similar packageDir
merge loop that handles packageDir/closestPackageContent merging.

}
}

if (packageDir && packageDir.length > 0) {
paths = Array.isArray(packageDir)
? packageDir.map(dir => path.resolve(dir))
: [path.resolve(packageDir)]
}

if (paths.length > 0) {
// use rule config to find package.json
for (const dir of paths) {
const packageJsonPath = path.resolve(dir, 'package.json')
const packageContent_ = getPackageDepFields(
packageJsonPath,
paths.length === 1,
)
if (packageContent_) {
for (const depsKey of Object.keys(packageContent)) {
const key = depsKey as keyof PackageDeps
Object.assign(packageContent[key], packageContent_[key])
}
}
}
} else {
// use closest package.json
const packageJsonPath = pkgUp({
cwd: context.physicalFilename,
})!

const packageContent_ = getPackageDepFields(packageJsonPath, false)

// Layer in any deps from `packageDir` on top of the closest match.
for (const dir of paths) {
const packageJsonPath = path.resolve(dir, 'package.json')
const packageContent_ = getPackageDepFields(
packageJsonPath,
paths.length === 1,
)
if (packageContent_) {
packageContent = packageContent_
for (const depsKey of Object.keys(packageContent)) {
const key = depsKey as keyof PackageDeps
Object.assign(packageContent[key], packageContent_[key])
}
Comment on lines +119 to +122

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same bundledDependencies array merge issue here.

The same Object.assign problem applies in this loop when merging deps from packageDir entries.

🐛 Proposed fix 
       if (packageContent_) {
         for (const depsKey of Object.keys(packageContent)) {
           const key = depsKey as keyof PackageDeps
-          Object.assign(packageContent[key], packageContent_[key])
+          if (key === 'bundledDependencies') {
+            packageContent[key] = [
+              ...new Set([...packageContent[key], ...packageContent_[key]]),
+            ]
+          } else {
+            Object.assign(packageContent[key], packageContent_[key])
+          }
         }
       }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/rules/no-extraneous-dependencies.ts` around lines 119 - 122, The loop
merging packageContent and packageContent_ uses Object.assign which overwrites
array fields (notably bundledDependencies) instead of merging them; update the
loop that iterates depsKey/packageContent[key] to detect when both
packageContent[key] and packageContent_[key] are arrays (e.g., key ===
'bundledDependencies') and concatenate them deduplicating entries (e.g., via a
Set), otherwise use Object.assign for plain objects or simple assignment for
primitives so arrays are merged rather than replaced.

}
}

Expand Down
31 changes: 25 additions & 6 deletions test/rules/no-extraneous-dependencies.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,31 @@ ruleTester.run('no-extraneous-dependencies', rule, {
{ packageDir: packageDirMonoRepoRoot, whitelist: ['not-a-dependency'] },
],
}),

// Closest package.json is always merged when packageDir is set.
// File is inside nested-package/ which has react, so react is found
// even though packageDir only points to monorepo root (which lacks react).
tValid({
code: 'import react from "react";',
filename: path.join(packageDirMonoRepoWithNested, 'foo.js'),
options: [{ packageDir: packageDirMonoRepoRoot }],
}),

// Deps from both the closest package.json AND packageDir are merged.
// right-pad is only in monorepo root, react is only in nested-package.
// File is in nested-package, packageDir points to root — both should be found.
tValid({
code: 'import rightpad from "right-pad";',
filename: path.join(packageDirMonoRepoWithNested, 'foo.js'),
options: [{ packageDir: packageDirMonoRepoRoot }],
}),

// When closest package.json IS the packageDir, behavior is unchanged.
tValid({
code: 'import rightpad from "right-pad";',
filename: path.join(packageDirMonoRepoRoot, 'foo.js'),
options: [{ packageDir: packageDirMonoRepoRoot }],
}),
],
invalid: [
tInvalid({
Expand Down Expand Up @@ -370,12 +395,6 @@ ruleTester.run('no-extraneous-dependencies', rule, {
filename: path.join(packageDirMonoRepoRoot, 'foo.js'),
errors: [{ messageId: 'missing', data: { packageName: 'react' } }],
}),
tInvalid({
code: 'import react from "react";',
filename: path.join(packageDirMonoRepoWithNested, 'foo.js'),
options: [{ packageDir: packageDirMonoRepoRoot }],
errors: [{ messageId: 'missing', data: { packageName: 'react' } }],
}),
tInvalid({
code: 'import "react";',
filename: path.join(packageDirWithEmpty, 'index.js'),
Expand Down
Loading