Skip to content

feat(sdk): shared store for multiple SDK clients + @forgerock/sdk-store - #729

Draft
ryanbas21 wants to merge 5 commits into
mainfrom
centralize-redux-stores
Draft

feat(sdk): shared store for multiple SDK clients + @forgerock/sdk-store#729
ryanbas21 wants to merge 5 commits into
mainfrom
centralize-redux-stores

Conversation

@ryanbas21

@ryanbas21 ryanbas21 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Allows davinci(), journey(), and oidc() to share a single Redux store, so OpenID Connect discovery (.well-known/openid-configuration) is fetched once instead of once per client.

What's in this PR

New package: @forgerock/sdk-store

A new scope:sdk-effects package that owns:

  • The single canonical wellknownApi instance and discovery cache (previously duplicated in each client package)
  • The shared store contract: SdkStore, SdkStoreHandle, createSdkStore(), injectClient()
  • OpenID Connect discovery helpers: initWellknownQuery, isValidWellknownResponse

Store sharing

All three client factories accept an optional store option:

import { createSdkStore } from '@forgerock/sdk-store';

const store = createSdkStore();

const davinci = davinci({ config, store });
const oidc    = oidc({ config, store });

Omitting store is unchanged behaviour — each client creates its own store, exactly as before.

davinci() and journey() expose their store as client.store for apps that want to pass it to oidc() without creating one explicitly.

Middleware and logging are scoped per client

Each client's requestMiddleware and logger are registered against that client alone. Middleware passed to davinci() or journey() is never applied to OIDC requests (AUTHORIZE, PAR, TOKEN_EXCHANGE, REVOKE, USER_INFO, END_SESSION), and vice versa. Both options are honoured on a shared store.

One OIDC client per store

oidc() mounts at a fixed key. Initialising a second OIDC client on the same store with a different clientId returns an argument_error instead of silently overwriting the first client's token state. Re-initialising with the same clientId is idempotent. Use a separate store per clientId.

Validation before attachment

oidc() validates its arguments before attaching to a store, so a rejected call no longer leaves a caller-provided store modified. Passing a non-SDK-store value to store returns an argument_error instead of throwing.

Bug fixes

  • Well-known selectors are now memoized per URL — createWellknownSelector previously rebuilt its selector on every call, so the cache never took effect.
  • enforce-module-boundaries lint rule promoted from warn to error across the repo. All packages pass.

Breaking changes

  • @forgerock/sdk-oidc: initWellknownQuery and isValidWellknownResponse move to @forgerock/sdk-store. Update imports if you were using them directly.

Testing

  • Unit tests for the new @forgerock/sdk-store package (store.effects.test.ts, store.utils.test.ts, wellknown.api.test.ts)
  • Unit tests for store shape validation and lifecycle in each client (store-shape.test.ts, store-lifecycle.test.ts, shared-store.test.ts)
  • E2E test covering the shared-store path end-to-end (e2e/davinci-suites/src/shared-store.test.ts)

@changeset-bot

changeset-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 28d2507

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@forgerock/davinci-client Minor
@forgerock/journey-client Minor
@forgerock/oidc-client Minor
@forgerock/sdk-store Minor
@forgerock/sdk-oidc Minor
@forgerock/device-client Minor
@forgerock/protect Minor
@forgerock/sdk-types Minor
@forgerock/sdk-utilities Minor
@forgerock/iframe-manager Minor
@forgerock/sdk-logger Minor
@forgerock/sdk-request-middleware Minor
@forgerock/storage Minor

Not sure what this means? Click here to learn what changesets are.

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

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1fdc33c1-7633-4414-98bf-e00d387cc583

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch centralize-redux-stores

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.

@nx-cloud

nx-cloud Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit 28d2507

Command Status Duration Result
nx run-many -t build --no-agents ✅ Succeeded <1s View ↗
nx affected -t build lint test typecheck e2e-ci ✅ Succeeded 2m 5s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-29 22:10:50 UTC

@pkg-pr-new

pkg-pr-new Bot commented Jul 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

@forgerock/davinci-client

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

@forgerock/device-client

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

@forgerock/journey-client

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

@forgerock/oidc-client

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

@forgerock/protect

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

@forgerock/sdk-types

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

@forgerock/sdk-utilities

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

@forgerock/iframe-manager

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

@forgerock/sdk-logger

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

