Skip to content

feat: update PNPM version handling in Dockerfile - #96

Merged
fenekku merged 1 commit into
inveniosoftware:masterfrom
Samk13:debian-update-args
Jul 10, 2026
Merged

feat: update PNPM version handling in Dockerfile#96
fenekku merged 1 commit into
inveniosoftware:masterfrom
Samk13:debian-update-args

Conversation

@Samk13

@Samk13 Samk13 commented Jun 3, 2026

Copy link
Copy Markdown
Member

❤️ Thank you for your contribution!

Description

  • Use ARG for PNPM_VERSION to allow dynamic versioning
  • Update npm install command to reference PNPM_VERSION

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Frontend

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@fenekku fenekku left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We would need to adapt invenio-cli to pass down the appropriate INVENIO_WEBPACKEXT_NPM_PKG_CLS to make sure things are coherent. But it's the right idea 👍

Comment thread debian/Dockerfile
ARG PYTHON_VERSION=3.14
ARG OS_VERSION=trixie
ARG NODE_VERSION=24
ARG PNPM_VERSION=latest-10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Comment thread debian/Dockerfile Outdated
@Samk13
Samk13 force-pushed the debian-update-args branch from 70712f1 to 1989e04 Compare June 3, 2026 14:03
@Samk13
Samk13 marked this pull request as ready for review June 3, 2026 14:07
@Samk13 Samk13 added this to v14 Jun 3, 2026
@Samk13 Samk13 moved this to 👀 In review in v14 Jun 3, 2026

@fenekku fenekku left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And then on invenio-cli side we can inject WEBPACKEXT_NPM_PKG_CLS .

Comment thread debian/Dockerfile Outdated
Comment thread debian/Dockerfile Outdated
@Samk13
Samk13 force-pushed the debian-update-args branch 3 times, most recently from 3efb141 to 505356e Compare June 4, 2026 13:27
* Redeclare PNPM_VERSION in the Dockerfile for scoping.
* Use PNPM_VERSION variable in the npm install command.
@Samk13
Samk13 force-pushed the debian-update-args branch from 505356e to c3caf81 Compare June 4, 2026 15:13
Comment thread debian/Dockerfile
@fenekku

fenekku commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for identifying these non-obvious behaviours @Samk13! I am happy to merge Friday morning (or you can when you feel appropriate).

@utnapischtim

utnapischtim commented Jun 11, 2026

Copy link
Copy Markdown

@fenekku

invenio-cli handles this already

https://github.com/inveniosoftware/invenio-cli/blob/d76ea66462f63905015007370be09555d156a2b5/invenio_cli/helpers/package_managers.py#L234

with javascript_package_manager in .invenio you can configure npm vs pnpm.

there is also the variable python_package_manager for uv

and rspack is configured over WEBPACKEXT_PROJECT in invenio.cfg for the sake of completeness

and inveniosoftware/invenio-cli#385 explains why rspack is configured in invenio.cfg and not in .invenio

@Samk13

Samk13 commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

Reading through the pnpm docs, it looks like we could potentially simplify this Dockerfile further by relying on pnpm runtime set node <version> -g to install Node.js as well.

Since we're already standardizing on pnpm, that could let us drop the NodeSource setup and the nodejs/npm apt packages entirely, and have pnpm manage both the package manager and runtime versions.

Might be worth exploring in a follow-up if it aligns with our support and deployment expectations.

WDYT @fenekku @utnapischtim?

@fenekku

fenekku commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@Samk13 Interesting 🤔 . I made a ticket for that: #98 so we can investigate after v14 release.

@fenekku
fenekku merged commit ff0593e into inveniosoftware:master Jul 10, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from 👀 In review to To release 🤖 in v14 Jul 10, 2026
@fenekku fenekku moved this from To release 🤖 to Done ✔️ in v14 Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done ✔️

Development

Successfully merging this pull request may close these issues.

3 participants