Skip to content

Conversation

@serhalp
Copy link
Member

@serhalp serhalp commented Mar 31, 2025

Summary

We accidentally reintroduced dev dependencies in the npm-shrinkwrap.json file here: https://github.com/netlify/cli/pull/7119/files?file-filters%5B%5D=.js&file-filters%5B%5D=.json&file-filters%5B%5D=.md&file-filters%5B%5D=.yml&show-viewed-files=true&show-deleted-files=false#diff-3d80e6f9eb8af7a28cbe2209d086150e29b68f860376ee618a0041ca9b5c786dL18-L49.

The deleted script was deleting package.json#devDependencies before running npm i && npm shrinkwrap, which resulted in the shrinkwrap being built from a modified package-lock.json with dev deps removed 😓.

cc @ndhoule

@serhalp serhalp requested a review from a team as a code owner March 31, 2025 19:29
@github-actions
Copy link

github-actions bot commented Mar 31, 2025

📊 Benchmark results

Comparing with 90afd98

  • Dependency count: 1,171 (no change)
  • Package size: 283 MB (no change)
  • Number of ts-expect-error directives: 713 (no change)

@ndhoule ndhoule force-pushed the fix/no-dev-deps-in-shrinkwrap branch from 46f2064 to 22c57c4 Compare April 1, 2025 19:34
We accidentally reintroduced dev dependencies in the `npm-shrinkwrap.json` file here: https://github.com/netlify/cli/pull/7119/files?file-filters%5B%5D=.js&file-filters%5B%5D=.json&file-filters%5B%5D=.md&file-filters%5B%5D=.yml&show-viewed-files=true&show-deleted-files=false#diff-3d80e6f9eb8af7a28cbe2209d086150e29b68f860376ee618a0041ca9b5c786dL18-L49.

The deleted script was deleting `package.json#devDependencies` before running `npm i && npm shrinkwrap`, which resulted in the shrinkwrap being built from a modified `package-lock.json` with dev deps removed 😓.
@ndhoule ndhoule force-pushed the fix/no-dev-deps-in-shrinkwrap branch from 22c57c4 to 29de735 Compare April 1, 2025 19:37
Copy link
Member Author

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

LGTM (can't review because I originally opened it)

"test:unit": "vitest run tests/unit/",
"postinstall": "node ./scripts/postinstall.js",
"prepublishOnly": "npm shrinkwrap",
"prepublishOnly": "node ./scripts/prepublishOnly.js",
Copy link
Member Author

Choose a reason for hiding this comment

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

:skinner-children-wrong:

@ndhoule ndhoule merged commit 8e9421a into main Apr 1, 2025
52 checks passed
@ndhoule ndhoule deleted the fix/no-dev-deps-in-shrinkwrap branch April 1, 2025 20:25
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.

3 participants