LINK-2574: Migrate linkedregistrations-ui from Yarn to pnpm and add runtime env injection#297
Conversation
b2fb5db to
19a81c5
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is failed for https://linkedregistrations-pr297-ui.dev.hel.ninja 😿💢💥💥 |
19a81c5 to
18755ce
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is success for https://linkedregistrations-pr297-ui.dev.hel.ninja 😆🎉🎉🎉 |
164d1df to
cae7db7
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is success for https://linkedregistrations-pr297-ui.dev.hel.ninja 😆🎉🎉🎉 |
cae7db7 to
3084f50
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is success for https://linkedregistrations-pr297-ui.dev.hel.ninja 😆🎉🎉🎉 |
3084f50 to
2247f6f
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is success for https://linkedregistrations-pr297-ui.dev.hel.ninja 😆🎉🎉🎉 |
2247f6f to
0c72e81
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is success for https://linkedregistrations-pr297-ui.dev.hel.ninja 😆🎉🎉🎉 |
0c72e81 to
70e19af
Compare
70e19af to
469f275
Compare
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
e2e tests result is failed for https://linkedregistrations-pr297-ui.dev.hel.ninja 😿💢💥💥 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughMigrates tooling from Yarn to pnpm on Node 24, introduces runtime env bundle generation (public/env-config.js) and a getEnvValue() accessor, and applies the changes across Docker, CI, app code, tests, and coverage settings. ChangesYarn to pnpm Migration and Runtime Environment System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
scripts/update-runtime-env.js (1)
16-46: ⚡ Quick winConsider using a standard .env parser library.
The custom parsing implementation doesn't handle several .env file edge cases such as multi-line values, escaped characters, variable expansion, or mismatched quotes. While this may be sufficient for current needs, using a well-tested library like
dotenvwould provide more robust parsing and future-proof the implementation.📚 Recommended refactor using dotenv
Install dotenv as a dependency, then simplify the parsing:
+const dotenv = require('dotenv'); const fs = require('fs'); const path = require('path'); -const parseEnvContent = (content) => { - const parsed = {}; - - content.split(/\r?\n/).forEach((line) => { - const trimmed = line.trim(); - - if (!trimmed || trimmed.startsWith('#')) { - return; - } - - const separatorIndex = trimmed.indexOf('='); - - if (separatorIndex < 0) { - return; - } - - const key = trimmed.slice(0, separatorIndex).trim(); - let value = trimmed.slice(separatorIndex + 1).trim(); - - if ( - (value.startsWith('"') && value.endsWith('"')) || - (value.startsWith("'") && value.endsWith("'")) - ) { - value = value.slice(1, -1); - } - - parsed[key] = value; - }); - - return parsed; -}; - const loadEnvFromFiles = () => { const env = {}; envFiles.forEach((fileName) => { const filePath = path.resolve(envDir, fileName); if (!fs.existsSync(filePath)) { return; } - const parsed = parseEnvContent(fs.readFileSync(filePath, 'utf8')); + const parsed = dotenv.parse(fs.readFileSync(filePath, 'utf8')); Object.assign(env, parsed); }); return env; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/update-runtime-env.js` around lines 16 - 46, The custom parseEnvContent function is brittle and should be replaced with a standard .env parser; remove or stop using parseEnvContent and instead call dotenv.parse(content) (after adding dotenv to dependencies) to build the parsed object, or use dotenv.config() if reading from a file; update any callers in scripts/update-runtime-env.js to use the object returned by dotenv.parse and keep existing handling of the parsed keys.pnpm-workspace.yaml (1)
4-5: 💤 Low valueClarify the comment or remove the configuration.
The comment states "Don't warn about ignored build scripts," but the array is empty, meaning no build scripts are actually ignored. Either remove this configuration if unused, or update the comment to reflect that this is a placeholder for future use.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpm-workspace.yaml` around lines 4 - 5, The comment above ignoredBuiltDependencies is misleading because the array is empty; either remove the ignoredBuiltDependencies: [] entry entirely if it's unused, or update the comment to indicate it is a placeholder (e.g., "Placeholder for ignored build scripts — currently empty") so the meaning matches the empty array; locate the ignoredBuiltDependencies key in the pnpm-workspace.yaml and apply one of those two changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 12-13: Update the reusable workflow reference to pin the commit
SHA instead of using `@main` (use the provided SHA
4de6bb1b0d7beb5d719dcfaa3a26613dac905e2a) and remove the broad secrets: inherit;
instead pass only the explicitly required secret declared by the called workflow
(SONAR_TOKEN) by mapping it from this repository's secrets (replace the secrets:
inherit block with a secrets block that forwards SONAR_TOKEN via the caller's
secrets).
In `@Dockerfile`:
- Around line 74-80: The files are copied with group "nodejs" which prevents a
random UID with gid 0 from using the intended group write path; update the
COPY/COPY --chown usage and permissions so group ID 0 owns or can write the
runtime assets: replace the COPY lines that use "--chown=default:nodejs" (the
two COPY --from=builder entries for /app/public and /app/scripts) with either no
--chown or with "--chown=default:0", then adjust the RUN step to explicitly set
group ownership and group-writable bits (e.g., use chgrp -R 0 /app/public
/app/scripts && chmod -R g+rwX /app/public /app/scripts) before
removing/creating the env-config.js symlink so a process running with an
arbitrary UID but gid 0 can write /tmp/env-config.js via the group permissions.
In `@README.md`:
- Line 188: The README references a pnpm script "test:debug" that doesn't exist
in package.json; either add a "test:debug" script entry to package.json (under
the "scripts" section) that runs the intended debug test command, or update
README.md to reference an existing script (e.g., "test" or "test:watch") and
ensure the script name in README matches the exact key in package.json
("test:debug"). Make the change so the README and package.json scripts are
consistent.
- Line 67: Update the development command text that currently reads "Run `pnpm i
&& pnpm start`" to use the development workflow command `pnpm dev` (for example:
"Run `pnpm i && pnpm dev`" or simply "pnpm dev"), ensuring the "local
development" section and any occurrences of `pnpm start` meant for dev are
changed to `pnpm dev` to match the stated workflow.
In `@src/instrumentation-client.ts`:
- Around line 32-33: tracePropagationTargets is being built from
getEnvValue('NEXT_PUBLIC_SENTRY_TRACE_PROPAGATION_TARGETS') and currently uses
(... || '').split(',') which yields [''] when the env is empty and causes
wildcard propagation; change the construction of tracePropagationTargets to
split, trim each entry and filter out empty strings (e.g. map trim then
filter(Boolean)) so an empty or unset env produces an empty array rather than
['']; update the expression that sets tracePropagationTargets (referencing
getEnvValue and the NEXT_PUBLIC_SENTRY_TRACE_PROPAGATION_TARGETS env)
accordingly.
In `@src/utils/__tests__/getServerRuntimeConfig.test.ts`:
- Around line 16-25: The tests in the getServerRuntimeConfig suite call
vi.stubEnv(...) but never restore environment, causing leaks between tests; add
cleanup by calling vi.unstubAllEnvs() in an afterEach (or beforeEach) for the
suite that contains the tests in
src/utils/__tests__/getServerRuntimeConfig.test.ts so each test resets env
state—locate the test suite/block where getServerRuntimeConfig tests are defined
and add a teardown (afterEach(() => vi.unstubAllEnvs())) to ensure stubbed env
vars are cleared between tests.
In `@src/utils/getEnvValue.ts`:
- Around line 20-23: The current getEnvValue function repeatedly calls
console.warn when globalThis.window._env_ is undefined, causing console spam;
modify getEnvValue to emit that warning only once by introducing a module-scoped
boolean flag (e.g., envWarningLogged) checked before calling console.warn, set
the flag to true after the first warning, and keep all existing message text and
condition logic around globalThis.window._env_ so subsequent calls skip the warn
path; ensure the flag is defined outside the getEnvValue function so it persists
across invocations.
---
Nitpick comments:
In `@pnpm-workspace.yaml`:
- Around line 4-5: The comment above ignoredBuiltDependencies is misleading
because the array is empty; either remove the ignoredBuiltDependencies: [] entry
entirely if it's unused, or update the comment to indicate it is a placeholder
(e.g., "Placeholder for ignored build scripts — currently empty") so the meaning
matches the empty array; locate the ignoredBuiltDependencies key in the
pnpm-workspace.yaml and apply one of those two changes.
In `@scripts/update-runtime-env.js`:
- Around line 16-46: The custom parseEnvContent function is brittle and should
be replaced with a standard .env parser; remove or stop using parseEnvContent
and instead call dotenv.parse(content) (after adding dotenv to dependencies) to
build the parsed object, or use dotenv.config() if reading from a file; update
any callers in scripts/update-runtime-env.js to use the object returned by
dotenv.parse and keep existing handling of the parsed keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0977669-f6f8-4815-a756-6e72967311ff
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
.dockerignore.env.github/workflows/ci.yml.gitignore.husky/commit-msg.nvmrc.vscode/launch.jsonDockerfileREADME.mdnext.config.jspackage.jsonpnpm-workspace.yamlscripts/docker-entrypoint.shscripts/env.shscripts/update-runtime-env.jssonar-project.propertiessrc/domain/app/layout/pageLayout/PageLayout.tsxsrc/domain/registration/__tests__/utils.test.tssrc/domain/registration/utils.tssrc/domain/signupGroup/participantAmountSelector/__tests__/ParticipantAmountSelector.test.tsxsrc/domain/signupGroup/reservationTimer/__tests__/ReservationTimer.test.tsxsrc/instrumentation-client.tssrc/pages/_document.tsxsrc/pages/api/readiness.tssrc/setupTests.tssrc/types/runtime-env.d.tssrc/utils/__tests__/getEnvValue.test.tssrc/utils/__tests__/getPublicRuntimeConfig.test.tssrc/utils/__tests__/getServerRuntimeConfig.test.tssrc/utils/featureFlags.tssrc/utils/getEnvValue.tssrc/utils/getPublicRuntimeConfig.tssrc/utils/getServerRuntimeConfig.tssrc/utils/mockNextJsConfig.tsvitest.config.ts
- switch local scripts, VS Code launch config, and Husky commit-msg hook to pnpm - update Dockerfile to install and use pnpm via Corepack - replace Yarn lockfile with pnpm lockfile and add pnpm workspace config - update README commands and prerequisites from Yarn to pnpm - add packageManager and pnpm engine constraints in package.json - move dependency constraints from resolutions to pnpm overrides - bump Next.js patch version and add missing Node types dependency Refs: LINK-2574
- update .nvmrc to v24.14.1 - update CI workflow Node version to 24.x - bump Docker base images from nodejs-22 to nodejs-24 - update package.json engine constraint to >=24.14.1 Refs: LINK-2574
- remove Next.js runtimeConfig usage in app config and test setup - update runtime config helpers to read values from process.env - update tests to stub required env vars with vi.stubEnv - map mock config keys to environment variable names Refs: LINK-2574
Playwright install could hang on Node 24. Upgrade Playwright to 1.60.0 and refresh lockfile entries. This version includes the fix for the install issue. Refs: LINK-2573
2f15820 to
42a1ef8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 13: The "start" npm script currently runs "pnpm update-runtime-env"
before "next start" without NODE_ENV=production, which can cause runtime config
to be generated as development; update the "start" script (the package.json
"start" entry) so NODE_ENV=production is set for the whole command that runs
update-runtime-env and next start (e.g., prefix the command with
NODE_ENV=production or use cross-env if cross-platform support is required) to
ensure runtime env generation and Next.js run in production mode.
In `@src/utils/__tests__/getEnvValue.test.ts`:
- Around line 15-21: The test file defines an unused helper function
unsetRuntimeEnv which triggers no-unused-vars; remove the entire unsetRuntimeEnv
function declaration (the Object.defineProperty call that sets _env_ to
undefined) from src/utils/__tests__/getEnvValue.test.ts and any
references/imports for windowWithEnv only if they become unused after removal so
the linter no longer flags an unused variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0bb82c4a-c43c-4497-8aa8-0bd4131a2aed
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
.dockerignore.env.github/workflows/ci.yml.gitignore.husky/commit-msg.nvmrc.vscode/launch.jsonDockerfileREADME.mdnext.config.jspackage.jsonpnpm-workspace.yamlscripts/docker-entrypoint.shscripts/env.shscripts/update-runtime-env.jssonar-project.propertiessrc/domain/app/layout/pageLayout/PageLayout.tsxsrc/domain/registration/__tests__/utils.test.tssrc/domain/registration/utils.tssrc/domain/signupGroup/participantAmountSelector/__tests__/ParticipantAmountSelector.test.tsxsrc/domain/signupGroup/reservationTimer/__tests__/ReservationTimer.test.tsxsrc/instrumentation-client.tssrc/pages/_document.tsxsrc/pages/api/readiness.tssrc/setupTests.tssrc/types/runtime-env.d.tssrc/utils/__tests__/getEnvValue.test.tssrc/utils/__tests__/getPublicRuntimeConfig.test.tssrc/utils/__tests__/getServerRuntimeConfig.test.tssrc/utils/featureFlags.tssrc/utils/getEnvValue.tssrc/utils/getPublicRuntimeConfig.tssrc/utils/getServerRuntimeConfig.tssrc/utils/mockNextJsConfig.tsvitest.config.ts
✅ Files skipped from review due to trivial changes (11)
- .nvmrc
- .gitignore
- vitest.config.ts
- src/setupTests.ts
- .husky/commit-msg
- src/domain/signupGroup/participantAmountSelector/tests/ParticipantAmountSelector.test.tsx
- .vscode/launch.json
- src/pages/api/readiness.ts
- src/types/runtime-env.d.ts
- sonar-project.properties
- README.md
🚧 Files skipped from review as they are similar to previous changes (17)
- scripts/docker-entrypoint.sh
- src/domain/app/layout/pageLayout/PageLayout.tsx
- src/utils/tests/getServerRuntimeConfig.test.ts
- src/pages/_document.tsx
- src/instrumentation-client.ts
- src/utils/tests/getPublicRuntimeConfig.test.ts
- src/domain/registration/utils.ts
- scripts/env.sh
- src/domain/signupGroup/reservationTimer/tests/ReservationTimer.test.tsx
- pnpm-workspace.yaml
- next.config.js
- src/utils/getServerRuntimeConfig.ts
- src/utils/getEnvValue.ts
- src/domain/registration/tests/utils.test.ts
- .dockerignore
- src/utils/getPublicRuntimeConfig.ts
- src/utils/mockNextJsConfig.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/getEnvValue.ts`:
- Around line 20-37: The check for Node's process is unsafe in browsers; replace
the `if (process !== undefined)` guard in getEnvValue with a typeof check (`if
(typeof process !== 'undefined'`) and then safely read `process.env?.[key]` (or
ensure you only access `process.env` after the typeof guard) so that referencing
process.env in the `getEnvValue` function (the `processValue` lookup) cannot
throw a ReferenceError in browser runtimes; keep existing checks around
globalThis.window, warnedAboutMissingEnv, toEnvString, and runtimeEnvValue
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99e5f135-0538-4c2c-bd33-efcc53b234c9
📒 Files selected for processing (25)
.dockerignore.env.gitignoreDockerfileREADME.mdpackage.jsonscripts/docker-entrypoint.shscripts/env.shscripts/update-runtime-env.jssonar-project.propertiessrc/domain/app/layout/pageLayout/PageLayout.tsxsrc/domain/registration/__tests__/utils.test.tssrc/domain/registration/utils.tssrc/instrumentation-client.tssrc/pages/_document.tsxsrc/pages/api/readiness.tssrc/setupTests.tssrc/types/runtime-env.d.tssrc/utils/__tests__/getEnvValue.test.tssrc/utils/__tests__/getPublicRuntimeConfig.test.tssrc/utils/__tests__/getServerRuntimeConfig.test.tssrc/utils/featureFlags.tssrc/utils/getEnvValue.tssrc/utils/getPublicRuntimeConfig.tsvitest.config.ts
✅ Files skipped from review due to trivial changes (5)
- scripts/docker-entrypoint.sh
- .gitignore
- sonar-project.properties
- src/domain/registration/tests/utils.test.ts
- README.md
🚧 Files skipped from review as they are similar to previous changes (15)
- vitest.config.ts
- scripts/env.sh
- src/types/runtime-env.d.ts
- src/utils/tests/getServerRuntimeConfig.test.ts
- src/setupTests.ts
- src/domain/registration/utils.ts
- src/utils/featureFlags.ts
- .dockerignore
- src/utils/tests/getPublicRuntimeConfig.test.ts
- src/domain/app/layout/pageLayout/PageLayout.tsx
- scripts/update-runtime-env.js
- src/utils/getPublicRuntimeConfig.ts
- package.json
- src/instrumentation-client.ts
- Dockerfile
e2e tests result is failed for https://linkedregistrations-pr297-ui.dev.hel.ninja 😿💢💥💥 |
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |
- add runtime env generator script and shell wrappers for local/test and container startup - generate public env config files and load env-config.js before app scripts - add getEnvValue helper with runtime window._env_ fallback to process.env - migrate Matomo, Sentry, feature flags, registration utils, and runtime config reads to getEnvValue - update Dockerfile to include scripts, run env generation on startup, and use pnpm install flow - run runtime env generation in dev/start/test scripts and document the workflow - update ignore and coverage settings for generated/runtime scripts Refs: LINK-2574
42a1ef8 to
57b80bd
Compare
| if (separatorIndex < 0) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: The parseEnvContent function does not unescape special characters like \n or \" in double-quoted .env values, leading to incorrect parsing of these variables.
Severity: LOW
Suggested Fix
Modify the parseEnvContent function to correctly handle and unescape standard escape sequences (e.g., \n, \t, \") for values enclosed in double quotes. This will align its behavior with standard .env file parsers and prevent silent data corruption if such variables are used in the future.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: scripts/update-runtime-env.js#L28-L31
Potential issue: The `parseEnvContent` function in `scripts/update-runtime-env.js` does
not correctly handle escaped characters within double-quoted values in `.env` files. For
example, a value like `"Hello\nWorld"` will be parsed as the literal string
`Hello\nWorld` instead of a string containing a newline character. While no current
environment variables use such escape sequences, this is a latent defect. If a developer
adds a variable with escaped quotes or newlines in the future, it would lead to silent
misconfiguration and unexpected behavior, as the application would receive a string with
literal backslashes instead of the intended characters.
|
e2e tests result is failed for https://linkedregistrations-pr297-ui.dev.hel.ninja 😿💢💥💥 |
|
LINKEDREGISTRATIONS-UI branch is deployed to platta: https://linkedregistrations-pr297-ui.dev.hel.ninja 🚀🚀🚀 |



Summary
This PR migrates the project tooling from Yarn to pnpm as the primary package manager.
It also introduces runtime environment variable injection so environment-specific values can be provided at runtime without rebuilding the app.
What Changed
Related Azure PR:
https://dev.azure.com/City-of-Helsinki/linkedevents/_git/linkedevents-pipelines/pullrequest/16079
Refs: LINK-2574
Summary by CodeRabbit
Chores
New Features
Documentation
Tests