Skip to content

Conversation

@d3xter666
Copy link
Member

@d3xter666 d3xter666 commented Dec 4, 2025

Enhances: #1231

@d3xter666 d3xter666 requested a review from a team December 4, 2025 11:30
@coveralls
Copy link

coveralls commented Dec 4, 2025

Coverage Status

coverage: 94.69% (+0.1%) from 94.552%
when pulling ce48200 on fix-shrinkwrap-generation
into d0976b0 on main.

@RandomByte
Copy link
Member

To verify the shrinkwrap:

npm i
cd packages/cli
node ../../internal/shrinkwrap-extractor/cli.js ../../
rm -rf node_modules # Delete node_modules within CLI package

# <Manually remove devDependencies in package.json>

npm ci --workspaces false

Before Matthias' fix this yielded the following error:

npm error Missing: @npmcli/package-json@7.0.4 from lock file
npm error Missing: @npmcli/package-json@7.0.4 from lock file
npm error Missing: globby@15.0.0 from lock file
npm error Missing: proc-log@6.1.0 from lock file
npm error Missing: proc-log@6.1.0 from lock file
npm error Missing: proc-log@5.0.0 from lock file
npm error Missing: proc-log@5.0.0 from lock file
npm error Missing: unicorn-magic@0.3.0 from lock file

Those dependencies where missing from the shrinkwrap, likely because multiple workspace packages dependent on different versions, overwriting each other.

I'm currently verifying the state after Matthias' fix. I suspect there might still be a problem with dependencies hoisted to the workspace root for packages other than the CLI, conflicting with dependencies of the CLI.

@RandomByte
Copy link
Member

I suspect there might still be a problem with dependencies hoisted to the workspace root for packages other than the CLI, conflicting with dependencies of the CLI.

I added a failing test for this case. I'll work on a fix

@d3xter666 d3xter666 force-pushed the fix-shrinkwrap-generation branch 2 times, most recently from 56e1877 to 47fde0f Compare December 5, 2025 19:33
@d3xter666 d3xter666 requested review from a team and matz3 December 5, 2025 19:40
"resolved": "https://registry.npmjs.org/package/version.tgz",
"integrity": "sha512-mock-integrity-hash"
},
"node_modules/@ui5/target/node_modules/@sapui5/some-thirdparty": {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will work. If @ui5/target is the root package, it will never search the path node_modules/@ui5/target/node_modules/ for its dependencies.

For module @sapui5/some-thirdparty, the target package would resolve it to node_modules/@sapui5/some-thirdparty, which contains the wrong version now...

@d3xter666 d3xter666 force-pushed the fix-shrinkwrap-generation branch from 47fde0f to 795c6f6 Compare December 8, 2025 12:06
@d3xter666 d3xter666 changed the title fix: Normalize workspace paths in npm-shrinkwrap.json fix: Improve shrinkwrap generation handling Dec 9, 2025
@d3xter666 d3xter666 force-pushed the fix-shrinkwrap-generation branch from 445b150 to 40d48f6 Compare December 9, 2025 11:53
@d3xter666 d3xter666 force-pushed the fix-shrinkwrap-generation branch from 50393a9 to ce48200 Compare December 15, 2025 07:42
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.

5 participants