feat: expose subscribe method on journey-client and oidc-client#633
feat: expose subscribe method on journey-client and oidc-client#633ryanbas21 wants to merge 1 commit into
Conversation
Aligns journey-client and oidc-client with davinci-client's public API by exporting store.subscribe from each client factory function.
🦋 Changeset detectedLatest commit: 6519fcc The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
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 |
|
View your CI Pipeline Execution ↗ for commit 6519fcc
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughBoth ChangesSubscribe Method Export
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (7)
packages/journey-client/src/lib/client.store.ts (1)
39-39: ⚡ Quick winConsider using
Unsubscribetype from@reduxjs/toolkitfor consistency.The return type
() => voidworks but is less precise than Redux Toolkit'sUnsubscribetype. According to the review context,oidc-clientimports and uses theUnsubscribetype for the same method being added in this PR. Using the same type would improve consistency across both clients and provide better type documentation.♻️ Suggested type refinement
+import type { Unsubscribe } from '@reduxjs/toolkit'; + /** The journey client instance returned by the `journey()` function. */ export interface JourneyClient { - subscribe: (listener: () => void) => () => void; + subscribe: (listener: () => void) => Unsubscribe; start: (options?: StartParam) => Promise<JourneyResult>;🤖 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 `@packages/journey-client/src/lib/client.store.ts` at line 39, The subscribe method signature currently uses a generic () => void return type; replace that with the Redux Toolkit Unsubscribe type to match other clients: import Unsubscribe from '@reduxjs/toolkit' (or import { Unsubscribe } if needed) and change the signature on subscribe (the subscribe declaration in client.store.ts) from subscribe: (listener: () => void) => () => void to subscribe: (listener: () => void) => Unsubscribe so the type is consistent with oidc-client and provides clearer docs.packages/journey-client/api-report/journey-client.api.md (2)
197-197: ⚡ Quick winAdd JSDoc documentation for the new public
subscribemethod.The
subscribemethod is marked as// (undocumented), meaning it lacks JSDoc comments in the source file. For a new public API being added in a minor version bump, documentation would help consumers understand:
- The purpose of the method (listening to store state changes)
- When the listener is invoked
- How to access state inside the listener
- What the returned unsubscribe function does
🤖 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 `@packages/journey-client/api-report/journey-client.api.md` at line 197, Add JSDoc for the public subscribe method to replace the "(undocumented)" tag: document purpose (listening to store state changes), describe when the listener is invoked (after state updates), specify the listener signature and how to access the current state from the provided callback (e.g., receives state or a selector), and explain that subscribe returns an unsubscribe function that removes the listener when called; apply this to the subscribe method declaration so consumers see purpose, invocation timing, parameter details, and that the return value is a cleanup function.
197-197: ⚡ Quick winUse
Unsubscribetype for consistency with oidc-client.Journey Client's
subscribemethod returns() => void, while oidc-client uses theUnsubscribetype (imported from@reduxjs/toolkit) for its subscribe method's return type. Consider using the sameUnsubscribetype here for consistency across both client packages.🤖 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 `@packages/journey-client/api-report/journey-client.api.md` at line 197, The subscribe method currently returns a bare () => void; change its return type to use the Unsubscribe type (imported from `@reduxjs/toolkit`) for consistency with oidc-client — update the method signature for subscribe (and any related types/exports) to import Unsubscribe and annotate the return type as Unsubscribe so callers and API docs match oidc-client.packages/oidc-client/src/lib/client.store.ts (1)
98-99: Add JSDoc documentation for the new public API method.The
subscribemethod is now part of the public client API but lacks documentation. Adding JSDoc here will improve developer experience by explaining the subscription pattern, listener signature, and how to unsubscribe.📚 Suggested documentation
+/** + * `@method` subscribe + * `@description` Subscribes to store state changes. The listener will be called whenever the state updates. + * `@param` {() => void} listener - Callback function invoked on each state change. + * `@returns` {Unsubscribe} - Function to call to unsubscribe the listener. + */ subscribe: store.subscribe,🤖 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 `@packages/oidc-client/src/lib/client.store.ts` around lines 98 - 99, Add JSDoc for the newly exported public subscribe method so developers understand the subscription pattern, listener signature, and how to unsubscribe: update the object that passes store.subscribe (the subscribe property) in client.store.ts to include a JSDoc block above it describing that subscribe is a public method on the client, the expected listener callback signature (e.g., receives the current client state or change), that it returns an unsubscribe function, and any edge cases (immediate vs. deferred invocation). Target the subscribe property being assigned to store.subscribe so the docs live next to the public API surface.packages/journey-client/api-report/journey-client.types.api.md (1)
184-184: ⚡ Quick winConsider using the
Unsubscribetype for consistency with oidc-client.The oidc-client package imports and uses
Unsubscribefrom@reduxjs/toolkitfor its subscribe method return type (seepackages/oidc-client/api-report/oidc-client.types.api.mdline 261), while journey-client uses the generic() => void. Since both packages expose Redux store subscriptions, using the semanticUnsubscribetype consistently across both clients improves API clarity and maintainability.📝 Suggested alignment
Import the type in your source file:
+import type { Unsubscribe } from '@reduxjs/toolkit';Then update the interface:
-subscribe: (listener: () => void) => () => void; +subscribe: (listener: () => void) => Unsubscribe;🤖 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 `@packages/journey-client/api-report/journey-client.types.api.md` at line 184, The subscribe signature currently uses a raw function type; change it to use the semantic Unsubscribe type from `@reduxjs/toolkit` for consistency with oidc-client: import Unsubscribe from '@reduxjs/toolkit' (or from the same place oidc-client uses) and update the subscribe declaration from subscribe: (listener: () => void) => () => void to subscribe: (listener: () => void) => Unsubscribe so the return type is the named Unsubscribe type.packages/journey-client/src/lib/client.store.test.ts (1)
111-111: Consider testing the subscription functionality, not just existence.The test verifies that
subscribeis a function but doesn't confirm that it actually notifies listeners on state changes or that the returned unsubscribe function works. The oidc-client test (line 140-141) goes one step further by verifying the unsubscribe return value.💡 Suggested enhancement
expect(client.subscribe).toBeInstanceOf(Function); +const listener = vi.fn(); +const unsubscribe = client.subscribe(listener); +expect(unsubscribe).toBeInstanceOf(Function); +// Optionally: trigger a state change and verify listener was called🤖 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 `@packages/journey-client/src/lib/client.store.test.ts` at line 111, The test currently only asserts client.subscribe is a Function; enhance it to verify subscription behavior by subscribing a mock listener (e.g., const listener = jest.fn()) via client.subscribe(listener), trigger a state change using the store update method used elsewhere in tests (e.g., client.setState(...) or client.update(...)/client.set(...)), assert listener was called with the new state, capture the return value from client.subscribe and call it to unsubscribe, then trigger another state change and assert listener was not called again; reference client.subscribe and the store's state mutation method to locate the code to change.packages/oidc-client/src/lib/client.store.test.ts (1)
132-142: Consider verifying that listeners are notified on state changes.The test confirms that
subscribereturns an unsubscribe function, which is good. For additional confidence, you could trigger a store action and verify the listener is called, though the current test provides reasonable coverage for the direct delegation tostore.subscribe.🤖 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 `@packages/oidc-client/src/lib/client.store.test.ts` around lines 132 - 142, Add an assertion that listeners are notified when the store changes: create a mock listener (vi.fn()), subscribe it via oidcClient.subscribe, trigger a store-changing method on the client (for example call a method that updates state such as saveClient or setToken on oidcClient), assert the mock listener was called, then call the returned unsubscribe and optionally assert it is a function; use the existing oidc, oidcClient and subscribe identifiers to locate where to add this behavior.
🤖 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.
Nitpick comments:
In `@packages/journey-client/api-report/journey-client.api.md`:
- Line 197: Add JSDoc for the public subscribe method to replace the
"(undocumented)" tag: document purpose (listening to store state changes),
describe when the listener is invoked (after state updates), specify the
listener signature and how to access the current state from the provided
callback (e.g., receives state or a selector), and explain that subscribe
returns an unsubscribe function that removes the listener when called; apply
this to the subscribe method declaration so consumers see purpose, invocation
timing, parameter details, and that the return value is a cleanup function.
- Line 197: The subscribe method currently returns a bare () => void; change its
return type to use the Unsubscribe type (imported from `@reduxjs/toolkit`) for
consistency with oidc-client — update the method signature for subscribe (and
any related types/exports) to import Unsubscribe and annotate the return type as
Unsubscribe so callers and API docs match oidc-client.
In `@packages/journey-client/api-report/journey-client.types.api.md`:
- Line 184: The subscribe signature currently uses a raw function type; change
it to use the semantic Unsubscribe type from `@reduxjs/toolkit` for consistency
with oidc-client: import Unsubscribe from '@reduxjs/toolkit' (or from the same
place oidc-client uses) and update the subscribe declaration from subscribe:
(listener: () => void) => () => void to subscribe: (listener: () => void) =>
Unsubscribe so the return type is the named Unsubscribe type.
In `@packages/journey-client/src/lib/client.store.test.ts`:
- Line 111: The test currently only asserts client.subscribe is a Function;
enhance it to verify subscription behavior by subscribing a mock listener (e.g.,
const listener = jest.fn()) via client.subscribe(listener), trigger a state
change using the store update method used elsewhere in tests (e.g.,
client.setState(...) or client.update(...)/client.set(...)), assert listener was
called with the new state, capture the return value from client.subscribe and
call it to unsubscribe, then trigger another state change and assert listener
was not called again; reference client.subscribe and the store's state mutation
method to locate the code to change.
In `@packages/journey-client/src/lib/client.store.ts`:
- Line 39: The subscribe method signature currently uses a generic () => void
return type; replace that with the Redux Toolkit Unsubscribe type to match other
clients: import Unsubscribe from '@reduxjs/toolkit' (or import { Unsubscribe }
if needed) and change the signature on subscribe (the subscribe declaration in
client.store.ts) from subscribe: (listener: () => void) => () => void to
subscribe: (listener: () => void) => Unsubscribe so the type is consistent with
oidc-client and provides clearer docs.
In `@packages/oidc-client/src/lib/client.store.test.ts`:
- Around line 132-142: Add an assertion that listeners are notified when the
store changes: create a mock listener (vi.fn()), subscribe it via
oidcClient.subscribe, trigger a store-changing method on the client (for example
call a method that updates state such as saveClient or setToken on oidcClient),
assert the mock listener was called, then call the returned unsubscribe and
optionally assert it is a function; use the existing oidc, oidcClient and
subscribe identifiers to locate where to add this behavior.
In `@packages/oidc-client/src/lib/client.store.ts`:
- Around line 98-99: Add JSDoc for the newly exported public subscribe method so
developers understand the subscription pattern, listener signature, and how to
unsubscribe: update the object that passes store.subscribe (the subscribe
property) in client.store.ts to include a JSDoc block above it describing that
subscribe is a public method on the client, the expected listener callback
signature (e.g., receives the current client state or change), that it returns
an unsubscribe function, and any edge cases (immediate vs. deferred invocation).
Target the subscribe property being assigned to store.subscribe so the docs live
next to the public API surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac2a5978-f319-4dae-aa1b-9b2a551ee5d1
📒 Files selected for processing (9)
.changeset/export-subscribe-method.mdpackages/journey-client/api-report/journey-client.api.mdpackages/journey-client/api-report/journey-client.types.api.mdpackages/journey-client/src/lib/client.store.test.tspackages/journey-client/src/lib/client.store.tspackages/oidc-client/api-report/oidc-client.api.mdpackages/oidc-client/api-report/oidc-client.types.api.mdpackages/oidc-client/src/lib/client.store.test.tspackages/oidc-client/src/lib/client.store.ts
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud has identified a possible root cause for your failed CI:
We reviewed all failing tasks and none are related to the changes introduced in this PR. The OIDC and protect suite failures are caused by an external PingAM/Forgeblocks service returning invalid request errors, confirmed as a pre-existing issue also present in branch 618. The api-report timeout is a local environment timing issue unrelated to the subscribe method additions.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
1 participant
Summary
subscribeto theJourneyClientinterface and exposesstore.subscribefrom thejourney()factory functionstore.subscribefrom theoidc()factory functiondavinci-clientChangeset
Minor bump for
@forgerock/journey-clientand@forgerock/oidc-client— backwards-compatible public API addition.Summary by CodeRabbit
Release Notes
subscribemethod, allowing you to listen for store state changes. Callsubscribe(listener)to register a listener function, which returns an unsubscribe function for cleanup.