Conversation
📝 WalkthroughWalkthroughThe PR adds a User generic and createUser flow across core and React layers, implements createGetUser (caching, subscriptions, refresh), and wires getUser/subscribeToUserChange/refreshUser into Oidc LoggedIn and SPA utilities, updating types and builders accordingly. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Oidc as OIDC Core
participant GetUser as createGetUser Controller
participant CreateUser as createUser Callback
participant UserInfo as UserInfo Endpoint
participant Tokens as Token Store / Events
participant Listeners as User Change Listeners
App->>Oidc: call getUser() / subscribeToUserChange()
Oidc->>GetUser: delegate getUser / register listener
GetUser->>Tokens: read current tokens / decodedIdToken
GetUser->>GetUser: compute hash(accessToken, decodedIdToken)
alt no cached user or hash changed
GetUser->>CreateUser: invoke createUser({accessToken, decodedIdToken, fetchUserInfo, issuerUri})
CreateUser->>UserInfo: fetch userinfo (Bearer accessToken)
UserInfo-->>CreateUser: user payload
CreateUser-->>GetUser: resolved User
GetUser->>Listeners: notify (newUser, previousUser)
else cached user valid
GetUser-->>Oidc: return cached User
end
Tokens->>GetUser: token change event
GetUser->>GetUser: recompute hash -> if changed trigger createUser flow and notify listeners
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/react-spa/createOidcSpaUtils.ts (2)
258-276:⚠️ Potential issue | 🟠 MajorMissing
Usergeneric onUseOidc.Oidc.LoggedIn<DecodedIdToken>.
UseOidc.Oidc.LoggedInnow requires two type parameters<DecodedIdToken, User>(seesrc/react-spa/types.tsline 47), but onlyDecodedIdTokenis supplied here. This will cause a compile error sinceUserhas no default. The returned object also doesn't include theuserorrefreshUserproperties thatLoggedIn<DecodedIdToken, User>now expects.- return id<UseOidc.Oidc.LoggedIn<DecodedIdToken>>({ + return id<UseOidc.Oidc.LoggedIn<DecodedIdToken, User>>({ isUserLoggedIn: true, ... + user: ..., // needs wiring + refreshUser: ..., // needs wiring });
315-316:⚠️ Potential issue | 🟠 MajorMissing
Usergeneric onGetOidc.Oidc.LoggedIn<DecodedIdToken>.Same issue as
useOidc:GetOidc.Oidc.LoggedInrequires<DecodedIdToken, User>but only one generic is provided. ThegetUserfield is also absent from the returned object.
🤖 Fix all issues with AI agents
In `@src/core/createGetUser.ts`:
- Around line 126-144: The getUser implementation references undefined
identifiers; change references of prUser to the state-held promise (use
state?.prUser or read const prUser = state?.prUser at top of getUser) and
replace the call to __updatePrUser() with the correct function
__updatePrUserIfHashChanged(); ensure the assert and subsequent await use the
same state-derived prUser variable and that refreshUser and
subscribeToUserChange are returned unchanged.
- Around line 69-114: __updatePrUserIfHashChanged currently references tokens
before they are fetched and relies on computeHash which returns nothing; fix by
first awaiting getTokens() at the top of __updatePrUserIfHashChanged and use
that tokens object when calling computeHash and later when calling createUser
(keep passing tokens.decodedIdToken_original to createUser as before), and
implement computeHash to return a deterministic string (e.g., compute a stable
SHA-256 or other stable digest over the accessToken and decodedIdToken payload)
so hash comparisons work correctly; ensure references to computeHash, getTokens,
__updatePrUserIfHashChanged, and createUser are updated to use the same tokens
source.
- Line 124: Replace the no-op token-change subscription with a call to refresh
the stored user state: change the subscribeToTokensChange callback from an empty
function to invoke __updatePrUserIfHashChanged() (e.g.,
subscribeToTokensChange(tokens => __updatePrUserIfHashChanged());) so
rotating/updated tokens trigger the user hash check and refresh; ensure you
import or reference __updatePrUserIfHashChanged in the same scope where
subscribeToTokensChange is used.
- Around line 96-105: The forEach callback currently returns the call expression
which triggers the Biome lint rule; in the block where you await prUser_current
and iterate on onUserChanges, change the arrow callback passed to
onUserChanges.forEach from an expression body to a block body that calls
onUserChange({...}); (i.e., wrap the call in braces so it becomes a void
statement) so functions like onUserChange({ user: user_new, user_previous:
user_current }) do not implicitly return a value.
- Around line 46-53: The userinfo fetch in createGetUser (the request to
userinfo_endpoint using accessToken) doesn't check HTTP status; update the logic
to verify r.ok before calling r.json(): if !r.ok, attempt to read/parse the
error body safely (fallback to r.statusText) and throw or return a structured
error including status and message, otherwise return the parsed JSON; ensure the
Authorization header and existing callsites of createGetUser remain unchanged
but now receive a thrown error or explicit error object on non-2xx responses.
- Around line 116-122: The refreshUser implementation is fire-and-forget and
returns undefined; update refreshUser (typed as R_GetUser["refreshUser"]) to
await the token renewal and return the refreshed User: inside refreshUser set
state.hash = "" as before, then await renewTokens() (i.e., const refreshed =
await renewTokens() or simply await renewTokens()), and finally return the
refreshed User (either the value returned by renewTokens() or the current user
from state/getter) so the function resolves to a Promise<User> as declared.
In `@src/core/createOidc.ts`:
- Line 68: The file createOidc.ts fails Prettier formatting; run the project's
formatter (yarn format) or apply Prettier to src/core/createOidc.ts to fix style
issues so the CI passes; locate the exported createOidc function and surrounding
imports/exports in that file and ensure the formatter updates whitespace, line
breaks and trailing commas per project Prettier config, then commit the
formatted file.
- Around line 1942-1956: The current getUser stub is unsafe: it returns user:
null as any and refreshUser as a void-returning function, which violates the
Oidc.LoggedIn signature and hides type errors; replace the stub with a proper
implementation using the createGetUser factory (import createGetUser) so the
returned object matches Oidc.LoggedIn, ensure user has the correct typed shape
(not any/null) and that refreshUser returns Promise<User> (or rejects) per the
declared signature, and wire this up to the existing createUser check so
createGetUser is used when createUser is provided.
- Around line 72-86: ParamsOfCreateOidc uses the generic type MaybeAsync in the
createUser signature but MaybeAsync is not imported; add a type-only import for
MaybeAsync at the top of this module (so the compiler can resolve MaybeAsync
used in ParamsOfCreateOidc and the createUser param typings) importing it from
the module where MaybeAsync is defined and ensure it is imported as a type
(e.g., `import type { MaybeAsync } from '...'`) to avoid runtime imports.
In `@src/react-spa/createOidcSpaUtils.ts`:
- Around line 33-38: The createUser and user_mock parameters are destructured
but never wired into the OIDC flow; update the factory to pass createUser and
user_mock into createOidc (or into the mock branch that builds the logged-in
state) and ensure the returned useOidc/getOidc implementations populate
user-related fields: set UseOidc.Oidc.LoggedIn.{user,refreshUser} and
GetOidc.Oidc.LoggedIn.getUser to use createUser (async if needed) and to fall
back to user_mock (or decodedIdToken_mock) when present; locate usages around
createOidc, useOidc and getOidc in this module and thread the
createUser/user_mock values through the mock login/initialization paths so the
user is populated.
- Line 40: ParamsOfBootstrap is instantiated with only two generics in the
declaration of dParamsOfBootstrap; update the instantiation to include the third
User generic to match the updated type signature. Locate the declaration const
dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin,
DecodedIdToken>>(); and change the generic to ParamsOfBootstrap<AutoLogin,
DecodedIdToken, User> (import or reference the appropriate User type if needed)
so the Deferred uses the correct three-parameter type.
In `@src/react-spa/types.ts`:
- Around line 141-147: The getUser return type currently declares refreshUser as
() => void but this operation is asynchronous; update the signature in
src/react-spa/types.ts so refreshUser returns a Promise (e.g., change
refreshUser: () => void to refreshUser: () => Promise<void>) to match other
Oidc.LoggedIn variants and allow callers to await the token renewal/user refetch
in the getUser result.
🧹 Nitpick comments (3)
src/core/createGetUser.ts (1)
1-6: Unused import:decodeJwt.
decodeJwtis imported at line 2 but never used in this file.src/core/createOidc.ts (1)
358-363:createOidcfunction signature doesn't exposeUsergeneric.The public
createOidcfunction doesn't includeUseras a generic parameter, and the return type still usesOidc.LoggedIn<DecodedIdToken>(withUserdefaulting tonever). WhileParamsOfCreateOidcacceptscreateUser, the function can't propagate the user type to callers. This will need to be addressed when completing the feature.src/react-spa/utilsBuilder.ts (1)
36-52:withUser'screateUsersignature includesfetchUserInfoandissuerUri, but the internal builder type at line 69 does not.The public
withUsermethod accepts acreateUsercallback with{ decodedIdToken, accessToken, fetchUserInfo, issuerUri }, but the internalcreateOidcSpaUtilsBuilderparam type (line 68-70) only has{ decodedIdToken: DecodedIdToken; accessToken: string }. This mismatch is masked by the@ts-expect-errorat line 96.When
createOidcSpaUtilseventually wires this through, it will need the full signature. Consider aligning the internal type to avoid the suppress:createUser: - | ((params: { decodedIdToken: DecodedIdToken; accessToken: string }) => MaybeAsync<User>) + | ((params: { + decodedIdToken: DecodedIdToken; + accessToken: string; + fetchUserInfo: () => Promise<{ [key: string]: unknown; sub: string }>; + issuerUri: string; + }) => MaybeAsync<User>) | undefined;
| const r = await fetch(userinfo_endpoint, { | ||
| headers: { | ||
| Authorization: `Bearer ${accessToken}` | ||
| } | ||
| }); | ||
|
|
||
| return r.json(); | ||
| } |
There was a problem hiding this comment.
No HTTP error handling for the userinfo endpoint response.
fetch will not throw on 4xx/5xx responses. If the authorization server returns a 401 (expired/invalid token) or 500, r.json() will parse the error body (or fail if the body isn't JSON), producing misleading results. Check r.ok before parsing.
🛡️ Proposed fix
const r = await fetch(userinfo_endpoint, {
headers: {
Authorization: `Bearer ${accessToken}`
}
});
+ if (!r.ok) {
+ throw new Error(
+ `oidc-spa: userinfo endpoint responded with ${r.status} ${r.statusText}`
+ );
+ }
+
return r.json();🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 46 - 53, The userinfo fetch in
createGetUser (the request to userinfo_endpoint using accessToken) doesn't check
HTTP status; update the logic to verify r.ok before calling r.json(): if !r.ok,
attempt to read/parse the error body safely (fallback to r.statusText) and throw
or return a structured error including status and message, otherwise return the
parsed JSON; ensure the Authorization header and existing callsites of
createGetUser remain unchanged but now receive a thrown error or explicit error
object on non-2xx responses.
| function __updatePrUserIfHashChanged() { | ||
| assert(createUser !== undefined, "94302"); | ||
|
|
||
| const hash_current = state?.hash; | ||
|
|
||
| const hash_new = computeHash({ | ||
| accessToken: tokens.accessToken, | ||
| decodedIdToken: tokens.decodedIdToken | ||
| }); | ||
|
|
||
| const prUser_new = (async () => { | ||
| const tokens = await getTokens(); | ||
|
|
||
| const prUser_current = state?.prUser; | ||
|
|
||
| if (hash_current === hash_new) { | ||
| assert(prUser_current !== undefined); | ||
| return prUser_current; | ||
| } | ||
|
|
||
| const user_new = await createUser({ | ||
| accessToken: tokens.accessToken, | ||
| decodedIdToken: tokens.decodedIdToken_original, | ||
| issuerUri, | ||
| fetchUserInfo: () => fetchUserInfo({ accessToken: tokens.accessToken }) | ||
| }); | ||
|
|
||
| { | ||
| const user_current = await prUser_current; | ||
|
|
||
| onUserChanges.forEach(onUserChange => | ||
| onUserChange({ | ||
| user: user_new, | ||
| user_previous: user_current | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| return user_new; | ||
| })(); | ||
|
|
||
| state = { | ||
| hash: hash_new, | ||
| prUser: prUser_new | ||
| }; | ||
| } |
There was a problem hiding this comment.
Multiple critical bugs in __updatePrUserIfHashChanged.
-
tokensis not in scope at lines 74-76. The variabletokensis only declared inside the async IIFE at line 80 (const tokens = await getTokens()), butcomputeHashat line 74 referencestokens.accessTokenandtokens.decodedIdTokenin the outer function scope where no such variable exists. This is aReferenceErrorat runtime. -
computeHash(line 149-152) has an empty body — it declares a return type ofstringbut returns nothing (undefined). Every hash comparison will compareundefined === undefined, so the "hash changed" logic will never work correctly.
These two issues together mean user creation/refresh will never work as intended.
🧰 Tools
🪛 Biome (2.3.13)
[error] 99-99: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 69 - 114, __updatePrUserIfHashChanged
currently references tokens before they are fetched and relies on computeHash
which returns nothing; fix by first awaiting getTokens() at the top of
__updatePrUserIfHashChanged and use that tokens object when calling computeHash
and later when calling createUser (keep passing tokens.decodedIdToken_original
to createUser as before), and implement computeHash to return a deterministic
string (e.g., compute a stable SHA-256 or other stable digest over the
accessToken and decodedIdToken payload) so hash comparisons work correctly;
ensure references to computeHash, getTokens, __updatePrUserIfHashChanged, and
createUser are updated to use the same tokens source.
| { | ||
| const user_current = await prUser_current; | ||
|
|
||
| onUserChanges.forEach(onUserChange => | ||
| onUserChange({ | ||
| user: user_new, | ||
| user_previous: user_current | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
forEach callback implicitly returns a value (Biome lint warning).
The arrow function passed to forEach returns the result of onUserChange(...). While harmless at runtime, it triggers the Biome useIterableCallbackReturn rule. Wrap the call in braces to make it a void statement.
Proposed fix
- onUserChanges.forEach(onUserChange =>
- onUserChange({
+ onUserChanges.forEach(onUserChange => {
+ onUserChange({
user: user_new,
user_previous: user_current
- })
- );
+ });
+ });📝 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.
| { | |
| const user_current = await prUser_current; | |
| onUserChanges.forEach(onUserChange => | |
| onUserChange({ | |
| user: user_new, | |
| user_previous: user_current | |
| }) | |
| ); | |
| } | |
| { | |
| const user_current = await prUser_current; | |
| onUserChanges.forEach(onUserChange => { | |
| onUserChange({ | |
| user: user_new, | |
| user_previous: user_current | |
| }); | |
| }); | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 99-99: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@src/core/createGetUser.ts` around lines 96 - 105, The forEach callback
currently returns the call expression which triggers the Biome lint rule; in the
block where you await prUser_current and iterate on onUserChanges, change the
arrow callback passed to onUserChanges.forEach from an expression body to a
block body that calls onUserChange({...}); (i.e., wrap the call in braces so it
becomes a void statement) so functions like onUserChange({ user: user_new,
user_previous: user_current }) do not implicitly return a value.
| createUser: | ||
| | ((params: { decodedIdToken: DecodedIdToken; accessToken: string }) => MaybeAsync<User>) | ||
| | undefined; | ||
| user_mock: User | undefined; | ||
| }): OidcSpaUtils<AutoLogin, DecodedIdToken, User> { | ||
| const { autoLogin, decodedIdTokenSchema, decodedIdToken_mock, createUser, user_mock } = params; |
There was a problem hiding this comment.
createUser and user_mock are accepted but never wired into the OIDC flow.
These params are destructured but never passed to createOidc, the mock flow, or used to construct the user-related properties on the returned useOidc/getOidc objects. This means the user/refreshUser fields on UseOidc.Oidc.LoggedIn and getUser on GetOidc.Oidc.LoggedIn will not be populated. Presumably this is a WIP gap — flagging for tracking.
🤖 Prompt for AI Agents
In `@src/react-spa/createOidcSpaUtils.ts` around lines 33 - 38, The createUser and
user_mock parameters are destructured but never wired into the OIDC flow; update
the factory to pass createUser and user_mock into createOidc (or into the mock
branch that builds the logged-in state) and ensure the returned useOidc/getOidc
implementations populate user-related fields: set
UseOidc.Oidc.LoggedIn.{user,refreshUser} and GetOidc.Oidc.LoggedIn.getUser to
use createUser (async if needed) and to fall back to user_mock (or
decodedIdToken_mock) when present; locate usages around createOidc, useOidc and
getOidc in this module and thread the createUser/user_mock values through the
mock login/initialization paths so the user is populated.
| }): OidcSpaUtils<AutoLogin, DecodedIdToken, User> { | ||
| const { autoLogin, decodedIdTokenSchema, decodedIdToken_mock, createUser, user_mock } = params; | ||
|
|
||
| const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>(); |
There was a problem hiding this comment.
ParamsOfBootstrap missing third User generic.
ParamsOfBootstrap in types.ts is now ParamsOfBootstrap<AutoLogin, DecodedIdToken, User>, but here it's instantiated with only two parameters. This will be a type error once the WIP types stabilize.
- const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>();
+ const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken, User>>();📝 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.
| const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>(); | |
| const dParamsOfBootstrap = new Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken, User>>(); |
🤖 Prompt for AI Agents
In `@src/react-spa/createOidcSpaUtils.ts` at line 40, ParamsOfBootstrap is
instantiated with only two generics in the declaration of dParamsOfBootstrap;
update the instantiation to include the third User generic to match the updated
type signature. Locate the declaration const dParamsOfBootstrap = new
Deferred<ParamsOfBootstrap<AutoLogin, DecodedIdToken>>(); and change the generic
to ParamsOfBootstrap<AutoLogin, DecodedIdToken, User> (import or reference the
appropriate User type if needed) so the Deferred uses the correct
three-parameter type.
| getUser: () => Promise<{ | ||
| user: User; | ||
| subscribeToUserChange: (onUserChange: (user: User) => void) => { | ||
| unsubscribeFromUserChange: () => void; | ||
| }; | ||
| refreshUser: () => void; | ||
| }>; |
There was a problem hiding this comment.
refreshUser return type is void — should be Promise<void> (or Promise<User>).
refreshUser will trigger a token renewal + user re-fetch, which are async operations. Declaring it as () => void prevents callers from await-ing the result, which could lead to race conditions. Compare with:
Oidc.LoggedIn(core, line 98):refreshUser: () => Promise<User>UseOidc.Oidc.LoggedIn(line 67):refreshUser: () => Promise<void>GetOidc.Oidc.LoggedIn(here, line 146):refreshUser: () => void← inconsistent
- refreshUser: () => void;
+ refreshUser: () => Promise<void>;📝 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.
| getUser: () => Promise<{ | |
| user: User; | |
| subscribeToUserChange: (onUserChange: (user: User) => void) => { | |
| unsubscribeFromUserChange: () => void; | |
| }; | |
| refreshUser: () => void; | |
| }>; | |
| getUser: () => Promise<{ | |
| user: User; | |
| subscribeToUserChange: (onUserChange: (user: User) => void) => { | |
| unsubscribeFromUserChange: () => void; | |
| }; | |
| refreshUser: () => Promise<void>; | |
| }>; |
🤖 Prompt for AI Agents
In `@src/react-spa/types.ts` around lines 141 - 147, The getUser return type
currently declares refreshUser as () => void but this operation is asynchronous;
update the signature in src/react-spa/types.ts so refreshUser returns a Promise
(e.g., change refreshUser: () => void to refreshUser: () => Promise<void>) to
match other Oidc.LoggedIn variants and allow callers to await the token
renewal/user refetch in the getUser result.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/core/createOidc.ts`:
- Around line 73-86: The ParamsOfCreateOidc User generic isn't threaded through
the factory functions: update both createOidc and createOidc_nonMemoized
signatures to accept the same User generic (e.g., <DecodedIdToken, AutoLogin,
User>) and propagate it into any params and return types (for example change
return types to Oidc.LoggedIn<DecodedIdToken, User> or equivalent) and ensure
functions that call ParamsOfCreateOidc use the User type when typing createUser
and getUser so the resolved user type is preserved instead of defaulting to
never.
- Line 1643: In createOidc.ts the Array.from(onTokenChanges).forEach call uses
an expression arrow which implicitly returns a value and triggers the Biome lint
rule; change the callback to a block-bodied function so it returns void
explicitly (e.g., replace the expression-arrow with a block that just calls
onTokenChange(currentTokens)); reference the onTokenChanges iterable and
currentTokens used in the forEach to locate and update the callback.
🧹 Nitpick comments (2)
src/core/createGetUser.ts (2)
154-188:computeHashusesJSON.stringifywith sorted top-level keys only — nested objects are not sorted.If any non-volatile claim in the ID token or access token contains a nested object whose key order varies across token issuances (some providers do not guarantee key order in JSON), the hash could differ even though the semantic content is identical, causing unnecessary
createUserinvocations. This is unlikely to be a real problem in practice but worth noting.
115-125:refreshUserrelies on synchronous dispatch of token-change events to setstate.After
await renewTokens()(line 120), line 122 assertsstate !== undefined. This only holds ifevtTokensChangelisteners are invoked synchronously within therenewTokenscall path (so that__updatePrUserIfHashChangedruns and setsstatebeforerenewTokensresolves).Confirmed:
Evt.post()insrc/tools/Evt.ts(lines 42-49) dispatches synchronously—it iterates through listeners in a for loop without deferral. However, this is a fragile coupling: if the Evt implementation were ever changed to usesetImmediateor microtasks, the assert would fail. Consider making this dependency explicit or using a different synchronization pattern (e.g., awaiting state updates) to decouple from the implementation detail.
| export type ParamsOfCreateOidc< | ||
| DecodedIdToken extends Record<string, unknown> = Oidc.Tokens.DecodedIdToken_OidcCoreSpec, | ||
| AutoLogin extends boolean = false | ||
| AutoLogin extends boolean = false, | ||
| User = never | ||
| > = { | ||
| createUser?: (params: { | ||
| decodedIdToken: Oidc.Tokens.DecodedIdToken_OidcCoreSpec; | ||
| accessToken: string; | ||
| fetchUserInfo: () => Promise<{ | ||
| [key: string]: unknown; | ||
| sub: string; | ||
| }>; | ||
| issuerUri: string; | ||
| }) => MaybeAsync<User>; |
There was a problem hiding this comment.
User generic not threaded through createOidc or createOidc_nonMemoized.
ParamsOfCreateOidc now accepts a User type parameter (line 76), but createOidc (line 359) and createOidc_nonMemoized (line 439) don't include User in their generic signatures. This means User always defaults to never, making getUser return Promise<{ user: never; ... }> — effectively unusable by callers.
Since this is a WIP PR, flagging for when the feature is completed: both createOidc and createOidc_nonMemoized need User threaded through their generics and return types (e.g., Oidc.LoggedIn<DecodedIdToken, User>).
🤖 Prompt for AI Agents
In `@src/core/createOidc.ts` around lines 73 - 86, The ParamsOfCreateOidc User
generic isn't threaded through the factory functions: update both createOidc and
createOidc_nonMemoized signatures to accept the same User generic (e.g.,
<DecodedIdToken, AutoLogin, User>) and propagate it into any params and return
types (for example change return types to Oidc.LoggedIn<DecodedIdToken, User> or
equivalent) and ensure functions that call ParamsOfCreateOidc use the User type
when typing createUser and getUser so the resolved user type is preserved
instead of defaulting to never.
| }); | ||
| } | ||
|
|
||
| Array.from(onTokenChanges).forEach(onTokenChange => onTokenChange(currentTokens)); |
There was a problem hiding this comment.
forEach callback implicitly returns a value (Biome lint).
Same pattern as in createGetUser.ts. While onTokenChange is typed as returning void, the expression-arrow form triggers useIterableCallbackReturn.
Proposed fix
- Array.from(onTokenChanges).forEach(onTokenChange => onTokenChange(currentTokens));
+ Array.from(onTokenChanges).forEach(onTokenChange => {
+ onTokenChange(currentTokens);
+ });📝 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.
| Array.from(onTokenChanges).forEach(onTokenChange => onTokenChange(currentTokens)); | |
| Array.from(onTokenChanges).forEach(onTokenChange => { | |
| onTokenChange(currentTokens); | |
| }); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 1643-1643: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@src/core/createOidc.ts` at line 1643, In createOidc.ts the
Array.from(onTokenChanges).forEach call uses an expression arrow which
implicitly returns a value and triggers the Biome lint rule; change the callback
to a block-bodied function so it returns void explicitly (e.g., replace the
expression-arrow with a block that just calls onTokenChange(currentTokens));
reference the onTokenChanges iterable and currentTokens used in the forEach to
locate and update the callback.
|
Superceeded by #173 |
1 participant
Summary by CodeRabbit