Update deps #522
Update deps #522
Conversation
|
📝 WalkthroughWalkthroughRemoves many TypeScript project references across e2e and package tsconfig files, upgrades Nx/Vite/Vitest/TypeScript tooling and package specifiers to catalog aliases, adds an Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ryanbas21
force-pushed
the
update-deps
branch
from
4d82054 to
5284f83
Compare
|
View your CI Pipeline Execution ↗ for commit 2ae02ac
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@e2e/am-mock-api/package.json`:
- Around line 11-15: Update the type packages to be compatible with Express 5:
replace the dependency version string for "@types/express" from "^4.17.17" to
"^5.0.0" and add or bump related type packages
"@types/express-serve-static-core", "@types/body-parser", and
"@types/serve-static" to their v5-compatible versions (e.g., "^5.0.0") in
package.json; after updating, run install and compile to surface stricter
handler signature errors and then update any Express handler types/usages in
your codebase to match the Express 5 handler signatures referenced by the new
"@types/*" packages.
In `@package.json`:
- Line 121: The package.json dependency for vitest-canvas-mock currently
(^0.3.3) conflicts with the lockfile (^1.1.3), blocking CI; fix by either
updating the package.json entry for "vitest-canvas-mock" to the lockfile version
(^1.1.3) so they match, or regenerate the lockfile by running your package
manager install (npm/yarn/pnpm) and committing the updated lockfile so
package.json and the lockfile are consistent.
In `@packages/journey-client/package.json`:
- Around line 32-33: Remove build/test tools from production dependencies:
delete the "vite" and the "vitest-canvas-mock" entries from the top-level
"dependencies" block in package.json so they only live in "devDependencies"
(ensuring "vitest-canvas-mock" is kept at the ^1.1.3 version under
devDependencies). After removing those keys, run your package manager to update
the lockfile so the dependency versions match the lockfile (resolving the ^0.3.3
vs ^1.1.3 mismatch).
🧹 Nitpick comments (1)
package.json (1)
130-131: Consider removing the emptydependenciesobject.The empty
dependencies: {}object serves no purpose in the root package.json and can be removed for cleaner configuration.Proposed fix
"nx": { "includedScripts": [] - }, - "dependencies": {} + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
e2e/am-mock-api/package.jsone2e/davinci-app/tsconfig.jsone2e/device-client-app/tsconfig.jsone2e/journey-app/tsconfig.jsone2e/oidc-app/tsconfig.jsone2e/protect-app/tsconfig.jsonnx.jsonpackage.jsonpackages/journey-client/package.jsonpackages/journey-client/src/lib/device/device-profile.test.tspackages/journey-client/tsconfig.jsonpackages/oidc-client/tsconfig.jsonpackages/sdk-effects/oidc/tsconfig.jsonpackages/sdk-effects/storage/tsconfig.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (9)
- packages/journey-client/tsconfig.json
- e2e/oidc-app/tsconfig.json
- packages/sdk-effects/oidc/tsconfig.json
- e2e/protect-app/tsconfig.json
- packages/oidc-client/tsconfig.json
- e2e/device-client-app/tsconfig.json
- e2e/davinci-app/tsconfig.json
- e2e/journey-app/tsconfig.json
- packages/sdk-effects/storage/tsconfig.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-28T14:44:26.883Z
Learnt from: ryanbas21
Repo: ForgeRock/ping-javascript-sdk PR: 427
File: .github/workflows/ci-fork.yml:50-56
Timestamp: 2025-10-28T14:44:26.883Z
Learning: Nx CLI accepts multiple targets with the short `-t` flag as space-separated tokens (e.g., `nx affected -t lint test build`) and also supports comma-separated values with the long form `--targets=` (e.g., `nx affected --targets=lint,test,build`). Both syntaxes are valid.
Applied to files:
nx.json
🪛 GitHub Actions: ForgeRock Pull Request CI
package.json
[error] 1-1: Dependency specifiers do not match the lockfile: vitest-canvas-mock (^0.3.3) vs lockfile (^1.1.3).
packages/journey-client/package.json
[error] 1-1: Dependency specifiers do not match the lockfile: vitest-canvas-mock (^0.3.3) vs lockfile (^1.1.3).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Mend Code Security Check
🔇 Additional comments (4)
nx.json (1)
148-155: LGTM!The migration of
testTargetNamefrom@nx/vite/pluginto the dedicated@nx/vitestplugin is the correct approach for Nx 22.x with Vitest 4.x. The include patterns are consistent with other plugin configurations.packages/journey-client/src/lib/device/device-profile.test.ts (1)
10-10: No changes needed. In Vitest 4.x,MockandMockInstanceare type aliases and can be used interchangeably. Thevi.spyOn()function returnsMock<T>, making the use ofMockas the imported type fully compatible and correct. The code requires no modifications.Likely an incorrect or invalid review comment.
pnpm-workspace.yaml (1)
22-25: Vite^7.3.1is a valid version.Vite 7.3.1 has been published to the npm registry and is available for production use. This is a normal version progression within the 7.x release line.
The vitest version
4.0.9alignment claim cannot be independently verified due to repository access limitations.packages/journey-client/package.json (1)
35-40: LGTM!The
devDependenciessection correctly places all build and test tooling. The version bumps for@vitest/coverage-v8andvitest-canvas-mockalign with the Vitest 4.x upgrade. Once the duplicate entries independenciesare removed, this will resolve the pipeline failure.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "@types/express": "^4.17.17", | ||
| "body-parser": "^2.2.0", | ||
| "body-parser": "^2.2.2", | ||
| "cookie-parser": "^1.4.7", | ||
| "cors": "^2.8.5", | ||
| "express": "^4.21.2", | ||
| "express": "^5.2.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Express 5.0 TypeScript types @types/express compatibility
💡 Result:
Short answer: Use the @types/express v5.x types (from DefinitelyTyped). They are published and intended to match Express 5; install express@5 (or express@next) and @types/express@^5 (and the related @types packages such as @types/express-serve-static-core, @types/body-parser) to get compatible TypeScript definitions. [1][2][4]
Notes / gotchas:
@types/expressv5 is maintained on DefinitelyTyped and has multiple 5.x releases — pick a 5.x tag that matches your setup (npm shows current 5.x releases). [1][2][7]- Some handler signatures changed/are stricter (you may see TS errors for middleware/error-handler overloads); follow the Express v4→v5 migration discussion and update your handler types accordingly. [6][3]
- Ensure all related
@typespackages (express-serve-static-core, body-parser, serve-static, qs) are compatible versions. The@types/expresspackage depends on those. [1][5]
Example install:
npm install express@5 @types/express --save
(or npm install express@next && npm i -D @types/express)
Sources:
[1] @types/express (npm package info, 5.x).
[2] @types/express (npm v5.0.3 page).
[3] TypeScript + Express setup / guidance (tutorial).
[4] @types/express npm listing (overview).
[5] @types/express release details (versions).
[6] Express v4 -> v5 Migration discussion (expressjs/express issue).
🏁 Script executed:
# Check npm registry for `@types/express` version availability
npm search `@types/express` --json 2>/dev/null | jq '.[] | {name, version: .version}' | head -20Repository: ForgeRock/ping-javascript-sdk
Length of output: 202
Update @types/express to v5.x for Express 5 compatibility.
@types/express at ^4.17.17 is incompatible with Express 5.x. Express 5 has significant API changes, and the v4 types will cause type errors and mask breaking changes.
Update to @types/express@^5.0.0. Additionally, ensure related @types packages (@types/express-serve-static-core, @types/body-parser, @types/serve-static) are also compatible v5.x versions. Note that some Express 5 handler signatures are stricter and may require code updates.
Proposed fix
- "@types/express": "^4.17.17",
+ "@types/express": "^5.0.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@types/express": "^4.17.17", | |
| "body-parser": "^2.2.0", | |
| "body-parser": "^2.2.2", | |
| "cookie-parser": "^1.4.7", | |
| "cors": "^2.8.5", | |
| "express": "^4.21.2", | |
| "express": "^5.2.1", | |
| "@types/express": "^5.0.0", | |
| "body-parser": "^2.2.2", | |
| "cookie-parser": "^1.4.7", | |
| "cors": "^2.8.5", | |
| "express": "^5.2.1", |
🤖 Prompt for AI Agents
In `@e2e/am-mock-api/package.json` around lines 11 - 15, Update the type packages
to be compatible with Express 5: replace the dependency version string for
"@types/express" from "^4.17.17" to "^5.0.0" and add or bump related type
packages "@types/express-serve-static-core", "@types/body-parser", and
"@types/serve-static" to their v5-compatible versions (e.g., "^5.0.0") in
package.json; after updating, run install and compile to surface stricter
handler signature errors and then update any Express handler types/usages in
your codebase to match the Express 5 handler signatures referenced by the new
"@types/*" packages.
| "vite": "6.4.1", | ||
| "vite": "catalog:vite", | ||
| "vitest": "catalog:vitest", | ||
| "vitest-canvas-mock": "^0.3.3" |
There was a problem hiding this comment.
Fix vitest-canvas-mock version mismatch blocking CI.
The pipeline failure indicates: "Dependency specifiers do not match the lockfile: vitest-canvas-mock (^0.3.3) vs lockfile (^1.1.3)."
Update the version to match the lockfile or regenerate the lockfile to resolve the CI failure.
Proposed fix (if lockfile version is correct)
- "vitest-canvas-mock": "^0.3.3"
+ "vitest-canvas-mock": "^1.1.3"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "vitest-canvas-mock": "^0.3.3" | |
| "vitest-canvas-mock": "^1.1.3" |
🤖 Prompt for AI Agents
In `@package.json` at line 121, The package.json dependency for vitest-canvas-mock
currently (^0.3.3) conflicts with the lockfile (^1.1.3), blocking CI; fix by
either updating the package.json entry for "vitest-canvas-mock" to the lockfile
version (^1.1.3) so they match, or regenerate the lockfile by running your
package manager install (npm/yarn/pnpm) and committing the updated lockfile so
package.json and the lockfile are consistent.
| "vite": "catalog:vite", | ||
| "vitest-canvas-mock": "^0.3.3" |
There was a problem hiding this comment.
Remove dev/test tools from production dependencies – this is causing the pipeline failure.
vite and vitest-canvas-mock are build/test utilities that belong exclusively in devDependencies. Currently:
vitest-canvas-mockappears in bothdependencies(^0.3.3) anddevDependencies(^1.1.3), causing the lockfile mismatch error.viteis duplicated in both sections.
The pipeline failure vitest-canvas-mock (^0.3.3) vs lockfile (^1.1.3) is due to line 33 not being updated/removed.
🔧 Proposed fix
"dependencies": {
"@forgerock/sdk-logger": "workspace:*",
"@forgerock/sdk-request-middleware": "workspace:*",
"@forgerock/sdk-types": "workspace:*",
"@forgerock/sdk-utilities": "workspace:*",
"@forgerock/storage": "workspace:*",
"@reduxjs/toolkit": "catalog:",
- "tslib": "^2.3.0",
- "vite": "catalog:vite",
- "vitest-canvas-mock": "^0.3.3"
+ "tslib": "^2.3.0"
},🤖 Prompt for AI Agents
In `@packages/journey-client/package.json` around lines 32 - 33, Remove build/test
tools from production dependencies: delete the "vite" and the
"vitest-canvas-mock" entries from the top-level "dependencies" block in
package.json so they only live in "devDependencies" (ensuring
"vitest-canvas-mock" is kept at the ^1.1.3 version under devDependencies). After
removing those keys, run your package manager to update the lockfile so the
dependency versions match the lockfile (resolving the ^0.3.3 vs ^1.1.3
mismatch).
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the Express 5 compatibility issue by updating wildcard route patterns from the deprecated /* syntax to the required /:path* named parameter syntax. This resolves the path-to-regexp parsing error that prevented the mock API server from starting, which was causing all e2e tests to fail with webServer timeout errors.
We verified this fix by re-running @forgerock/journey-suites:e2e-ci--src/no-session.test.ts.
Suggested Fix changes
diff --git a/e2e/am-mock-api/src/app/routes.resource.js b/e2e/am-mock-api/src/app/routes.resource.js
index d10dc278..f1aa042e 100644
--- a/e2e/am-mock-api/src/app/routes.resource.js
+++ b/e2e/am-mock-api/src/app/routes.resource.js
@@ -63,7 +63,7 @@ async function authorization(req, res, next) {
export default function (app) {
// Passthrough route that enforces authentication
- app.all('/resource/*', async (req, res, next) => {
+ app.all('/resource/*path', async (req, res, next) => {
if (env.NODE_ENV === 'LIVE' && req.hostname === FORGEOPS) {
// Only enforce authentication if IG is not used
// In other words, the call comes directly from app
@@ -156,7 +156,7 @@ export default function (app) {
}
});
- app.get('/resource/rest/*', wait, authorization, async (req, res) => {
+ app.get('/resource/rest/*path', wait, authorization, async (req, res) => {
if (env.NODE_ENV === 'live') {
if (req.access.actions && req.access.actions.GET) {
res.json({ message: 'Successfully retrieved resource!' });
diff --git a/packages/device-client/README.md b/packages/device-client/README.md
index 3630ad21..75e29071 100644
--- a/packages/device-client/README.md
+++ b/packages/device-client/README.md
@@ -8,7 +8,6 @@ The `deviceClient` API provides a structured interface for managing various type
2. [Installation](#installation)
3. [Configuration](#configuration)
4. [API Methods](#api-methods)
-
- [OATH Management](#oath-management)
- [PUSH Management](#push-management)
- [WebAuthn Management](#webauthn-management)
diff --git a/packages/sdk-effects/iframe-manager/README.md b/packages/sdk-effects/iframe-manager/README.md
index 31e28168..12dd217c 100644
--- a/packages/sdk-effects/iframe-manager/README.md
+++ b/packages/sdk-effects/iframe-manager/README.md
@@ -40,16 +40,13 @@ This is the main factory function that initializes the effect.
This method creates a hidden iframe, initiates navigation, and waits for a redirect back to the application's origin containing specific query parameters.
- **`options`**: `GetParamsFromIFrameOptions` - An object containing configuration for the iframe request.
-
- **`url: string`**: The initial URL to load within the hidden iframe. This URL is expected to eventually redirect back to the application's origin.
- **`timeout: number`**: The maximum time in milliseconds to wait for the entire operation to complete successfully (i.e., for a redirect containing success or error parameters). If the timeout is reached before completion, the promise rejects.
-
* **`successParams: string[]`**: An array of query parameter _keys_. If the final redirect URL (on the same origin) contains **at least one** of these keys in its query string, the promise will **resolve**.
* **`errorParams: string[]`**: An array of query parameter _keys_. If the final redirect URL (on the same origin) contains **any** of these keys in its query string, the promise will **reject**. Error parameters are checked _before_ success parameters.
- _Note:_ Both `successParams` and `errorParams` must be provided and contain at least one key.
- **Returns**: `Promise<ResolvedParams>`
-
- **On Success**: Resolves with `ResolvedParams`, an object containing _all_ query parameters parsed from the final redirect URL's query string. This occurs when the iframe redirects back to the same origin and its URL contains at least one key listed in `successParams` (and no keys listed in `errorParams`).
- **On Failure**: Rejects with:
- `ResolvedParams`: An object containing _all_ parsed query parameters if the final redirect URL contains any key listed in `errorParams`.
Or Apply changes locally with:
npx nx-cloud apply-locally U2cj-gxXu
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
e2e/am-mock-api/src/app/routes.resource.js (2)
127-127:res.send(status, body)signature removed in Express 5.The two-argument
res.send(status, body)form was deprecated in Express 4 and removed in Express 5. This will not set the 401 status correctly—401may be interpreted as the body instead.🐛 Proposed fix
- res.send(401, null); + res.status(401).send(null);
151-151: Sameres.send(status, body)issue here.Apply the same fix as above.
🐛 Proposed fix
- res.send(401, null); + res.status(401).send(null);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/am-mock-api/src/app/routes.resource.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Mend Code Security Check
- GitHub Check: pr
🔇 Additional comments (2)
e2e/am-mock-api/src/app/routes.resource.js (2)
66-66: LGTM!Correct update to Express 5 named splat syntax (
{*splat}). The route will match all paths under/resource/as before.
159-159: LGTM!Correct update to Express 5 named splat syntax.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 565d932 to https://ForgeRock.github.io/ping-javascript-sdk/pr-522/565d9324a08b5b99c3073339b56c954131ef6669 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/journey-client - 0.0 KB (-82.4 KB, -100.0%) 📊 Minor Changes📈 @forgerock/journey-client - 82.4 KB (+0.0 KB) ➖ No Changes➖ @forgerock/device-client - 9.2 KB 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
JIRA Ticket
N/A
Description
Has some dependency updates and nx updates per the mend issues.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.