fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: rename npm-shrinkwrap.json to package-lock.json - #1129

Closed
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock
Closed

fix: rename npm-shrinkwrap.json to package-lock.json#1129
MikeMcC399 wants to merge 1 commit into
nodejs:mainfrom
MikeMcC399:convert-to-package-lock

Conversation

@MikeMcC399

Copy link
Copy Markdown
Contributor

Situation

npm 12 drops npm-shrinkwrap.json used in this repo as a published lockfile.

The npm 12.0.0 changelog states:

npm shrinkwrap is removed, the shrinkwrap config alias is removed, and npm-shrinkwrap.json is no longer loaded or honored at the project root or from inside dependency tarballs. Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

npm install under npm 12 installs dependencies according to their SemVer range specified in package.json and ignores the lower locked versions specified in npm-shrinkwrap.json.

Change

To ensure that dependencies are consistently installed by downstream consumers of citgm, whichever version of npm is being used to execute the Installation instructionsnpm install -g citgm, rename:

npm-shrinkwrap.json to package-lock.json.

This should be considered a breaking change (semver-major) when the next release is cut.

Checklist
  • npm test passes (for currently bundled versions of npm 10.x & 11.x)
  • contribution guidelines followed
    here

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (eef880d) to head (93baef8).

Additional details and impacted files
@@ Coverage Diff @@## main #1129 +/- ##
=======================================
Coverage 96.20% 96.20% =======================================
Files 29 29 Lines 2213 2213 =======================================
Hits 2129 2129 Misses 84 84 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399
MikeMcC399 marked this pull request as ready for review August 19, 2026 11:46
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from e7300df to 21412aaCompareAugust 21, 2026 13:03
npm 12 does not support npm-shrinkwrap.json
semver-major change since npm install -g citgm will
install dependencies according to the published package.json,
ensuring consistency independent of npm version used.
package-lock.json is only used for repo development and test.
Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
@MikeMcC399
MikeMcC399force-pushed the convert-to-package-lock branch from 21412aa to 93baef8CompareAugust 23, 2026 08:55
@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

@targos

This would need a review, if you are available.

I imagine that npm 12 will at some stage land on main in nodejs/node, and then when builds for Node.js 27 Alpha start to be tested in Sept / Oct 2026, they would be using npm 12, which ignores npm-shrinkwrap.json and so global installs of citgm would fallback to using package.json.

Making this change now would make sure that any issues resulting from installing with package.json are surfaced now instead of later when npm 12 starts being used. I'm not aware of any such issues, so this move would be purely defensive for the future.

@targos

Copy link
Copy Markdown
Member

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I think we should solve it the same way as in node-core-utils for consistency: nodejs/node-core-utils#1135

I'm not sure that is the best way for citgm if the objective is to catch compatibility issues early, although bundleDependencies is one of the alternatives suggested by the warning message:

Rename project-root npm-shrinkwrap.json to package-lock.json; use bundleDependencies if you need to ship a locked dependency tree.

Allowing global installs to use only package.json would re-evaluate the SemVer ranges each time citgm is installed, for instance in Jenkins. That means testing against latest of the configured ranges, rather than being pinned to older versions.

I don't have the history in this repo to judge what is best, so if this PR doesn't fit the needs then please go ahead and close it. I'll leave it to the citgm team then to resolve the issue as it sees best.

@targos

Copy link
Copy Markdown
Member

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

citgm is run on relatively sensitive infrastructure. We are using shrinkwrap to lock down the dependencies so that in case of compromise, we stay unaffected. My opinion is that safety is more important but I would like to hear what other people from @nodejs/citgm think about it.

citgm is pulling in unpinned packages listed in lib/lookup.json, any of which could be potentially compromised, so citgm should be run in a sandboxed environment where it cannot cause damage. I'm not sure that pinning the dependencies that citgm itself uses would provide a security advantage in that case. @nodejs/citgm will have their own views on that I expect.

Independent of this discussion, perhaps .npmrc should start using min-release-age? A typical value would be 3, 5 or 7 days.

@MikeMcC399

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this PR, since the current npm-shrinkwrap.json is outdated and causes:

32 vulnerabilities (1 low, 6 moderate, 24 high, 1 critical)

to be installed with citgm. The use of npm-shrinkwrap.json also means that npm audit fix can't remediate the vulnerabilities on a downstream system.

I suggest first to update the pinned dependencies before moving them to any other variation including bundleDependencies.

Sign up for freeto 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.

npm 12 - drops npm-shrinkwrap.json

3 participants

@MikeMcC399@codecov-commenter@targos