Skip to content

chore(deps): convert package-lock.json to lockfileVersion 3 - #557

Open
tkislan wants to merge 2 commits into
mainfrom
tk/lockfile-v3
Open

tkislan wants to merge 2 commits into
mainfrom
tk/lockfile-v3

Conversation

@tkislan

@tkislan tkislan commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Converts the root package-lock.json from lockfileVersion 2 to 3. No package versions change: the packages section is byte-for-byte the same data, and the only removal is the legacy dependencies section that v2 kept for npm 5/6.

  • package-lock.json: +1 / −25,367. The diff is the lockfileVersion line plus the deleted legacy section.
  • build/ci/postInstall.js: verifyMomentIsOnlyUsedByJupyterLabCoreUtils threw unless the lockfile had both packages and dependencies, so on v3 every npm install/npm ci failed in postinstall. It now reads packages only, and checks optionalDependencies alongside dependencies. In main's lockfile, legacy requires equals dependencies ∪ optionalDependencies from packages for all 1,621 entries that have one, so the check covers the same declarations. Upstream vscode-jupyter has the same check and is still on v2.
  • osv-scanner.toml (new) and the comment above the qlty dependency scan in ci.yml: see below.

Why

v2 stores every package twice: once in packages and again in the nested legacy dependencies tree. Only npm 5/6 read the legacy tree. Every supported npm (7+) reads only packages, the repo pins Node 22 / npm 10.9.4 via .nvmrc, and the perf-test fixture lockfile is already v3. Dropping the copy shrinks the file from 61,378 to 36,012 lines. It also ends npm 11's rewriting of legacy requires lines, which used to show up as lock drift.

This reduces inflated dependency diffs but doesn't eliminate them. Git's default (Myers) diff stops searching for the best alignment in large, repetitive files. #556's real change of +280/−6,367 renders as +32,095/−38,182 on v2. On v3 the same change renders as +11,860/−15,491 (real: +120/−3,751). Most of the misalignment happens in the packages section, which v3 keeps. The legacy section alone diffs almost exactly (+316/−2,772 vs. a real +160/−2,616).

qlty now scans the root lockfile

Converting to v3 also turns on qlty's osv-scanner check for the root package-lock.json. qlty silently skips files over 2,098,000 bytes (MAX_FILE_SIZE):

Root lockfile Size qlty osv-scanner
v2 (main) 2,789,530 bytes skipped, which is why main reports "No issues"
v3 (this PR) 1,555,451 bytes scanned

The first scan reported the three advisories that .nsprc already accepts for better-npm-audit: elliptic GHSA-848j-6mx2-7j84, braces GHSA-vfj7-8cjw-p6xm and sprintf-js GHSA-hp3w-g68c-fv3c. They're copied into a root osv-scanner.toml with the same expiry dates; osv-scanner reads it from the lockfile's own directory, so the perf-test fixture is unaffected. The ci.yml comment claiming the root lockfile is "too large for qlty to analyze" is updated.

From now on:

  • Every accepted risk for the root lockfile goes in both .nsprc and osv-scanner.toml.
  • If the lockfile grows past 2,098,000 bytes (+35%), qlty silently stops scanning it again.

How it was converted

Per the npm docs, an existing lockfile is converted when --lockfile-version is set (lockfile-version, npm/cli#5605), and --package-lock-only works from the lockfile alone, ignoring node_modules:

npm install --package-lock-only --lockfile-version=3 --ignore-scripts   # Node 22.21.1 / npm 10.9.4 (.nvmrc)

A full npm install --lockfile-version=3 from the same starting point produces a byte-identical file. No .npmrc is needed: npm 7+ defaults to "maintain current lockfile version" (v8, v10 docs).

Verification

  • No version changes: assert.deepStrictEqual passes on the packages section (2,909 entries) of main vs this branch. Every other top-level field is equal too, except lockfileVersion and the removed dependencies.
  • Same installed tree: npm ci --ignore-scripts from the v3 lockfile produces a node_modules/.package-lock.json deep-equal to an install from main's v2 lockfile (2,827 packages).
  • postinstall:
    • A real npm ci with scripts on this branch passes.
    • The moment check, run in a harness against the v3 lockfile, passes.
    • It still throws when a fake package declares moment in dependencies or in optionalDependencies.
    • The old version throws Invalid package-lock.json, as it does not contain the key 'dependencies' on v3.
  • Lock drift: a fresh npm install leaves the lockfile unchanged with npm 10.9.4 (CI) and with npm 11.19.0. On v2, npm 11 rewrote legacy requires lines; that section is now gone.
  • Audits: npx better-npm-audit audit --production and npx better-npm-audit audit both pass.
  • Qlty Check: the dependency scan failed with the 3 advisories on the first commit and passes with osv-scanner.toml (no other change in between). Its IDs and expiry dates match .nsprc exactly.
  • npm run format passes.

Notes

  • Renovate's npm version changes. Renovate manages the root lockfile, and it picks npm from lockfileVersion when package.json has no npm constraint (source). v2 → npm <9 (8.x), so far. v3 → npm >=7, i.e. a current npm. To pin Renovate to the .nvmrc npm instead, constraints.npm can be set in renovate.json. Not done here.
  • Dependabot only manages src/test/vscode-notebook-perf (already v3), and it has supported v3 since March 2023.
  • The unused gulp task checkNpmDependencies already skips a missing dependencies key.

🤖 Generated with Claude Code

https://claude.ai/code/session_01USEnGCRP32gQH4AZHbRVqQ

Summary by CodeRabbit

  • Chores
    • Updated build-time dependency checks to inspect declared and optional dependencies in package-lock entries; Moment usage outside the permitted packages continues to trigger an error.
    • Updated dependency scanning notes and recorded three time-limited vulnerability exceptions for the root lockfile, with risk details.
    • No changes to end-user features or behavior.

Converted with `npm install --package-lock-only --lockfile-version=3
--ignore-scripts` (npm 10.9.4). The `packages` section is unchanged;
only the legacy npm 6 `dependencies` section is dropped.

postInstall's moment check required the legacy section, so it now reads
`packages` only, and checks `optionalDependencies` as well, which the
legacy `requires` field used to cover.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01USEnGCRP32gQH4AZHbRVqQ
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 0f981edf-ad03-4e13-8183-2a577ed15ae7
📥 Commits

Reviewing files that changed from the base of the PR and between 2706808 and 57f5e72.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • osv-scanner.toml

Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

The Moment usage check now scans only the package-lock packages map. It checks each package’s dependencies and optionalDependencies. The CI comments describe the root lockfile’s Qlty file-size limit and point to osv-scanner.toml for accepted risks. The scanner configuration adds three vulnerability-ignore entries with expiration dates and reasons.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: jamesbhobbs

Merge Risk: ⚪ Minimal · up to 57f5e

No concrete merge-blocking risk is introduced by this PR.

🚥 Pre-merge checks | ✅ 5 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Updates Docs ❓ Inconclusive The pull request changes only CI, post-install logic, the root lockfile, and osv-scanner.toml; it adds no documentation file. The required documentation repositories (deepnote/deepnote and the pri… Please verify and update the primary documentation in deepnote/deepnote and the roadmap on the landing page in deepnote/deepnote-internal. Re-run this check when those repositories are available.
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main change: converting package-lock.json to lockfileVersion 3.
Full details: Updates Docs

Explanation

The pull request changes only CI, post-install logic, the root lockfile, and osv-scanner.toml; it adds no documentation file. The required documentation repositories (deepnote/deepnote and the private deepnote/deepnote-internal roadmap) are not available for inspection.

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38%. Comparing base (595792c) to head (57f5e72).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #557   +/-   ##
=====================================
  Coverage     38%     38%           
=====================================
  Files        822     822           
  Lines      41098   41098           
  Branches    9044    9044           
=====================================
  Hits       15620   15620           
  Misses     23405   23405           
  Partials    2073    2073           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 7, 2026
The v3 root package-lock.json is 1,555,451 bytes, under qlty's
2,098,000-byte file limit, so osv-scanner now scans it for the first
time (v2 was 2,789,530 bytes and silently skipped). It reports the three
accepted risks .nsprc already excepts for better-npm-audit, so mirror
them in osv-scanner.toml with the same expiry dates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01USEnGCRP32gQH4AZHbRVqQ
@tkislan
tkislan marked this pull request as ready for review October 7, 2026 20:11
@tkislan
tkislan requested a review from a team as a code owner October 7, 2026 20:11
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