Skip to content

Switch from NPM to PNPM for package management #1818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 27, 2024
Merged
2 changes: 2 additions & 0 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ jobs:
steps:
-
uses: actions/checkout@v3
-
uses: pnpm/action-setup@v4
-
name: Playwright E2E Tests
run: make e2e-tests-ci browser=${{ matrix.browser }} shard="${{ matrix.shard }}/${{ matrix.shards }}"
Expand Down
14 changes: 10 additions & 4 deletions .github/workflows/integrate-and-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,18 @@ jobs:
TAG_LFMERGE=${TAG_LFMERGE:-latest}
echo "TAG_LFMERGE=${TAG_LFMERGE}" >> $GITHUB_OUTPUT
-
uses: actions/setup-node@v3
uses: pnpm/action-setup@v4
-
uses: actions/setup-node@v4
with:
node-version: '16.14.0'
cache: 'npm'
node-version: '22.2.0'
cache: 'pnpm'
-
run: pnpm install
-
run: npm ci
name: Prep for production build of legacy app
# docker compose build doesn't allow passing a `--target` parameter, so we have to replace the target in docker-compose.yml
run: "sed -i 's/target: development/target: production/g' docker-compose.yml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of this could we introduce a merge and a prod build yml file? That way if we need to make other tweaks for prod we can do it easily via adding changes to the file?
then our compose might look like this docker compose -f compose.yml -f compose.prod.yml up -d where the prod just contains the different targets we want, very similar to kustomzie actually.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for the compose merge solution. That could also be a separate PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll make it a separate PR to minimize the chances of merge conflicts (e.g., #1820 also touches workflows and I expect one merge conflict there, which will be easily resolved but if I'm making larger changes to the build process I might get harder-to-handle conflicts).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolving conversation for now, just to unlock the merge button. Then I'll come back and unresolve it so that the suggestion is easy for me to find later.

-
name: Build legacy app
run: docker compose build --build-arg ENVIRONMENT=production --build-arg BUILD_VERSION=${{ steps.image.outputs.TAG_APP }} app
Expand Down
12 changes: 7 additions & 5 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,14 @@ jobs:
-
uses: actions/checkout@v3
-
uses: actions/setup-node@v3
uses: pnpm/action-setup@v4
-
uses: actions/setup-node@v4
with:
node-version: "16.14.0"
cache: "npm"
node-version: "22.2.0"
cache: "pnpm"
-
name: Run prettier check
run: |
npx prettier -v
npx prettier --check .
pnpm dlx prettier -v
pnpm dlx prettier --check .
2 changes: 1 addition & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npx lint-staged
pnpm exec lint-staged
4 changes: 2 additions & 2 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
"label": "npm install",
"type": "shell",
"windows": {
"command": "bash -c 'npm install'"
"command": "bash -c 'pnpm install'"
},
"command": "npm install",
"command": "pnpm install",
"presentation": {
"reveal": "silent"
},
Expand Down
30 changes: 15 additions & 15 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

.PHONY: start
start: build
# starts the entire runtime infrastructure
# starts the entire runtime infrastructure
docker compose up -d ssl

.PHONY: dev
Expand All @@ -14,22 +14,22 @@ ui-builder:

.PHONY: e2e-tests-ci
e2e-tests-ci:
npm ci
$(MAKE) e2e-app
npx playwright install ${browser} --with-deps
npx playwright test -c ./test/e2e/playwright.config.ts --project=${browser} --shard=${shard}
pnpm install
"$(MAKE)" e2e-app
pnpm exec playwright install ${browser} --with-deps
pnpm exec playwright test -c ./test/e2e/playwright.config.ts --project=${browser} --shard=${shard}

.PHONY: e2e-tests
e2e-tests: ui-builder
npm install
$(MAKE) e2e-app
npx playwright install chromium firefox
npx playwright test -c ./test/e2e/playwright.config.ts $(params)
pnpm install
"$(MAKE)" e2e-app
pnpm exec playwright install chromium firefox
pnpm exec playwright test -c ./test/e2e/playwright.config.ts $(params)

.PHONY: e2e-app
e2e-app:
# delete any cached session storage state files if the service isn't running
docker compose ps e2e-app > /dev/null 2>&1 || $(MAKE) clean-test
# delete any cached session storage state files if the service isn't running
docker compose ps e2e-app > /dev/null 2>&1 || "$(MAKE)" clean-test
docker compose up -d e2e-app --build

.PHONY: unit-tests
Expand All @@ -45,9 +45,9 @@ unit-tests-ci:

.PHONY: build
build:
npm install
pnpm install

# ensure we start with a clean ui-dist volume for every build
# ensure we start with a clean ui-dist volume for every build
-docker volume rm web-languageforge_lf-ui-dist 2>/dev/null

docker compose build mail app lfmerge ld-api next-proxy next-app
Expand Down Expand Up @@ -83,11 +83,11 @@ clean:

.PHONY: clean-test
clean-test:
cd test/e2e && npx rimraf test-storage-state
cd test/e2e && pnpm dlx rimraf test-storage-state

.PHONY: clean-powerwash
clean-powerwash: clean
$(MAKE) clean-test
"$(MAKE)" clean-test
docker system prune -f --volumes
- docker rmi -f `docker images -q "lf-*"` sillsdev/web-languageforge:base-php
docker builder prune -f
6 changes: 4 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ services:
- ./webpack-dev.config.js:/data/webpack-dev.config.js
- ./webpack-prd.config.js:/data/webpack-prd.config.js
- ./package.json:/data/package.json
- ./package-lock.json:/data/package-lock.json
- ./pnpm-lock.yaml:/data/pnpm-lock.yaml

# needed these volume maps so changes to typescript/scss would be reflected in running app, actually rebundled and output to dist which is then shared to the app container.
- ./src/angular-app:/data/src/angular-app
Expand All @@ -34,6 +34,7 @@ services:
build:
context: .
dockerfile: docker/app/Dockerfile
target: development
args:
- ENVIRONMENT=development
image: lf-app
Expand Down Expand Up @@ -178,7 +179,7 @@ services:
container_name: lf-next-app-dev
environment:
- API_HOST=http://app
command: npm run dev-w-host
command: pnpm run dev-w-host
volumes:
# for developer convenience
- ./next-app/src:/app/src
Expand Down Expand Up @@ -269,6 +270,7 @@ services:
build:
context: .
dockerfile: docker/app/Dockerfile
target: development
args:
- ENVIRONMENT=development
image: lf-e2e-app
Expand Down
36 changes: 22 additions & 14 deletions docker/app/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@
ARG ENVIRONMENT=${ENVIRONMENT:-'production'}

# UI-BUILDER
FROM node:22.0.0-alpine3.18 AS ui-builder-base
FROM node:22.2.0-alpine3.18 AS ui-builder-base

RUN mkdir -p /data
WORKDIR /data

# Copy in files needed for compilation, located in the repo root
COPY typings ./typings/
COPY package.json webpack.config.js webpack-dev.config.js webpack-prd.config.js tsconfig.json tslint.json ./

# Use built node_modules from host
COPY node_modules ./node_modules/
COPY package.json pnpm-lock.yaml ./

# node-sass is platform dependent and so must be rebuilt inside docker
RUN npm rebuild node-sass
RUN corepack enable
RUN pnpm install

COPY typings ./typings/
COPY webpack.config.js webpack-dev.config.js webpack-prd.config.js tsconfig.json tslint.json ./
# copy in src local files
# Note: *.html files in src/angular-app aren't necessary for webpack compilation, however changes to HTML files will invalidate this layer
COPY src/angular-app ./src/angular-app
Expand All @@ -33,7 +31,7 @@ ENV NPM_BUILD_SUFFIX=dev
FROM ${ENVIRONMENT}-ui-builder AS ui-builder

# artifacts built to /data/src/dist
RUN npm run build:${NPM_BUILD_SUFFIX}
RUN pnpm run build:${NPM_BUILD_SUFFIX}

# COMPOSER-BUILDER
# download composer app dependencies
Expand All @@ -44,23 +42,24 @@ WORKDIR /composer
COPY src/composer.json src/composer.lock /composer/
RUN composer install

# PRODUCTION IMAGE
FROM sillsdev/web-languageforge:base-php AS production-app
# PRODUCTION BASE IMAGE
FROM sillsdev/web-languageforge:base-php AS production-base
RUN rm /usr/local/bin/install-php-extensions /usr/local/bin/composer
RUN apt-get remove -y gnupg2 git
RUN mv $PHP_INI_DIR/php.ini-production $PHP_INI_DIR/php.ini
# had to add /wait into prod image so the prod image could be run through E2E tests.
COPY --from=sillsdev/web-languageforge:wait-latest /wait /wait

# DEVELOPMENT IMAGE
FROM sillsdev/web-languageforge:base-php AS development-app
# DEVELOPMENT BASE IMAGE
FROM sillsdev/web-languageforge:base-php AS development-base
RUN install-php-extensions xdebug-^3.1
COPY docker/app/docker-php-ext-xdebug.ini /usr/local/etc/php/conf.d
RUN mv $PHP_INI_DIR/php.ini-development $PHP_INI_DIR/php.ini
COPY --from=sillsdev/web-languageforge:wait-latest /wait /wait
COPY docker/app/run-with-wait.sh /run-with-wait.sh

FROM ${ENVIRONMENT}-app AS languageforge-app
# COMMON BASE IMAGE
FROM ${ENVIRONMENT}-base AS languageforge-base
ARG BUILD_VERSION=${BUILD_VERSION:-'9.9.9'}

# copy app into image
Expand All @@ -84,5 +83,14 @@ COPY --from=composer-builder /composer/vendor /var/www/html/vendor
RUN echo "${BUILD_VERSION}" > build-version.txt \
&& sed -i "s/9.9.9/${BUILD_VERSION}/" version.php

# Final image, to be selected with docker build --target=development
FROM languageforge-base AS development

ENTRYPOINT [ "tini", "-g", "--" ]
CMD [ "apache2-foreground" ]

# Final image, to be selected with docker build --target=production
FROM languageforge-base AS production

ENTRYPOINT [ "tini", "-g", "--" ]
CMD [ "apache2-foreground" ]
11 changes: 6 additions & 5 deletions docker/next-app/Dockerfile.next-app
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
# Build the app
FROM node:alpine AS builder
FROM node:22.2.0-alpine3.18 AS builder

WORKDIR /app

COPY tsconfig.json package-lock.json package.json postcss.config.cjs svelte.config.js vite.config.js tailwind.config.cjs /app/
COPY tsconfig.json pnpm-lock.yaml package.json postcss.config.cjs svelte.config.js vite.config.js tailwind.config.cjs /app/
COPY src /app/src
COPY static /app/static

RUN npm install
RUN npm run build
RUN corepack enable
RUN pnpm install
RUN pnpm run build

# Run
FROM node:alpine
FROM node:22.2.0-alpine3.18

COPY --from=builder /app/build /app/
COPY --from=builder /app/node_modules /app/node_modules
Expand Down
7 changes: 4 additions & 3 deletions docker/next-app/dev/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
FROM node:alpine
FROM node:22.2.0-alpine3.18

WORKDIR /app

COPY tsconfig.json package-lock.json package.json postcss.config.cjs svelte.config.js vite.config.js tailwind.config.cjs /app/
COPY tsconfig.json pnpm-lock.yaml package.json postcss.config.cjs svelte.config.js vite.config.js tailwind.config.cjs /app/
COPY src /app/src
COPY static /app/static

RUN npm install
RUN corepack enable
RUN pnpm install

EXPOSE 3000
10 changes: 6 additions & 4 deletions docker/ui-builder/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
FROM node:16.14.0-alpine3.15
FROM node:22.2.0-alpine3.18

RUN mkdir -p /data
WORKDIR /data

COPY package.json ./
COPY node_modules ./node_modules/
COPY package.json pnpm-lock.yaml ./

RUN corepack enable
RUN pnpm install

# Copy in files needed for compilation, located in the repo root
COPY typings ./typings/
Expand All @@ -18,4 +20,4 @@ COPY src/json ./src/json
COPY src/sass ./src/sass
COPY src/Site/views ./src/Site/views

CMD npm run build:dev:watch
CMD pnpm run build:dev:watch
9 changes: 5 additions & 4 deletions docs/DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ While Docker is great way to encapsulate all of the dependencies, build tools an

- PHP 7.4
- Composer
- Node and npm
- Node and pnpm
- .Net SDK

### Project Setup
Expand All @@ -52,7 +52,8 @@ While Docker is great way to encapsulate all of the dependencies, build tools an
2. Install [Make](https://www.gnu.org/software/make/): `sudo apt install make`.
3. Install [Node 22](https://nodejs.org/en/download/). We recommend using [nvm](https://github.com/nvm-sh/nvm#installation-and-update) or [nvm on Windows](https://github.com/coreybutler/nvm-windows).
4. Clone the repo: `git clone https://github.com/sillsdev/web-languageforge`.
5. Run `npm install` (required for git pre-commit hook with Prettier)
5. Run `corepack enable` to download and set up PNPM if it's not alraedy installed
6. Run `pnpm install` (required for git pre-commit hook with Prettier)

### Running the App Locally

Expand Down Expand Up @@ -99,8 +100,8 @@ ngrok will return two URLs, one http and one https, that contain what is being s

### Running Playwright E2E Tests

Before running Playwright tests for the first time use `npx playwright install --with-deps chromium` to install chromium with its dependencies. It will ask for root access.
After Playwright updates, you'll likely need to run `npx playwright install` to update the browsers, but Playwright should provide fairly explicit failure logs if that's the case.
Before running Playwright tests for the first time use `pnpm exec playwright install --with-deps chromium` to install chromium with its dependencies. It will ask for root access.
After Playwright updates, you'll likely need to run `pnpm exec playwright install` to update the browsers, but Playwright should provide fairly explicit failure logs if that's the case.

1. `make e2e-tests`
1. Test results will appear in your terminal
Expand Down
2 changes: 1 addition & 1 deletion next-app/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ From the project root directory, `make next-dev` and access app via http://local

### Alternate local development (limited)

From within the `/next-app` directory, `npm run dev` will also start the next app and the root page can be used for simple testing, i.e., http://localhost:3000/. This approach is limited in that the backend will not be started so all development must be isolated to pages without a required backend call.
From within the `/next-app` directory, `pnpm run dev` will also start the next app and the root page can be used for simple testing, i.e., http://localhost:3000/. This approach is limited in that the backend will not be started so all development must be isolated to pages without a required backend call.
Loading
Loading