@forgerock/sdk-oidc

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

@forgerock/sdk-request-middleware

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

@forgerock/storage

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

@forgerock/sdk-store

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

commit: 28d2507

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Deployed 82c3792 to https://ForgeRock.github.io/ping-javascript-sdk/pr-729/82c3792bdb4a42617c69295d8a64b100fc663665 branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

🔻 @forgerock/sdk-oidc - 3.5 KB (-2.2 KB, -38.4%)

🆕 New Packages

🆕 @forgerock/sdk-store - 10.7 KB (new)
🆕 @forgerock/journey-client - 92.5 KB (new)
🆕 @forgerock/journey-client - 0.0 KB (new)
🆕 @forgerock/device-client - 10.0 KB (new)
🆕 @forgerock/device-client - 0.0 KB (new)

📊 Minor Changes

📉 @forgerock/davinci-client - 54.3 KB (-0.9 KB)
📈 @forgerock/sdk-types - 9.1 KB (+0.0 KB)
📉 @forgerock/oidc-client - 34.8 KB (-0.5 KB)

➖ No Changes

@forgerock/sdk-logger - 1.6 KB
@forgerock/iframe-manager - 3.2 KB
@forgerock/storage - 1.5 KB
@forgerock/sdk-request-middleware - 4.6 KB
@forgerock/sdk-utilities - 18.6 KB
@forgerock/protect - 144.6 KB


15 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

@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.19608% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.59%. Comparing base (eafe277) to head (28d2507).
⚠️ Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
packages/davinci-client/src/lib/client.store.ts 66.66% 5 Missing ⚠️
packages/davinci-client/src/lib/davinci.api.ts 70.58% 5 Missing ⚠️
packages/journey-client/src/lib/client.store.ts 54.54% 5 Missing ⚠️
packages/oidc-client/src/lib/oidc.api.ts 80.00% 4 Missing ⚠️
packages/sdk-effects/store/src/index.ts 20.00% 4 Missing ⚠️
packages/journey-client/src/lib/journey.api.ts 92.85% 1 Missing ⚠️
packages/oidc-client/src/types.ts 50.00% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (23.59%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #729      +/-   ##
==========================================
+ Coverage   18.07%   23.59%   +5.52%     
==========================================
  Files         155      163       +8     
  Lines       24398    25765    +1367     
  Branches     1203     1674     +471     
==========================================
+ Hits         4410     6080    +1670     
+ Misses      19988    19685     -303     
Files with missing lines Coverage Δ
...ges/davinci-client/src/lib/client.store.effects.ts 49.73% <ø> (ø)
...kages/davinci-client/src/lib/client.store.utils.ts 57.14% <100.00%> (+33.41%) ⬆️
packages/davinci-client/src/types.ts 12.50% <100.00%> (ø)
...kages/journey-client/src/lib/client.store.utils.ts 100.00% <100.00%> (ø)
packages/oidc-client/src/lib/client.store.ts 50.00% <100.00%> (+22.29%) ⬆️
packages/oidc-client/src/lib/client.store.utils.ts 62.96% <100.00%> (+1.92%) ⬆️
packages/oidc-client/src/lib/client.types.ts 100.00% <ø> (ø)
packages/sdk-effects/oidc/src/index.ts 25.00% <ø> (ø)
...ackages/sdk-effects/store/src/lib/store.effects.ts 100.00% <100.00%> (ø)
packages/sdk-effects/store/src/lib/store.types.ts 100.00% <100.00%> (ø)
... and 11 more

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ryanbas21
ryanbas21 force-pushed the centralize-redux-stores branch 2 times, most recently from 410cb72 to 9a87a40 Compare July 28, 2026 20:00
Allow davinci(), journey(), and oidc() to share a single Redux store, so
the OpenID Connect discovery document is fetched once regardless of how many
clients are initialised. Three ownership modes are supported:

  // Mode 1 — implicit (unchanged default)
  const client = await oidc({ config });

  // Mode 2 — one client owns the store (primary sharing story)
  const dv = await davinci({ config: davinciConfig });
  const oc = await oidc({ config: oidcConfig, store: dv.store });

  // Mode 3 — consumer owns the store
  const store = createSdkStore();
  await davinci({ config: davinciConfig, store });
  await oidc({ config: oidcConfig, store });

New @forgerock/sdk-store package (scope:sdk-effects) owns:
- The single canonical wellknownApi instance and its RTK Query selectors.
  createWellknownSelector was rebuilt on every call; it is now memoized per
  URL via a module-level Map, restoring the memoization that was always cold.
- initWellknownQuery and isValidWellknownResponse (moved from sdk-oidc).
- The shared store contract: SdkStore / SdkStoreHandle / createSdkStore /
  injectClient / isSdkStoreHandle.
- clientExtra() — the per-client slot resolver used by every *.api.ts.

Request middleware and logger are scoped per client (security fix):
Before, every *.api.ts resolved its middleware and logger from the store's
store-wide thunk extraArgument. On a shared store that extra belongs to the
owning client, so DaVinci middleware ran against AUTHORIZE, PAR,
TOKEN_EXCHANGE, REVOKE, USER_INFO and END_SESSION, and oidc's own logger was
silently discarded. extraArgument is now a registry keyed by each api's
reducerPath; clientExtra() returns only the calling client's slot. A missing
or malformed slot yields empty middleware and an error-level fallback logger —
never another client's values.

Public API changes:
- oidc() takes store as part of its options object (not a positional second
  arg), consistent with every other factory in the SDK.
- davinci() and journey() expose store: SdkStore on the returned client.
  Both factories also accept store?: SdkStore as input (mode 2 / 3).
- One OIDC client per store: a second oidc() with a different clientId
  returns argument_error instead of silently overwriting token state.
- oidc() validates arguments before injecting into a store (RTK inject is
  irreversible); a rejected call leaves a caller-owned store unchanged.
- A value that fails isSdkStoreHandle() returns argument_error, not TypeError.

Store contract is declared once structurally:
InjectableStore was declared three times — twice identically and once as a
weaker mirror — joined only by `as unknown as`. The fictional __sdkStoreBrand
required casts on both sides with no compile-time link between them. There
is now one structural interface in sdk-store; producers satisfy it without a
cast, consumers receive it without a cast. The two unavoidable widenings live
in store.effects.ts with documented rationale. Deleted: toSdkStore,
fromSdkStore x3, InjectableStore x3, __sdkStoreBrand, JourneyStore,
injectIntoStore, the requestMiddleware log.warn (obsolete under per-client
scoping), and the two tests that existed only to cover that warning.

Layering enforcement:
sdk-store previously depended on sdk-oidc to access initWellknownQuery,
which violated the scope:sdk-effects constraint that allows only sdk-types
and sdk-utilities. Moving the file removes the only cross-effects dependency
and promotes enforce-module-boundaries from warn to error. All 23 affected
projects lint clean.

Tests:
- sdk-store: 40 tests (was 0; passWithNoTests removed).
- wellknownApi: cache-per-URL, error mapping, selector memoization (instance
  identity asserted).
- clientExtra: never returns another client's slot; degrades gracefully on
  any malformed extra.
- Middleware isolation: DaVinci middleware does not run against OIDC requests,
  proven for both the foreign-slot shape and the old flat shape.
- State-shape assertions: combineSlices keys are now implicit (via slice.name)
  rather than literal; pinned so a rename fails here, not in selectors.
- shared-store.test.ts fully rewritten: fake handle removed; all three D1
  modes tested against the real factories and real createSdkStore().
- store-lifecycle.test.ts: validate-before-inject; clientId conflict guard.
- store.effects.test.ts: createSdkStore, isSdkStoreHandle, injectClient.

E2E:
- Playwright suite in davinci-suites asserts exactly one .well-known network
  request when davinci() and oidc() share a store (mode 2). Requests are
  intercepted via page.route() so no live credential is needed.

BREAKING (sdk-oidc): initWellknownQuery and isValidWellknownResponse are
removed from @forgerock/sdk-oidc and are now exported from @forgerock/sdk-store.
Update imports if you were using them directly.
@ryanbas21
ryanbas21 force-pushed the centralize-redux-stores branch from 9a87a40 to aae6ae5 Compare July 28, 2026 23:12
@ryanbas21 ryanbas21 changed the title chore: centralize-redux-stores feat(sdk): shared store for multiple SDK clients + @forgerock/sdk-store Jul 28, 2026
* not require a live PingOne endpoint.
*/

const WELLKNOWN_URL = 'https://sdk-test.example.com/as/.well-known/openid-configuration';
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