Skip to content

Conversation

@ryanbas21
Copy link
Collaborator

@ryanbas21 ryanbas21 commented Nov 3, 2025

JIRA Ticket

N/A

Description

Updates Nx, adds catalog support, updated deps to use catalogs where appropriate. Fixed failing test from vite upgrade.

Summary by CodeRabbit

  • Chores

    • Upgraded Nx and related tooling from v21 to v22 and consolidated package metadata and formatting.
    • Centralized several build/test versions to workspace catalog aliases.
  • Tests

    • Aligned test typings with current test tooling.
    • Expanded coverage collection to include JavaScript and TypeScript sources.

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025

⚠️ No Changeset found

Latest commit: 8e45f72

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Walkthrough

Consolidates workspace catalog entries and replaces several explicit devDependency/version pins with pnpm catalog specifiers; bumps Nx-related packages to 22.x; adds a Vitest coverage include pattern; and replaces a Vitest test type import from SpyInstance to MockInstance.

Changes

Cohort / File(s) Summary
Workspace catalog & root manifests
pnpm-workspace.yaml, package.json
Adds vite and typescript to workspace catalog; expands vitest catalog with vitest-canvas-mock, @vitest/coverage-v8, @vitest/ui; inlines/simplifies fields in root package.json (bugs, config.commitizen, engines, nx.includedScripts).
Root Nx and tooling upgrades
package.json
Bumps Nx-related devDependencies from 21.x to 22.x (e.g., nx, @nx/devkit, @nx/eslint-plugin, plugins); replaces explicit TypeScript/Vite/Vitest-related version pins with catalog: placeholders.
Package-level catalog migrations
packages/journey-client/package.json
Replaces pinned versions for vite, vitest, @vitest/coverage-v8, and vitest-canvas-mock with catalog: specifiers (applied in dependencies and devDependencies).
Vitest config change
tools/user-scripts/vitest.config.ts
Adds coverage.include: ['src/**/*.{js,ts}'] to Vitest coverage configuration.
Test typing adjustment
packages/journey-client/src/lib/device/device-profile.test.ts
Changes import/type usage from SpyInstance to MockInstance for a Vitest mock/spy variable.

Sequence Diagram(s)

