fix(cli): don't re-run npm install on every TUI launch with npm>=10 reduced hidden lockfile
_tui_need_npm_install compared every field of the root package-lock.json
against node_modules/.package-lock.json. npm>=10/11 writes a reduced hidden
lockfile that omits declarative fields (version/dependencies/dev) and adds
extraneous, so nearly every package looked 'changed'; workspace link entries
("link": true, paths outside node_modules/) are never materialized by the
partial --workspace install. Both made the check return True forever, so
hermes --tui re-ran npm install (and dirtied package-lock.json) on every
launch (#84617).
Compare only the keys both sides record with non-null values (resolved,
integrity, ...), ignore workspace link entries and non-node_modules paths in
the missing-entry check, and treat extraneous as an npm runtime annotation.
Real skew (lockfile bumped while node_modules is behind) is still detected.
This commit is contained in:
+23
-4
@@ -2019,8 +2019,9 @@ installed, so we exclude them from the comparison in :func:`_tui_need_npm_instal
|
||||
to avoid false-positive reinstalls on every launch.
|
||||
|
||||
``dev``, ``optional``, ``extraneous``, and ``hasInstallScript`` are boolean
|
||||
annotations that npm populates differently in the hidden lock (e.g. ``dev: true``
|
||||
from the root lock may be absent or ``false`` in the hidden actualized tree).
|
||||
annotations that npm populates differently in the hidden lock (npm >= 10/11
|
||||
writes ``extraneous`` into the hidden lock only, and ``dev: true`` from the
|
||||
root lock may be absent or ``false`` in the hidden actualized tree).
|
||||
They never indicate a changed dependency — the authoritative check is the
|
||||
``resolved``/``integrity`` pair, which the intersection comparison always
|
||||
catches.
|
||||
@@ -2246,7 +2247,18 @@ def _tui_need_npm_install(root: Path) -> bool:
|
||||
return lock.stat().st_mtime > marker.stat().st_mtime
|
||||
|
||||
def comparable(pkg: dict) -> dict:
|
||||
return {k: v for k, v in pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS}
|
||||
# npm >= 10/11 writes a reduced hidden lockfile that omits declarative
|
||||
# fields (`version`, `dependencies`, `dev`, `engines`, `bin`, ...) or
|
||||
# stores them as null. Comparing every key field-by-field then flags
|
||||
# nearly every installed package as "changed". Compare only the keys
|
||||
# both sides actually record with a non-null value (`resolved`,
|
||||
# `integrity`, ...): a genuinely stale install (root lockfile bumped
|
||||
# while node_modules is behind) still differs on those keys, while the
|
||||
# reduced-lockfile field omissions no longer false-positive.
|
||||
a = {k: v for k, v in pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS}
|
||||
b = {k: v for k, v in installed_pkg.items() if k not in _NPM_LOCK_RUNTIME_KEYS}
|
||||
common = a.keys() & b.keys()
|
||||
return {k: a[k] for k in common if a[k] is not None and b[k] is not None}
|
||||
|
||||
# In a shared workspace checkout the launch install is scoped to the ui-tui
|
||||
# workspace (plus its child packages/* workspaces on Termux), so only that
|
||||
@@ -2272,7 +2284,14 @@ def _tui_need_npm_install(root: Path) -> bool:
|
||||
continue
|
||||
|
||||
if name not in installed:
|
||||
if pkg.get("optional") or pkg.get("peer"):
|
||||
# Workspace link entries (`"link": true`, paths outside
|
||||
# node_modules/ like `apps/desktop`, `node_modules/web`) are never
|
||||
# materialized by a partial `npm install --workspace ui-tui` —
|
||||
# they're deliberately skipped (see #38772) and would otherwise
|
||||
# force a reinstall on every launch.
|
||||
if pkg.get("optional") or pkg.get("peer") or pkg.get("link"):
|
||||
continue
|
||||
if not name.startswith("node_modules/"):
|
||||
continue
|
||||
return True
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""_tui_need_npm_install: auto npm when node_modules is behind the lockfile."""
|
||||
|
||||
import json
|
||||
import os
|
||||
import types
|
||||
from pathlib import Path
|
||||
@@ -565,6 +566,135 @@ def test_make_tui_argv_exits_with_recovery_hint_when_workspace_unrecoverable(
|
||||
# for the install cwd — both use _workspace_root(sub).
|
||||
|
||||
|
||||
def test_need_npm_install_false_with_reduced_npm11_hidden_lockfile(
|
||||
tmp_path: Path, main_mod
|
||||
) -> None:
|
||||
"""npm >= 10/11 writes a reduced hidden `.package-lock.json` that omits
|
||||
declarative fields (version/dependencies/dev) and adds `extraneous`,
|
||||
and it never materializes workspace `"link": true` entries. A fresh
|
||||
install therefore used to look perpetually stale and re-ran `npm install`
|
||||
on every TUI launch (#84617). After the fix it must be stable."""
|
||||
ws = tmp_path / "ui-tui"
|
||||
ws.mkdir()
|
||||
(ws / "package.json").write_text("{}")
|
||||
_touch_ink(tmp_path)
|
||||
(tmp_path / "package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"version": "5.0.0",
|
||||
"resolved": "https://reg/ink.tgz",
|
||||
"integrity": "sha512-aaaa",
|
||||
"dependencies": {"yocto": "^1.0.0"},
|
||||
},
|
||||
"apps/desktop": {"link": True, "resolved": "apps/desktop"},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
# Hidden lockfile as npm 11 writes it: reduced, plus extraneous.
|
||||
(tmp_path / "node_modules").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "node_modules" / ".package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"resolved": "https://reg/ink.tgz",
|
||||
"integrity": "sha512-aaaa",
|
||||
"extraneous": True,
|
||||
},
|
||||
"apps/desktop": {"link": True, "resolved": "apps/desktop"},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
# Must be False: real skew keys (resolved/integrity) match, declarative
|
||||
# omissions and extraneous are ignored, and the workspace link is skipped.
|
||||
assert main_mod._tui_need_npm_install(ws) is False
|
||||
|
||||
|
||||
def test_need_npm_install_true_when_resolved_drifts(tmp_path: Path, main_mod) -> None:
|
||||
"""A genuinely stale install (lockfile bumped the resolved URL/integrity
|
||||
while node_modules is behind) must still be detected — the reduced-lockfile
|
||||
fix must not paper over real skew (#84617)."""
|
||||
ws = tmp_path / "ui-tui"
|
||||
ws.mkdir()
|
||||
(ws / "package.json").write_text("{}")
|
||||
_touch_ink(tmp_path)
|
||||
(tmp_path / "package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"version": "5.0.0",
|
||||
"resolved": "https://reg/ink-NEW.tgz",
|
||||
"integrity": "sha512-bbbb",
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
(tmp_path / "node_modules").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "node_modules" / ".package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"resolved": "https://reg/ink-OLD.tgz",
|
||||
"integrity": "sha512-aaaa",
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
# resolved/integrity differ on both sides → must reinstall.
|
||||
assert main_mod._tui_need_npm_install(ws) is True
|
||||
|
||||
|
||||
def test_need_npm_install_true_when_regular_pkg_missing(tmp_path: Path, main_mod) -> None:
|
||||
"""A real non-link node_modules/ package missing from the install must
|
||||
still trigger a reinstall — only workspace links and optional/peer skips
|
||||
are exempt (#84617)."""
|
||||
ws = tmp_path / "ui-tui"
|
||||
ws.mkdir()
|
||||
(ws / "package.json").write_text("{}")
|
||||
_touch_ink(tmp_path)
|
||||
(tmp_path / "package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"resolved": "https://reg/ink.tgz",
|
||||
"integrity": "sha512-aaaa",
|
||||
},
|
||||
"node_modules/missing-pkg": {
|
||||
"resolved": "https://reg/missing.tgz",
|
||||
"integrity": "sha512-cccc",
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
(tmp_path / "node_modules").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "node_modules" / ".package-lock.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"packages": {
|
||||
"node_modules/ink": {
|
||||
"resolved": "https://reg/ink.tgz",
|
||||
"integrity": "sha512-aaaa",
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
assert main_mod._tui_need_npm_install(ws) is True
|
||||
|
||||
|
||||
def test_no_stray_lockfiles_in_workspace_subdirs(main_mod) -> None:
|
||||
"""Workspace sub-directories must not contain their own package-lock.json.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user