Skip to content

feat(assets): Switch to pnpm and rspack#832

Open
sakshamarora1 wants to merge 2 commits into
CERNDocumentServer:masterfrom
sakshamarora1:feat/pnpm
Open

feat(assets): Switch to pnpm and rspack#832
sakshamarora1 wants to merge 2 commits into
CERNDocumentServer:masterfrom
sakshamarora1:feat/pnpm

Conversation

@sakshamarora1

Copy link
Copy Markdown
Contributor

No description provided.

executeSearch(defaultRun, "");
}
}, []);
}, [currentQueryState, runs, defaultRun]);

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.

could you check this warning?

Comment thread site/.npmrc
@@ -0,0 +1 @@
lockfile=false No newline at end of file

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.

what will this do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Disable package-lock.json

Comment thread pnpm-lock.yaml
@@ -0,0 +1,12382 @@
lockfileVersion: '9.0'

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.

this is quite sensitive change. If I remember correctly package-lock.json is used to build our deployment images - did you adapt that to the change, will the image still build correctly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is also how Zenodo and other instances are doing it as well.

Comment thread site/package.json
@@ -0,0 +1,17 @@
{
"name": "cds-rdm-site",

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.

why do we need to add package.json here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am also confused about this, it would maybe make sense if it was included as a workspace in the root level package.json and all of the dependencies were instead listed here. But I'm not sure why it's necessary in the current form.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is to fix the broken linter, purpose mentioned in the description already. For the dependencies for eslint.

@kpsherva kpsherva moved this from In review 🔍 to In progress in Sprint Q3 2026 ☀️ Jul 3, 2026
- name: Use Node.js v20.x
uses: actions/setup-node@v6
- name: Install pnpm
uses: pnpm/action-setup@v4

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any reason why we are not using v6 of the pnpm action?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No reason 👀
I will update

Comment thread site/package.json
@@ -0,0 +1,17 @@
{
"name": "cds-rdm-site",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am also confused about this, it would maybe make sense if it was included as a workspace in the root level package.json and all of the dependencies were instead listed here. But I'm not sure why it's necessary in the current form.

@kpsherva kpsherva moved this from In progress to Ready in Sprint Q3 2026 ☀️ Jul 6, 2026
@sakshamarora1 sakshamarora1 moved this from Ready to In review 🔍 in Sprint Q3 2026 ☀️ Jul 21, 2026
@sakshamarora1 sakshamarora1 removed their assignment Jul 21, 2026
Comment thread Dockerfile
COPY package-lock.json ${INVENIO_INSTANCE_PATH}/assets/
COPY ./assets/ ${INVENIO_INSTANCE_PATH}/assets/
COPY ./static/ ${INVENIO_INSTANCE_PATH}/static/
RUN cd ${INVENIO_INSTANCE_PATH}/assets && pnpm run build

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.

could you keep it the way it was? that we copy the file from the repo to instance assets and we use that to install later?

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