(Skipped — changes are configuration and version/catalog updates and a typing tweak; no control-flow or runtime feature additions to diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas needing attention:
    • Nx 21.x → 22.x upgrade surface for breaking changes in workspace tooling and generators.
    • pnpm catalog placeholders: ensure CI/publish tooling resolves catalog: entries as expected.
    • Vitest coverage addition: confirm coverage collection matches intended files.
    • Test typing swap: ensure MockInstance type exists and aligns with the targeted Vitest version.

Possibly related PRs

  • chore: update-pnpm #422 — modifies workspace package configuration and pnpm settings; related to workspace/package metadata changes.

Suggested reviewers

  • cerebrl

Poem

🐇 I hopped through manifests, tidy and quick,

swapped pins for catalog, neat and slick.
Nx took a leap, Vitest cast its net,
mocks donned new names — no panic yet.
A carrot of builds, sprightly and lit! 🍒

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'chore: update-nx' is vague and generic, using a non-descriptive term 'update-nx' that doesn't clearly convey the scope of changes including catalog support, dependency consolidation, and test fixes. Consider a more specific title like 'chore: upgrade Nx to 22.x and consolidate dependencies with catalog support' to better reflect the main changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description covers the main changes (Nx updates, catalog support, and test fixes) and follows the required template structure with JIRA ticket and description sections.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-nx

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0fdc1c and 8e45f72.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • package.json (6 hunks)
  • packages/journey-client/package.json (1 hunks)
  • packages/journey-client/src/lib/device/device-profile.test.ts (2 hunks)
  • pnpm-workspace.yaml (2 hunks)
  • tools/user-scripts/vitest.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/journey-client/package.json
  • package.json
  • packages/journey-client/src/lib/device/device-profile.test.ts
  • pnpm-workspace.yaml
⏰ 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 (1)
tools/user-scripts/vitest.config.ts (1)

15-15: Coverage include pattern is correct for this package.

The coverage include pattern ['src/**/*.{js,ts}'] appropriately targets all source files in this TypeScript-only package. Verification confirms there are no JSX/TSX source files in the src/ directory—only TypeScript files (.ts)—making the pattern perfectly aligned with the actual codebase.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Contributor

nx-cloud bot commented Nov 3, 2025

View your CI Pipeline Execution ↗ for commit 8e45f72

Command Status Duration Result
nx run-many -t build ✅ Succeeded <1s View ↗
nx affected -t build typecheck lint test e2e-ci ✅ Succeeded 1m 3s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-05 22:22:38 UTC

nx-cloud[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa3d434 and e0fdc1c.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • package.json (6 hunks)
  • packages/journey-client/package.json (1 hunks)
  • packages/journey-client/src/lib/device/device-profile.test.ts (2 hunks)
  • pnpm-workspace.yaml (2 hunks)
⏰ 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 (6)
pnpm-workspace.yaml (1)

27-31: LGTM! Catalog structure for Vitest packages looks good.

The vitest catalog consolidates testing dependencies and will ensure consistent versions across the workspace.

packages/journey-client/package.json (1)

33-38: LGTM! Catalog specifiers are correctly applied.

The migration to catalog-based dependency resolution ensures consistent versions across the workspace and aligns with the catalog definitions in pnpm-workspace.yaml.

packages/journey-client/src/lib/device/device-profile.test.ts (2)

89-89: Type annotation correctly updated.

The warnSpy type annotation is properly updated to match the import change.


10-10: SpyInstance → MockInstance migration is complete.

Verification across all 69 test files in the codebase confirms zero remaining SpyInstance usages. The file mentioned in the review correctly imports MockInstance from vitest. The migration is complete.

package.json (2)

81-82: LGTM! Catalog-based dependency resolution applied correctly.

The migration to catalog specifiers for Vitest, Vite, and TypeScript dependencies ensures consistent versions across the workspace.

Also applies to: 99-99, 111-111, 114-116


58-67: Nx 22 compatibility verified — no breaking changes apply to this workspace.

The workspace is already compatible with Nx 22.0.2. Verification confirms:

  • All plugins in nx.json use v2 syntax (no CreateNodes V1 migration needed)
  • No deprecated configurations detected (useLegacyTypescriptPlugin, NX_DISABLE_DB, simpleName, deleted webpack options)
  • Release management uses Changesets (third-party tool), not Nx's built-in system affected by the legacy release removal
  • tsconfig.base.json has no path mappings (nx format sorting change doesn't apply)
  • Using pnpm (package manager --legacy-peer-deps rule only affects npm)

No additional configuration or migration steps required.

Comment on lines +14 to +15
vite: ^7.1.12
typescript: ^5.9.3
Copy link

@coderabbitai coderabbitai bot Nov 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the Vite version is correct.

The version ^7.1.12 appears unusually high. Please confirm this is the intended version.


🌐 Web query:

What is the latest stable version of Vite?

💡 Result:

The latest stable Vite release is v7.1.4. [1][2]

Sources:

  • npm package page for vite (shows v7.1.4). [1]
  • Vite releases documentation (points to npm for latest stable). [2]

[1] https://www.npmjs.com/package/vite
[2] https://vite.dev/releases


Correct the Vite version constraint in pnpm-workspace.yaml.

The latest stable Vite release is v7.1.4, but the file specifies ^7.1.12, which references a version that does not exist in stable releases. This constraint will fail to resolve or pull a pre-release version. Update to ^7.1.4 or an appropriate stable version.

🤖 Prompt for AI Agents
In pnpm-workspace.yaml around lines 14 to 15, the Vite version constraint is
incorrect (currently ^7.1.12) and references a non-existent/stable-unstable
release; update the constraint to a valid stable release such as ^7.1.4 (or
another chosen stable version) so dependency resolution succeeds, ensuring the
line for vite is replaced with the corrected version string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this version is correct, vite is on 7.1.12

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

🌐 Web query:

Vite 7.1.12 version release

💡 Result:

Vite v7.1.12 was released on October 23, 2025 (tag v7.1.12). [1][2]

Sources:

  • GitHub releases (vitejs/vite) — v7.1.12 (Oct 23, 2025). [1]
  • Package listing / metadata (vite v7.1.12). [2]

@ryanbas21, you're absolutely right! I apologize for the incorrect flag. Vite v7.1.12 was indeed released on October 23, 2025, and is a valid version. The version constraint ^7.1.12 in the catalog is correct.

Thank you for the correction!


✏️ Learnings added
Learnt from: ryanbas21
Repo: ForgeRock/ping-javascript-sdk PR: 472
File: pnpm-workspace.yaml:14-15
Timestamp: 2025-11-03T19:07:15.245Z
Learning: Vite version 7.1.12 is a valid release that was published on October 23, 2025.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

nx-cloud[bot]

This comment was marked as outdated.

Copy link
Contributor

@nx-cloud nx-cloud bot left a comment

Choose a reason for hiding this comment

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

Nx Cloud has identified a possible root cause for your failed CI:

The failure was classified as 'environment_state' rather than 'code_change' for the following reasons:

No Changes to Failing Test File:
The PR diff shows no modifications to the failing test file (e2e/davinci-suites/src/phone-number-field.test.ts). The tests that are failing were not touched by this PR.

No Application Logic Changes:
The PR contains only dependency version upgrades (Nx v21 to v22, Vitest v3 to v4, Vite v6 to v7) and formatting changes. There are no changes to application code, UI components, or business logic that would affect the device registration flow being tested.

External Service Dependency Failure:
Both failing tests timeout while waiting for a 'Registration Complete' heading to appear after submitting user registration information. This indicates the backend authentication/registration service is not responding or completing the registration flow. The tests are making real API calls to external services that manage user registration and authentication.

Cleanup Hook Also Fails:
The afterEach cleanup hook also times out trying to delete the test user via the 'USER_DELETE' button. This further confirms the external service is not responding properly, as both creating and deleting users fail.

Timing-Based Failures:
The errors are all timeout-based ("Test timeout of 30000ms exceeded", "Timeout: 5000ms"), which is characteristic of external service unavailability rather than code defects. If the code changes introduced a bug, we would expect different error types (syntax errors, type errors, assertion failures on wrong values).

No Logical Connection to Changes:
Upgrading Nx tooling and test framework versions does not affect whether external authentication APIs respond to HTTP requests. The only code change in a test file was changing SpyInstance to MockInstance in a completely different test file (device-profile.test.ts), which is unrelated to these e2e tests.

The failures indicate an environmental issue where the external authentication/user management service required by these e2e tests is not available or not responding during test execution.

A code change would likely not resolve this issue, so no action was taken.

Nx Cloud View in Nx Cloud ↗


🎓 To learn more about Self Healing CI, please visit nx.dev

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 5, 2025

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/davinci-client@472

@forgerock/oidc-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/oidc-client@472

@forgerock/protect

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/protect@472

@forgerock/sdk-types

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-types@472

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-utilities@472

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/iframe-manager@472

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-logger@472

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-oidc@472

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-request-middleware@472

@forgerock/storage

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/storage@472

commit: 8e45f72

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Deployed 83564da to https://ForgeRock.github.io/ping-javascript-sdk/pr-472/83564da00c13e295a587820f05ae796e2a0a3014 branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

🔻 @forgerock/journey-client - 0.0 KB (-82.0 KB, -100.0%)

📊 Minor Changes

📉 @forgerock/journey-client - 82.0 KB (-0.0 KB)

➖ No Changes

@forgerock/device-client - 9.2 KB
@forgerock/oidc-client - 23.0 KB
@forgerock/protect - 150.1 KB
@forgerock/sdk-utilities - 7.5 KB
@forgerock/sdk-types - 8.0 KB
@forgerock/storage - 1.4 KB
@forgerock/sdk-logger - 1.6 KB
@forgerock/iframe-manager - 2.4 KB
@forgerock/sdk-request-middleware - 4.5 KB
@forgerock/sdk-oidc - 2.5 KB
@forgerock/davinci-client - 35.4 KB


13 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated
  • Current Size: Total gzipped size of all files in the package's dist directory
  • Baseline: Comparison against the latest build from the main branch
  • Files included: All build outputs except source maps and TypeScript build cache
  • Exclusions: .map, .tsbuildinfo, and .d.ts.map files

🔄 Updated automatically on each push to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants