switched docker to use pnpm instead of npm#805
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request transitions the frontend Docker build processes from using 'npm' to 'pnpm'. This change aims to standardize the package manager used within the Docker environments, potentially leading to more efficient dependency management and consistent builds across development and production setups. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully switches the frontend Dockerfiles from npm to pnpm. The changes are mostly correct, but I've identified a couple of areas for improvement to ensure build reproducibility and correctness. Specifically, the pnpm version should be pinned to avoid unexpected changes from @latest, and the pnpm-workspace.yaml file should be copied before installing dependencies to ensure the workspace configuration is respected. These changes will make the Docker builds more robust and predictable.
| # Copy package files and install dependencies | ||
| COPY SparkyFitnessFrontend/package.json SparkyFitnessFrontend/package-lock.json ./ | ||
| RUN npm install | ||
| COPY SparkyFitnessFrontend/package.json SparkyFitnessFrontend/pnpm-lock.yaml ./ |
There was a problem hiding this comment.
The pnpm-workspace.yaml file is not being copied before pnpm install. This file can contain important workspace configuration that affects how dependencies are installed. To ensure a consistent and correct installation, it should be copied along with package.json and pnpm-lock.yaml.
COPY SparkyFitnessFrontend/package.json SparkyFitnessFrontend/pnpm-lock.yaml SparkyFitnessFrontend/pnpm-workspace.yaml ./
| # Install pnpm | ||
| RUN corepack enable && corepack prepare pnpm@latest --activate | ||
|
|
||
| COPY SparkyFitnessFrontend/package.json SparkyFitnessFrontend/pnpm-lock.yaml ./ |
There was a problem hiding this comment.
The pnpm-workspace.yaml file is not being copied before pnpm install. This file can contain important workspace configuration that affects how dependencies are installed. To ensure a consistent and correct installation, it should be copied along with package.json and pnpm-lock.yaml.
COPY SparkyFitnessFrontend/package.json SparkyFitnessFrontend/pnpm-lock.yaml SparkyFitnessFrontend/pnpm-workspace.yaml ./
| WORKDIR /app/SparkyFitnessFrontend | ||
|
|
||
| # Install pnpm | ||
| RUN corepack enable && corepack prepare pnpm@latest --activate |
There was a problem hiding this comment.
For reproducible builds, it's best to pin the version of pnpm instead of using @latest. This prevents unexpected issues if a new major version of pnpm is released with breaking changes. Based on your pnpm-lock.yaml, you are using a version compatible with lockfile version 9, which corresponds to pnpm v9.
RUN corepack enable && corepack prepare pnpm@9 --activate
| WORKDIR /app/SparkyFitnessFrontend | ||
|
|
||
| # Install pnpm | ||
| RUN corepack enable && corepack prepare pnpm@latest --activate |
There was a problem hiding this comment.
For reproducible builds, it's best to pin the version of pnpm instead of using @latest. This prevents unexpected issues if a new major version of pnpm is released with breaking changes. Based on your pnpm-lock.yaml, you are using a version compatible with lockfile version 9, which corresponds to pnpm v9.
RUN corepack enable && corepack prepare pnpm@9 --activate
Tip
Help us review and merge your PR faster!
Please ensure you have completed the Checklist below.
For Frontend changes, please run
pnpm run validateto check for any errors.PRs that include tests and clear screenshots are highly preferred!
Description
Provide a brief summary of your changes.
Related Issue
PR type [x] Issue [ ] New Feature [ ] Documentation
Linked Issue: # #803
Checklist
Please check all that apply:
pnpm run validate(especially for Frontend).en) translation file (if applicable).rls_policies.sqlfor any new user-specific tables.Screenshots (if applicable)
Before
[Insert screenshot/GIF here]
After
[Insert screenshot/GIF here]