Skip to content

Conversation

@d3xter666
Copy link
Member

JIRA: CPOUI5FOUNDATION-1142

@d3xter666 d3xter666 marked this pull request as draft October 31, 2025 13:28
@d3xter666 d3xter666 marked this pull request as ready for review October 31, 2025 13:50
@d3xter666 d3xter666 requested a review from a team October 31, 2025 13:50
@d3xter666
Copy link
Member Author

d3xter666 commented Oct 31, 2025

Please find a sample result of this flow in here: d3xter666#139

Note: Please ignore the title, it has been updated later in the config

with:
node-version: 24.x

- name: Install and publish ${{ matrix.package }}
Copy link
Member

Choose a reason for hiding this comment

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

Special handling for the CLI package is required, where the shrinkwrap needs to be updated/created after all dependencies have been released. Should this be done in a separate PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!
I think we had something related to the shrinkwrap itself, but I can't find the discussion/BLI!

I have updated the action so that it:

  • Runs the publishing of all the packages in a sequence. This ensures that during npm ci for publishing for the next package in the chain, it will have the previous package already published on NPM.
  • For the CLI, it has a special step where all the packages are available already on NPM and it's been updated accordingly.

As per my research this is the approach many monorepos are wokring. Lerna, for example

@matz3
Copy link
Member

matz3 commented Nov 5, 2025

Please see my comment here: d3xter666#139 (comment)

@d3xter666 d3xter666 requested review from RandomByte and matz3 November 6, 2025 11:44
run: |
echo "📦 Installing production dependencies for CLI"
npm ci --omit=dev
- name: Generate npm-shrinkwrap.json (prod only)
Copy link
Member

Choose a reason for hiding this comment

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

Does this guarantee that the exact same transitive dependency versions from package-lock.json are used in the packages/cli/npm-shrinkwrap.json?

Copy link
Member Author

@d3xter666 d3xter666 Nov 6, 2025

Choose a reason for hiding this comment

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

Thanks for challenging this!
Now, the answer is Yes!

I have reworked it, so that it:

  1. runs npm ci for the monorepo -> Creates a package-lock.json
  2. then isolates the cli out of the monorepo (npm shrinkwrap does not work properly for workspaces)
  3. prunes unneeded dependencies from it
  4. creates npm shrinkwrap for the isolated cli package
  5. copies the generated npm-shrinkwrap.json back to the packages/cli
  6. Deploys cli to npm

Copy link
Member

Choose a reason for hiding this comment

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

I have tested this locally and compared the two lockfiles to see differences in package versions. Obviously they won't have the same content, but it helps to identify whether transitive dependencies are using the exact same version as in the existing package-lock.json. This is what I saw:

  1. The devDependencies are still included and would get installed by consumers (see https://github.com/UI5/linter/blob/01500503f8063ea9e639939ca6c515b332305580/.github/workflows/release-please.yml#L40-L54 for our current solution to this problem).
  2. I found at least one example where a transitive dependency uses a different (newer) version than defined in the package-lock.json. For https://www.npmjs.com/package/ci-info the shrinkwrap was using 4.3.1, while currently our lockfile uses 4.3.0 . See:
    "version": "4.3.0",

I'm very unhappy about the missing support from npm for such a scenario, but I don't see that we should use such a setup as it defeats the purpose of using a lockfile in the project. We will end up shipping a release with dependency version combinations that were never tested before. If there happens to be no solution to this problem, I would rather prefer to stop using a shrinkwrap, as it would at least allow consumers to upgrade the transitive packages on their end if issues occur. But not using a shrinkwrap for CLI packages is IMHO not a good idea at all and I would really like to continue using it.

@d3xter666 d3xter666 requested a review from matz3 November 6, 2025 13:54
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.

4 participants