Skip to content

Conversation

@pedrosousa13
Copy link
Owner

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds improved support for dev dependencies when running lnpm add, including preserving devDependencies placement for packages that are already listed there.

Changes:

  • Update updatePackageJSON to select devDependencies when appropriate and remove duplicate entries across dependency fields.
  • Add a regression test ensuring lnpm add (and no --dev) keeps an existing dev dependency in devDependencies.
  • Adjust progress-line clearing spacing in pack/store workflows.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/add_test.go Adds coverage for preserving devDependencies location when re-adding without --dev.
internal/cli/add.go Implements dependency-field selection logic and de-duplication when updating package.json.
internal/store/store.go Tweaks console progress-line clearing width after large operations.
internal/pack/pack.go Tweaks console progress-line clearing width after large scans.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 150 to 155
data, _ = os.ReadFile(pkgJSONPath)
var result map[string]interface{}
json.Unmarshal(data, &result)

deps := result["dependencies"]
devDeps := result["devDependencies"].(map[string]interface{})
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The test ignores the error from json.Unmarshal and then does an unchecked type assertion on result["devDependencies"]. If package.json can't be read/parsed, this can panic and obscure the real failure. Please check the ReadFile/Unmarshal errors and guard the type assertion (or reuse the existing helper accessors used elsewhere in tests).

Suggested change
data, _ = os.ReadFile(pkgJSONPath)
var result map[string]interface{}
json.Unmarshal(data, &result)
deps := result["dependencies"]
devDeps := result["devDependencies"].(map[string]interface{})
data, err := os.ReadFile(pkgJSONPath)
if err != nil {
t.Fatalf("Failed to read package.json: %v", err)
}
var result map[string]interface{}
if err := json.Unmarshal(data, &result); err != nil {
t.Fatalf("Failed to unmarshal package.json: %v", err)
}
deps := result["dependencies"]
devDepsVal, ok := result["devDependencies"]
if !ok {
t.Fatalf("package.json missing devDependencies field")
}
devDeps, ok := devDepsVal.(map[string]interface{})
if !ok {
t.Fatalf("devDependencies has unexpected type: %T", devDepsVal)
}

Copilot uses AI. Check for mistakes.

// Determine which field to use:
// - If --dev flag: use devDependencies
// - Else if package exists in devDependencies: use devDependencies (preserve location)
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

The comment says “if package exists in devDependencies: use devDependencies (preserve location)”, but the condition only preserves devDependencies when the package is NOT also in dependencies (!inDeps && inDevDeps). Either adjust the condition to match the documented behavior, or update the comment to reflect the actual precedence when a package appears in both fields.

Suggested change
// - Else if package exists in devDependencies: use devDependencies (preserve location)
// - Else if package exists only in devDependencies (and not in dependencies): use devDependencies

Copilot uses AI. Check for mistakes.
@pedrosousa13 pedrosousa13 merged commit 160a321 into main Jan 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant