Skip to content

NXT-17571: Support npm lockfileVersion 2 and 3 in bootstrap --override#395

Open
bongsok wants to merge 5 commits into
developfrom
NXT-17571-bootstrap-lockfile-v3
Open

NXT-17571: Support npm lockfileVersion 2 and 3 in bootstrap --override#395
bongsok wants to merge 5 commits into
developfrom
NXT-17571-bootstrap-lockfile-v3

Conversation

@bongsok

@bongsok bongsok commented Jul 20, 2026

Copy link
Copy Markdown

Checklist

  • I have read and understand the contribution guide
  • A CHANGELOG entry is included
  • I have run automated testing and it is passed
  • Documentation was added or is not needed
  • This is an API breaking change

Issue Resolved / Feature Added

Fixes enact bootstrap --override not supporting npm package-lock.json with lockfileVersion 2 or 3.

The existing override logic only rewrites the top-level dependencies tree used by lockfileVersion 1. lockfileVersion 2 keeps that tree for backwards compatibility, but lockfileVersion 3 drops the dependencies tree and uses only the packages map. As a result, on a v3 lockfile the override silently found nothing to rewrite and no-oped.

Resolution

Added packages map handling to the override lockfile-rewriting logic in commands/bootstrap.js.

  • packages map (v2 & v3): sets each node_modules/<dep> entry's resolved to the local file:...tgz path and removes from/integrity. Also updates the root package ("") dependencies/devDependencies/optionalDependencies specifiers.
  • dependencies tree (v1, and the legacy tree kept in v2): behavior preserved for backwards compatibility.
  • Moved the lockfileVersion/requires metadata assignment out of the dep loop so it is set correctly even when no dependency matches.
  • Changed the lockfile check from an array index (i > 0) to a filename check (path.basename(f) !== 'package.json') to avoid a misidentification when package.json is absent.

Additional Considerations

Should be verified against projects whose package-lock.json uses lockfileVersion 1, 2, and 3 respectively, to confirm --override correctly swaps in the local tgz dependencies in each case.

Links

NXT-17571

Comments

Set `version` and `resolved` to the local file dependency and remove `from`,
`integrity`, and `requires`, matching the legacy `dependencies` tree logic.
@bongsok
bongsok force-pushed the NXT-17571-bootstrap-lockfile-v3 branch from a5a45f1 to 7727513 Compare July 20, 2026 06:19
A dependency can be installed at the top level or nested under another
package (e.g. node_modules/@enact/limestone/node_modules/@enact/core), so
rewrite every packages entry whose path resolves to the overridden package
instead of only the top-level one. Also drop `resolved` alongside the other
stale fields to mirror the lockfileVersion 1 handling.
Comment thread commands/bootstrap.js Outdated
Object.keys(obj.packages)
.filter(key => key === suffix || key.endsWith('/' + suffix))
.forEach(key => {
obj.packages[key].version = fileDep;

@daniel-stoian-lgp daniel-stoian-lgp Jul 20, 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.

In the "packages" map the file path lives in "resolved"

Suggested change
obj.packages[key].version = fileDep;
obj.packages[key].resolved= fileDep;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated

Comment thread commands/bootstrap.js Outdated
.forEach(key => {
obj.packages[key].version = fileDep;
// Remove unneeded properties to avoid issues
['resolved', 'from', 'integrity', 'requires'].forEach(

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.

Suggested change
['resolved', 'from', 'integrity', 'requires'].forEach(
['from', 'integrity', 'requires'].forEach(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated

Comment thread commands/bootstrap.js Outdated
);
} else {
local.forEach(dep => {
const fileDep = 'file:' + path.join(override, dep, 'package.tgz');

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.

fileDep is built with path.join(...), which emits \ on Windows, producing file:..\override....
npm can't resolve that (npm error enoent). Normalizing separators would fix the problem. please check

Suggested change
const fileDep = 'file:' + path.join(override, dep, 'package.tgz');
const fileDep = 'file:' + path.join(override, dep, 'package.tgz').split(path.sep).join('/');

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated

bongsok added 2 commits July 21, 2026 08:38
For the packages map (lockfileVersion 2 & 3), set `resolved` to the local
file dependency and drop `version` (npm derives it from the tarball), matching
how npm records local file installs. Also normalize the file dependency path
to POSIX separators so generated lockfiles are consistent on Windows.
Instead of rewriting nested packages entries
(node_modules/@enact/limestone/node_modules/@enact/core), remove them so the
dependent package resolves to the overridden top-level package
(node_modules/@enact/core). Only the top-level entry is repointed at the local
tgz via `resolved`, keeping its `version`.

@daniel-stoian-lgp daniel-stoian-lgp left a comment

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.

LGTM

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.

2 participants