Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntegrates Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Kinde Client
participant Adapter as KindeStorageAdapter
participant MemStore as MemoryStorage
participant JsUtils as `@kinde/js-utils`
rect rgba(100, 150, 200, 0.5)
Note over Client,Adapter: Store tokens in dual formats
Client->>Adapter: setSessionItem('accessToken', rawJwt)
Adapter->>MemStore: setSessionItem('accessToken', rawJwt)
MemStore-->>Adapter: ack
Client->>Adapter: setSessionItem('kinde_access_token', decodedObj)
Adapter->>MemStore: setSessionItem('kinde_access_token', decodedObj)
MemStore-->>Adapter: ack
end
rect rgba(150, 100, 200, 0.5)
Note over Client,JsUtils: Read claim via js-utils sync helper
Client->>Adapter: getSessionItem('accessToken')
Adapter->>MemStore: getSessionItem('accessToken')
MemStore-->>Adapter: rawJwt
Adapter-->>Client: rawJwt
Client->>JsUtils: getClaimSync(rawJwt, claimName)
JsUtils-->>Client: claimValue
end
sequenceDiagram
participant Init as SDK Init
participant StorageInit as initStorage
participant Adapter as KindeStorageAdapter
participant JsUtils as `@kinde/js-utils`
participant Client as Kinde Client
rect rgba(100, 200, 150, 0.5)
Init->>StorageInit: import/init
StorageInit->>Adapter: instantiate KindeStorageAdapter
Adapter->>JsUtils: setActiveStorage(adapter)
JsUtils-->>Adapter: active storage set
StorageInit-->>Init: adapter ready
Init->>Client: continue initialization (storage available)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 7
🤖 Fix all issues with AI agents
In `@CHANGES_SUMMARY.md`:
- Around line 191-196: The CHANGES_SUMMARY.md snippet is incorrect: the actual
implementation unconditionally sets storageSettings.keyPrefix to 'kinde_' rather
than using the conditional shown; update the documentation to reflect the real
code by replacing the conditional example with a note or snippet showing the
direct assignment (storageSettings.keyPrefix = 'kinde_') and mention the symbol
storageSettings.keyPrefix and the unconditional assignment so contributors are
not misled.
In `@examples/STORAGE_PREFIX.md`:
- Around line 134-150: The example incorrectly mutates the same global
storageSettings.keyPrefix sequentially which means only the last prefix is
active; update the "Storage Isolation" example to show independent settings by
creating separate settings objects or separate store instances (e.g., create
distinct storageSettingsApp1 and storageSettingsApp2 or instantiate two stores)
and then call store.setItem with those distinct configurations so that app1_key
and app2_key are truly isolated; reference storageSettings.keyPrefix and
store.setItem in the updated instructions.
- Around line 84-96: The example is misleading because storageSettings.keyPrefix
is a global singleton so setting storageSettings.keyPrefix before
createKindeClient will affect all clients; update the doc to either state this
limitation explicitly and warn that storageSettings.keyPrefix is global, or
replace the example with a recommended approach (e.g., use per-client scoping by
prefixing keys at call site or run separate contexts like iframes/workers) and
reference the symbols storageSettings.keyPrefix, createKindeClient, client1 and
client2 so readers know which APIs are affected.
In `@examples/utils-export-example.ts`:
- Around line 36-39: The example shadows the browser global localStorage by
declaring const localStorage = new LocalStorage(); — rename the variable (e.g.,
appStorage or exampleStorage) wherever it’s used (the LocalStorage instantiation
and subsequent calls to setSessionItem and getSessionItem) to avoid colliding
with window.localStorage; update the references to
setSessionItem('user_preference', ...), getSessionItem('user_preference'), and
the console.log accordingly so the example remains clear and non-conflicting.
In `@LOCALSTORAGE_MIGRATION.md`:
- Around line 99-104: The two docs contain mismatched test counts:
LOCALSTORAGE_MIGRATION.md says "All 69 tests passing" while
examples/MIGRATION_TO_JSUTILS.md says "67 tests passing"; verify the actual
current test count by running the test suite and then update the text string
"All 69 tests passing"/"67 tests passing" in both documents to the verified
number so they match; also search the repo for any other occurrences of the
test-count string and update them to the same verified value to keep docs
consistent.
In `@rollup.config.ts`:
- Around line 43-50: Remove the TypeScript declaration generation options from
the utils bundle's Rollup typescript plugin: delete declaration: true and
declarationDir: 'dist' from the typescript({ ... }) call in the plugins array so
only the build pipeline in rollup.types.config.ts produces .d.ts files; keep
other options like rootDir intact.
In `@src/state/store.ts`:
- Line 8: The module currently unconditionally sets storageSettings.keyPrefix =
'kinde_' at import time which overwrites any user-provided value; change this to
a conditional assignment in src/state/store.ts so you only set
storageSettings.keyPrefix to 'kinde_' when storageSettings.keyPrefix is falsy or
equals the legacy 'kinde-' value (e.g., if (!storageSettings.keyPrefix ||
storageSettings.keyPrefix === 'kinde-') storageSettings.keyPrefix = 'kinde_'),
keeping user configuration intact while preserving backward compatibility.
🧹 Nitpick comments (11)
src/state/storageAdapter.ts (1)
22-34: Consider hoistingkeyMapto a module-level or static constant.
mapKeyis called on every get/set/remove operation, and it recreates thekeyMapobject each time. Since it's static data, hoist it out.♻️ Suggested refactor
+const KEY_MAP: Record<string, string> = { + accessToken: 'accessToken', + idToken: 'idToken', + refreshToken: 'kinde_refresh_token', + state: 'kinde_state', + nonce: 'kinde_nonce', + codeVerifier: 'kinde_code_verifier' +}; + export class KindeStorageAdapter<V extends string = StorageKeys> extends SessionBase<V> implements SessionManager<V> { asyncStore = false; private mapKey(itemKey: V | StorageKeys): string { - const keyMap: Record<string, string> = { - accessToken: 'accessToken', - idToken: 'idToken', - refreshToken: 'kinde_refresh_token', - state: 'kinde_state', - nonce: 'kinde_nonce', - codeVerifier: 'kinde_code_verifier' - }; - - return keyMap[itemKey as string] || (itemKey as string); + return KEY_MAP[itemKey as string] || (itemKey as string); }src/kindeUtils.ts (1)
1-7: Wildcard re-export ties this package's public API to@kinde/js-utilssurface.Any new or breaking export added to
@kinde/js-utilswill automatically propagate to consumers of/utils. This is fine if you control both packages and version them together, but worth noting for future semver considerations.src/kindeUtils.test.ts (1)
8-53: Tests validate@kinde/js-utilsdirectly, not the/utilsre-export barrel.Every
import()in this file targets'@kinde/js-utils'rather than'./kindeUtils'(or the built/utilspath). This only confirms that@kinde/js-utilsexposes the expected symbols — it doesn't verify thatsrc/kindeUtils.tscorrectly re-exports them. The companion filesrc/package-exports.test.tsdoes import from'./kindeUtils', which provides the actual coverage.Consider either updating these imports to target
'./kindeUtils'so this suite genuinely tests the barrel, or removing/merging it withpackage-exports.test.tsto avoid duplication.♻️ Example fix for one test case
test('should be able to import storageSettings from /utils', async () => { - const {storageSettings} = await import('@kinde/js-utils'); + const {storageSettings} = await import('./kindeUtils'); expect(storageSettings).toBeDefined(); expect(storageSettings.keyPrefix).toBeDefined(); expect(storageSettings.maxLength).toBeDefined(); });Apply the same change to the other test cases.
src/createKindeClient.ts (2)
110-112:LocalStorageis instantiated even when not needed.
localStorageAdapteris created unconditionally but only used whenisUseLocalStorageis true (lines 173, 187, 515). Consider deferring instantiation or gating it behind the flag to avoid unnecessary work and clarify intent.♻️ Suggested change
- // Use LocalStorage from `@kinde/js-utils` for persistent storage - const localStorageAdapter = new LocalStorage(); + // Use LocalStorage from `@kinde/js-utils` for persistent storage (only when needed) + const localStorageAdapter = isUseLocalStorage ? new LocalStorage() : null;Note: If you do this, the three call-sites (lines 173, 187, 515) should use
localStorageAdapter!.or be guarded by the existingisUseLocalStoragechecks (which they already are).
172-176: Repeatedas anycasts indicate a type mismatch betweenstorageMapandStorageKeys.
storageMap.refresh_tokenis cast toanyat every call-site for theLocalStorageadapter. This bypasses type safety. Consider addingrefresh_tokento theStorageKeysunion or creating a typed constant for it.Also applies to: 186-189
src/utils/getClaimValue/getClaimValue.ts (1)
11-22: Silent error swallowing may mask bugs during development.The
catchblock logs toconsole.errorbut always returnsnull, making it indistinguishable from a genuinely absent claim. Consider re-throwing in development mode or including the claim name and token type in the log message for easier debugging.♻️ Suggested improvement for debuggability
} catch (error) { - console.error('Error getting claim value:', error); + console.error(`Error getting claim "${claim}" from ${tokenKey}:`, error); return null; }examples/storage-prefix-example.ts (1)
11-12: Consider consolidating imports.The default and named exports from the same package can be combined into a single import statement.
♻️ Suggested fix
-import {storageSettings} from '@kinde-oss/kinde-auth-pkce-js'; -import createKindeClient from '@kinde-oss/kinde-auth-pkce-js'; +import createKindeClient, {storageSettings} from '@kinde-oss/kinde-auth-pkce-js';src/state/store.types.ts (1)
13-15: GenericTis effectively erased byT | unknown | null.In TypeScript,
T | unknownsimplifies tounknown, soAwaitable<T | unknown | null>is equivalent toAwaitable<unknown>. The genericTprovides no type narrowing for callers. If the intent is to allow callers to specify a return type, change toAwaitable<T | null>.♻️ Suggested fix
getSessionItem: <T = unknown>( itemKey: string | StorageKeys - ) => Awaitable<T | unknown | null>; + ) => Awaitable<T | null>;src/state/store.ts (3)
39-53:serializeValuehas redundant branches — all paths callJSON.stringify(value).The null/undefined check, the primitive check, and the object fallback all do the same thing. This can be simplified.
♻️ Simplified version
- const serializeValue = (value: unknown): string => { - if (value === null || value === undefined) { - return JSON.stringify(value); - } - // For primitive types, just convert to string - if ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return JSON.stringify(value); - } - // For objects and arrays, use JSON - return JSON.stringify(value); - }; + const serializeValue = (value: unknown): string => JSON.stringify(value);
15-25: Unhandled listener exceptions will silently break subsequent listeners.If any listener throws synchronously,
forEachaborts and remaining listeners are never called. Consider wrapping each invocation in a try-catch.♻️ Suggested fix
queueMicrotask(() => { notificationScheduled = false; listeners.forEach((listener) => { - listener(); + try { + listener(); + } catch (err) { + console.error('Store listener error:', err); + } }); });
122-128:removeItemstriggersnotifyListenersN+1 times.Each
removeSessionItemcall on line 124 already callsnotifyListeners(). The additional call on line 126 is redundant. The queueMicrotask batching prevents actual duplicate work, but the extra call is unnecessary noise.♻️ Suggested fix
const removeItems = (...itemKeys: string[]): Promise<void> => { itemKeys.forEach((key) => { removeSessionItem(key); }); - notifyListeners(); return Promise.resolve(); };
| plugins: [ | ||
| resolve(), | ||
| typescript({ | ||
| declaration: true, | ||
| declarationDir: 'dist', | ||
| rootDir: 'src' | ||
| }) | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name 'rollup.types.config.ts' -o -name 'rollup.types.config.js' | head -5Repository: kinde-oss/kinde-auth-pkce-js
Length of output: 96
🏁 Script executed:
cat -n ./rollup.types.config.tsRepository: kinde-oss/kinde-auth-pkce-js
Length of output: 1179
🏁 Script executed:
cat -n ./tsconfig.jsonRepository: kinde-oss/kinde-auth-pkce-js
Length of output: 625
🏁 Script executed:
cat -n ./rollup.config.ts | head -60Repository: kinde-oss/kinde-auth-pkce-js
Length of output: 1537
Remove declaration: true from the utils bundle typescript plugin.
The utils bundle is generating .d.ts files via declaration: true with declarationDir: 'dist', but rollup.types.config.ts is already responsible for generating the final utils type declarations by reading from TypeScript's dist/types/kindeUtils.d.ts output. This creates redundant declaration generation; remove the declaration options from the typescript plugin in the utils bundle config.
Current utils bundle config (lines 43-50)
plugins: [
resolve(),
typescript({
declaration: true,
declarationDir: 'dist',
rootDir: 'src'
})
]
🤖 Prompt for AI Agents
In `@rollup.config.ts` around lines 43 - 50, Remove the TypeScript declaration
generation options from the utils bundle's Rollup typescript plugin: delete
declaration: true and declarationDir: 'dist' from the typescript({ ... }) call
in the plugins array so only the build pipeline in rollup.types.config.ts
produces .d.ts files; keep other options like rootDir intact.
| type StorageKeys | ||
| } from '@kinde/js-utils'; | ||
|
|
||
| storageSettings.keyPrefix = 'kinde_'; |
There was a problem hiding this comment.
Unconditional prefix assignment overwrites any pre-existing user configuration.
storageSettings.keyPrefix = 'kinde_' runs at module load. If @kinde/js-utils ships with a different default (e.g., 'kinde-'), this correctly overrides it to maintain backward compatibility. However, this also means any prefix configured before this module is imported will be lost.
This is acceptable since users are expected to configure storageSettings after importing the SDK, but note that the CHANGES_SUMMARY.md (lines 193-195) shows a conditional check (if (!storageSettings.keyPrefix || storageSettings.keyPrefix === 'kinde-')) that doesn't match this unconditional assignment.
🤖 Prompt for AI Agents
In `@src/state/store.ts` at line 8, The module currently unconditionally sets
storageSettings.keyPrefix = 'kinde_' at import time which overwrites any
user-provided value; change this to a conditional assignment in
src/state/store.ts so you only set storageSettings.keyPrefix to 'kinde_' when
storageSettings.keyPrefix is falsy or equals the legacy 'kinde-' value (e.g., if
(!storageSettings.keyPrefix || storageSettings.keyPrefix === 'kinde-')
storageSettings.keyPrefix = 'kinde_'), keeping user configuration intact while
preserving backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/createKindeClient.ts (1)
555-561:⚠️ Potential issue | 🟠 MajorActive storage is not restored after portal URL generation.
setActiveStorage(storage)on line 560 switches the global active storage to a temporaryMemoryStoragefor portal token usage, but aftergeneratePortalUrlcompletes, the original storage is never restored. All subsequent@kinde/js-utilscalls will use this empty temporary storage.Restore the previous active storage
+ const previousStorage = getActiveStorage(); const storage = new MemoryStorage(); await storage.setSessionItem( StorageKeys.accessToken, tokens.access_token ); setActiveStorage(storage); const portalUrl = await generatePortalUrl({ ...options, domain: config.domain, returnUrl }); + setActiveStorage(previousStorage); window.location.href = portalUrl.url.toString();src/types.ts (1)
124-124:⚠️ Potential issue | 🟠 Major
portalparameter type is stricter than the implementation.The
KindeClienttype declaresportalas requiringOmit<GeneratePortalUrlParams, 'domain'>(all fields mandatory), but the implementation increateKindeClient.ts(line 540) acceptsPartial<Omit<GeneratePortalUrlParams, 'domain'>>with a default of{}. Callers using theKindeClienttype will be forced to provide allGeneratePortalUrlParamsfields (minusdomain) even though the implementation doesn't require them.Align the type with the implementation
- portal: (options: Omit<GeneratePortalUrlParams, 'domain'>) => Promise<void>; + portal: (options?: Partial<Omit<GeneratePortalUrlParams, 'domain'>>) => Promise<void>;
🤖 Fix all issues with AI agents
In `@src/createKindeClient.ts`:
- Around line 423-454: generateAuthUrl now returns {url, state, codeVerifier}
but we never persist these values to sessionStorage, so handleRedirectToApp
can't find them on return; before redirecting (after calling generateAuthUrl in
createKindeClient), store an object keyed by `${SESSION_PREFIX}-${state}` in
sessionStorage that includes at minimum { codeVerifier, appState } (use the
existing app state variable from the request or options, e.g., options.appState
or reauthState if that was the intended app state), and any other fields
setupChallenge previously saved (e.g., loginHint/orgCode/etc.) so
handleRedirectToApp can read them; reference generateAuthUrl,
handleRedirectToApp, SESSION_PREFIX, state, codeVerifier, and sessionStorage
when making the change.
- Around line 603-606: The redirect URI comparison mistakenly includes a
trailing space in the fallback check (currentRedirectUri ===
`${expectedRedirectUri}/ `), which will never match; update the comparison to
use `${expectedRedirectUri}/` (no space) so the fallback correctly detects a
redirected URL; locate the expression using currentRedirectUri and
expectedRedirectUri in createKindeClient.ts and replace the
`${expectedRedirectUri}/ ` literal with `${expectedRedirectUri}/`.
- Around line 456-462: The Biome lint flags that the callback passed to
Object.entries(...).forEach is returning the result of merged.set(); change the
iteration so the callback does not return a value: replace the forEach with an
explicit for...of loop over Object.entries(authUrlParams) (or call
merged.set(...) and prefix with void to silence the return), keeping the same
behavior that merges authUrlParams into the URLSearchParams instance named
merged and assigns it back to url.search; update the block that references
authUrlParams, merged, and URLSearchParams accordingly.
- Around line 101-108: Remove the debug console.log statements that print
internal state (the "PESICKA" logs) so they don't ship to production: delete the
console.log calls around setActiveStorage/getActiveStorage in createKindeClient
(the logs that print store and getStorage), the other "PESICKA" logs at the
later positions referenced (around line 578 and lines 598–602), and the similar
debug log in src/state/store.ts (around line 170); leave the functional calls
setActiveStorage(...) and getActiveStorage() intact and do not add new console
output—if logging is still required, replace with a proper logger and
appropriate level (debug) instead of console.log.
In `@src/state/initStorage.ts`:
- Around line 12-23: The module auto-initializes and calls
setActiveStorage(adapter) in initializeStorageAdapter which conflicts with other
callers (setActiveStorage(memoryStorage) in src/state/store.ts and
setActiveStorage(store) in src/createKindeClient.ts); remove the module-load
side-effect (the initializeStorageAdapter() call) and ensure
initializeStorageAdapter only returns/creates the adapter (export it for
explicit invocation), or add a single explicit call site where setActiveStorage
is invoked once; update initializeStorageAdapter, KindeStorageAdapter usage, and
any exports (adapter) so the active storage is set deterministically from one
well-known location instead of at module load.
In `@src/state/store.ts`:
- Around line 170-171: The code is setting the raw MemoryStorage as active which
bypasses the store's JSON serialization layer; replace the call to
setActiveStorage(memoryStorage) with setActiveStorage(store) (the wrapper that
performs JSON serialize/deserialize) or defer calling setActiveStorage until
after createKindeClient has been initialized so all writes go through the store
wrapper; specifically update the usage of memoryStorage, setActiveStorage, and
store (and ensure createKindeClient still overrides active storage later) so no
writes can occur against the raw MemoryStorage.
🧹 Nitpick comments (7)
rollup.config.ts (2)
34-56: TheresolveIdguard on line 37 is redundant — Rollup always passes a stringid.Minor: the
typeof id === 'string'check is unnecessary sinceresolveId(id)always receives a string from Rollup. More importantly, the second branch (id.includes('expo-secure-store')) already covers the first (id === 'expo-secure-store'), so the equality check is also redundant.Simplify resolveId
resolveId(id) { - if ( - id === 'expo-secure-store' || - (typeof id === 'string' && id.includes('expo-secure-store')) - ) { + if (id.includes('expo-secure-store')) { return {id: EXPO_STUB_ID, moduleSideEffects: false}; } return null; },
106-106:external: ['expo-secure-store']conflicts with the stub plugin.Marking
expo-secure-storeas external tells Rollup to leave imports of it unresolved in the output, but thestubExpoSecureStoreplugin (withorder: 'pre') intercepts and resolves those imports to a stub. Since the plugin fires first, this works in practice, but theexternaldeclaration is misleading and could cause issues if plugin ordering changes. Consider removing it.Remove conflicting external declaration
- external: ['expo-secure-store'],src/createKindeClient.ts (1)
120-124:isUseLocalStorageis hardcoded tofalse, makinglocalStorageAdapterdead code.The
LocalStorageadapter is instantiated on everycreateKindeClientcall but never used becauseisUseLocalStorageis alwaysfalse. This adds an unnecessary allocation and confuses readers. If this is WIP, consider gating the instantiation behind the flag or removing it until the feature is ready.Defer instantiation until the feature is enabled
- const isUseLocalStorage = false; - //isDevelopment || is_dangerously_use_local_storage; - - // Use LocalStorage from `@kinde/js-utils` for persistent storage - const localStorageAdapter = new LocalStorage(); + const isUseLocalStorage = false; + //isDevelopment || is_dangerously_use_local_storage; + + // Use LocalStorage from `@kinde/js-utils` for persistent storage + const localStorageAdapter = isUseLocalStorage ? new LocalStorage() : null;src/state/store.ts (2)
40-54:serializeValuehas unnecessary branching — every path callsJSON.stringify.All three branches (null/undefined, primitives, objects) end with
JSON.stringify(value). The function can be simplified to a singleJSON.stringifycall.JSON.stringifyalready handlesnull,undefined, primitives, objects, and arrays correctly.Simplify serializeValue
- const serializeValue = (value: unknown): string => { - if (value === null || value === undefined) { - return JSON.stringify(value); - } - // For primitive types, just convert to string - if ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return JSON.stringify(value); - } - // For objects and arrays, use JSON - return JSON.stringify(value); - }; + const serializeValue = (value: unknown): string => JSON.stringify(value);
123-129:removeItemstriggers redundant listener notifications.Each
removeSessionItemcall on line 125 already callsnotifyListeners(), so the additionalnotifyListeners()on line 127 is superfluous. While the microtask batching prevents duplicate dispatches, the intent is unclear. Consider removing the extra call for clarity.Remove redundant notifyListeners call
const removeItems = (...itemKeys: string[]): Promise<void> => { itemKeys.forEach((key) => { removeSessionItem(key); }); - notifyListeners(); return Promise.resolve(); };src/index.ts (1)
4-4: Wildcard re-export of@kinde/js-utilsexpands the public API surface implicitly.
export * from './kindeUtils'forwards every current and future export of@kinde/js-utilsas part of this package's public API. Any new export added upstream could collide with./typesexports (line 5) or with consumer code. Consider using explicit named re-exports to keep the API surface intentional and stable.src/types.ts (1)
112-112: Prefer a top-level import forPromptTypesinstead of an inlineimport()type.
PromptTypesis used inline asimport('@kinde/js-utils').PromptTypes, but@kinde/js-utilsis already imported at line 1. ImportPromptTypesalongsideGeneratePortalUrlParamsandLoginMethodParamsfor consistency.Move to top-level import
-import {GeneratePortalUrlParams, type LoginMethodParams} from '@kinde/js-utils'; +import {GeneratePortalUrlParams, type LoginMethodParams, type PromptTypes} from '@kinde/js-utils';Then on line 112:
- prompt?: string | import('@kinde/js-utils').PromptTypes; + prompt?: string | PromptTypes;
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/createKindeClient.ts`:
- Around line 550-554: The current token check incorrectly calls the async
getToken() without awaiting, so tokens becomes a Promise and the null-check is
bypassed; update the code around the tokens variable in createKindeClient (where
getToken() is invoked) to await the call (const tokens = await getToken();) and
handle potential rejection/errors accordingly so the existing guard (throw new
Error('No valid access token found')) can run when no token is returned.
- Around line 176-180: Replace the unsafe casts of storageMap.refresh_token as
any with the proper enum/key type: use StorageKeys.refreshToken when calling
localStorageAdapter.setSessionItem, getSessionItem, and removeSessionItem (these
calls live around isUseLocalStorage and where localStorageAdapter is used).
Update the three locations that currently cast storageMap.refresh_token to pass
StorageKeys.refreshToken instead so the KindeStorageAdapter receives the correct
typed key (matching how StorageKeys.accessToken and StorageKeys.idToken are
already used).
- Around line 161-164: Replace the raw string keys used in the
store.setSessionItem calls with the StorageKeys enum to match the rest of the
file: change the calls using 'accessToken' and 'idToken' to use
StorageKeys.accessToken and StorageKeys.idToken respectively (the calls to
store.setSessionItem are the targets to update).
- Around line 270-287: The isAuthenticated function uses the in-memory store
(store) so isAuthenticatedFromJsUtils cannot see refresh tokens saved in
localStorageAdapter when isUseLocalStorage is true; update isAuthenticated to
pass the correct storage to isAuthenticatedFromJsUtils (or switch the active
store) based on isUseLocalStorage: if isUseLocalStorage is true, ensure you read
tokens from localStorageAdapter (mirroring the earlier localStorageAdapter
usage) and call isAuthenticatedFromJsUtils with that storage (or set the active
store to localStorageAdapter) so the refresh_token saved under
storageMap.token_bundle is accessible; reference isAuthenticated, store,
localStorageAdapter, isUseLocalStorage, storageMap.token_bundle, and
isAuthenticatedFromJsUtils when making the change.
In `@src/state/store.ts`:
- Around line 20-25: The microtask currently calls listeners with
listeners.forEach(listener => listener()) which ignores returned Promises and
allows unhandled rejections; update the queueMicrotask callback to invoke all
listeners via Promise.allSettled on listeners.map(l => l()) (or otherwise await
each and handle errors) so async listeners are awaited and their rejections are
caught/handled (log or swallow) before setting notificationScheduled to false;
reference the queueMicrotask invocation and the listeners
array/notificationScheduled variable when making the change.
- Around line 170-171: The direct call setActiveStorage(memoryStorage) is
unnecessary and should be removed or replaced; either delete that line (since
initStorage.ts and createKindeClient.ts override it and store already wraps
memoryStorage with serializeValue/deserializeValue) or, if an initialization to
the serialized store is required, change it to setActiveStorage(store) so the
active storage uses the wrapped/serialized methods; check KindeStorageAdapter
and the store/storageSettings exports to confirm behavior after the change.
🧹 Nitpick comments (2)
src/state/store.ts (2)
40-54:serializeValuehas three branches that all do the same thing.Every code path calls
JSON.stringify(value). The conditional structure and comments suggest differentiated handling, but the implementation is identical throughout. Simplify to a singlereturn JSON.stringify(value).♻️ Proposed simplification
- const serializeValue = (value: unknown): string => { - if (value === null || value === undefined) { - return JSON.stringify(value); - } - // For primitive types, just convert to string - if ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return JSON.stringify(value); - } - // For objects and arrays, use JSON - return JSON.stringify(value); - }; + const serializeValue = (value: unknown): string => JSON.stringify(value);
123-129: RedundantnotifyListeners()call inremoveItems.Each
removeSessionItemcall on Line 125 already invokesnotifyListeners(). The explicit call on Line 127 is a no-op thanks to microtask batching but is misleading — it suggests the per-item calls don't notify. Consider removing it for clarity, or conversely, usememoryStorage.removeSessionItemdirectly inside the loop and callnotifyListeners()once at the end to make the batching intent explicit.
| const isAuthenticated = async () => { | ||
| const accessToken = store.getItem(storageMap.access_token); | ||
| if (!accessToken) { | ||
| return false; | ||
| } | ||
|
|
||
| const isTokenActive = isJWTActive(accessToken as JWT); | ||
| if (isTokenActive) { | ||
| return true; | ||
| // Backwards compatibility: ensure js-utils sees token from legacy keys | ||
| const sessionAccessToken = store.getSessionItem(StorageKeys.accessToken); | ||
| if (!sessionAccessToken) { | ||
| const tokenBundle = store.getItem( | ||
| storageMap.token_bundle | ||
| ) as KindeState | null; | ||
| if (tokenBundle?.access_token) { | ||
| store.setSessionItem(StorageKeys.accessToken, tokenBundle.access_token); | ||
| } | ||
| } | ||
|
|
||
| await useRefreshToken(); | ||
| return true; | ||
| return isAuthenticatedFromJsUtils({ | ||
| useRefreshToken: true, | ||
| domain, | ||
| clientId: client_id | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/createKindeClient.ts | head -200Repository: kinde-oss/kinde-auth-pkce-js
Length of output: 5498
🏁 Script executed:
rg -n "isAuthenticatedFromJsUtils" -A 10 -B 5 src/Repository: kinde-oss/kinde-auth-pkce-js
Length of output: 1779
🏁 Script executed:
rg -n "localStorageAdapter\|getActiveStorage" -A 5 -B 5 src/Repository: kinde-oss/kinde-auth-pkce-js
Length of output: 54
🌐 Web query:
@kinde/js-utils isAuthenticatedFromJsUtils refresh token implementation
💡 Result:
isAuthenticatedFromJsUtils (as used by Kinde’s JS/TS SDKs) is implemented as: “valid access token in storage → authenticated; otherwise try refresh-token grant → authenticated if refresh succeeds; else false.” This matches Kinde’s documented isAuthenticated behavior (“if the token is expired it first attempts to refresh first, if the refresh fails false is returned”) and how Kinde refresh tokens work (refresh via /oauth2/token using grant_type=refresh_token, with refresh-token rotation). [1], [2]
What the refresh step does (effectively)
- Read tokens from the configured storage (js-utils provides the storage abstraction and token helpers). [3]
- If the access token exists and is unexpired → return
true. - If access token is missing/expired:
- Read the refresh token from storage.
- Call Kinde’s token endpoint with the refresh token (
grant_type=refresh_token). - If successful: store the new
access_tokenand the newly rotatedrefresh_token, then returntrue. [2] - If refresh fails (no refresh token / revoked / expired / network) → return
false. [1], [2]
Where the refresh token comes from (frontend nuance)
- Kinde only issues refresh tokens when
offlinescope is requested. [2] - Frontend SDKs typically keep tokens in memory; to survive page refresh/new tab they recommend using a custom domain + httpOnly cookie for the refresh token, or (dev-only) an “insecure localStorage” escape hatch. [4]
- If you’re mixing SDKs in local-dev, note there can be different localStorage key names (e.g., some setups expect
kinde_refresh_tokenwhile others userefreshToken0). [5]
Sources:
[1] @kinde-oss/kinde-typescript-sdk docs snippet describing isAuthenticated refresh-then-false behavior. (npmjs.com)
[2] Kinde refresh token docs (refresh grant, /oauth2/token, rotation). (docs.kinde.com)
[3] kinde-oss/js-utils repo README on storage + token helpers. (github.com)
[4] Kinde JavaScript SDK docs on persistence + is_dangerously_use_local_storage. (docs.kinde.com)
[5] AnswerOverflow thread noting differing refresh-token localStorage keys between SDKs. (answeroverflow.com)
Citations:
- 1: https://www.npmjs.com/package/%40kinde-oss/kinde-typescript-sdk/v/1.0.3?utm_source=openai
- 2: https://docs.kinde.com/build/tokens/refresh-tokens/?utm_source=openai
- 3: https://github.com/kinde-oss/js-utils
- 4: https://docs.kinde.com/developer-tools/sdks/frontend/javascript-sdk/?utm_source=openai
- 5: https://www.answeroverflow.com/m/1395250156131323944?utm_source=openai
isAuthenticatedFromJsUtils cannot find the refresh token when isUseLocalStorage is enabled.
The active storage is set to store (in-memory) on line 100 and never changed. However, when isUseLocalStorage is true, the refresh token is stored in a separate localStorageAdapter instance (line 172 and 188). Since isAuthenticatedFromJsUtils receives no storage parameter and uses only the configured active storage, it cannot access the refresh token from localStorageAdapter. When the access token expires, the refresh attempt fails because the refresh token is not found in the active storage, causing isAuthenticatedFromJsUtils to return false even for authenticated users.
🤖 Prompt for AI Agents
In `@src/createKindeClient.ts` around lines 270 - 287, The isAuthenticated
function uses the in-memory store (store) so isAuthenticatedFromJsUtils cannot
see refresh tokens saved in localStorageAdapter when isUseLocalStorage is true;
update isAuthenticated to pass the correct storage to isAuthenticatedFromJsUtils
(or switch the active store) based on isUseLocalStorage: if isUseLocalStorage is
true, ensure you read tokens from localStorageAdapter (mirroring the earlier
localStorageAdapter usage) and call isAuthenticatedFromJsUtils with that storage
(or set the active store to localStorageAdapter) so the refresh_token saved
under storageMap.token_bundle is accessible; reference isAuthenticated, store,
localStorageAdapter, isUseLocalStorage, storageMap.token_bundle, and
isAuthenticatedFromJsUtils when making the change.
There was a problem hiding this comment.
@DanielRivers Could you check on this one? I implemented this change, but I'm not sure if this is 100%. I wasn't ever testing refresh tokens when I was going through this.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@DanielRivers I've checked this and it is going to be an issue. Either we can update js-utils to check the localStorageAdapter, or we need to update it here so that it's not storing it in a completely different storage method.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/createKindeClient.ts (1)
114-115:localStorageAdapteris always instantiated regardless ofisUseLocalStorage.By line 115,
isUseLocalStorageis already determined (line 112). Creating theLocalStorageinstance unconditionally is wasteful when it will never be used.♻️ Conditional instantiation
- // Use LocalStorage from `@kinde/js-utils` for persistent storage - const localStorageAdapter = new LocalStorage(); + const localStorageAdapter = isUseLocalStorage ? new LocalStorage() : null;All downstream usages are already gated on
isUseLocalStorage, so this is safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/createKindeClient.ts` around lines 114 - 115, localStorageAdapter is being instantiated unconditionally via new LocalStorage() even when isUseLocalStorage is false; defer construction until needed by moving the new LocalStorage() call behind the isUseLocalStorage check (or lazily instantiate when first used) so that LocalStorage is only created when isUseLocalStorage is true—update references to localStorageAdapter accordingly (e.g., in createKindeClient where isUseLocalStorage is computed) to ensure downstream usages remain gated by isUseLocalStorage.src/state/store.ts (1)
37-53:serializeValuehas three identical branches and anundefined-serialization edge case.All three branches unconditionally call
JSON.stringify(value), making theif/elsedead code. More critically,JSON.stringify(undefined)returnsundefined(not a string), soserializeValue(undefined)silently violates the declaredstringreturn type and would passundefinedintomemoryStorage.setSessionItem.♻️ Simplified form with guard
- const serializeValue = (value: unknown): string => { - if (value === null || value === undefined) { - return JSON.stringify(value); - } - // For primitive types, just convert to string - if ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' - ) { - return JSON.stringify(value); - } - // For objects and arrays, use JSON - return JSON.stringify(value); - }; + const serializeValue = (value: unknown): string => { + if (value === undefined) return 'null'; + return JSON.stringify(value); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/state/store.ts` around lines 37 - 53, serializeValue currently has redundant branches and calls JSON.stringify(undefined) which returns undefined (not a string), risking passing undefined into memoryStorage.setSessionItem; simplify the function to a single return that guarantees a string by converting undefined to null before stringifying. Replace the body of serializeValue with a single expression like JSON.stringify(value === undefined ? null : value) so it always returns a JSON string for primitives, objects, null, and undefined (as "null") and keep callers such as memoryStorage.setSessionItem unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/createKindeClient.ts`:
- Around line 176-180: Replace the unsafe cast "storageMap.refresh_token as any"
with the strongly-typed enum key "StorageKeys.refreshToken" wherever used (the
callers of localStorageAdapter.setSessionItem and similar calls that reference
storageMap.refresh_token). Locate uses around the
localStorageAdapter.setSessionItem calls and the other two occurrences and swap
the key to StorageKeys.refreshToken; ensure the StorageKeys symbol is
imported/available and that the calls target the
KindeStorageAdapter/LocalStorage API which expects StorageKeys.
- Around line 451-465: generateAuthUrl returns url, state and codeVerifier but
they are not saved to sessionStorage before redirect, so handleRedirectToApp
later cannot validate state or perform the PKCE exchange; persist state and
codeVerifier under keys using SESSION_PREFIX and the state (matching how
handleRedirectToApp reads `${SESSION_PREFIX}-${state}`) before setting
window.location.href. Also ensure the app_state (with kindeOriginUrl) is added
to loginOptions or saved to sessionStorage so on_redirect_callback(user,
appState) receives the value; update the code around
generateAuthUrl/loginOptions and window.location.href to write these values to
sessionStorage prior to redirect.
- Around line 270-287: The isAuthenticated function only backfills
StorageKeys.accessToken into the in-memory store but not the refresh token, so
isAuthenticatedFromJsUtils({ useRefreshToken: true, ... }) cannot find a refresh
token when isUseLocalStorage is true; update the backfill logic inside
isAuthenticated to also check tokenBundle?.refresh_token and call
store.setSessionItem(StorageKeys.refreshToken, tokenBundle.refresh_token) (using
the same storageMap.token_bundle lookup and tokenBundle variable) so the
in-memory adapter has both access and refresh tokens before calling
isAuthenticatedFromJsUtils.
- Around line 457-463: The Biome lint error is caused by the
Object.entries(...).forEach callback implicitly returning the value of
merged.set(), so update the loop in createKindeClient.ts to avoid returning a
value from the callback; for example, replace the forEach with an explicit
for..of over Object.entries(authUrlParams) (using the variables merged and
authUrlParams) and call merged.set(key, value) inside the loop without returning
anything.
In `@src/state/store.ts`:
- Line 4: The module unconditionally overwrites storageSettings.keyPrefix at
import time; change this to only set a default when a consumer hasn't already
configured one by checking storageSettings.keyPrefix (or null/undefined/empty)
before assignment so existing consumer-provided prefixes are preserved; update
the assignment site (the line setting storageSettings.keyPrefix) to perform a
conditional/defaulting assignment and add a brief comment explaining this
behavior.
---
Nitpick comments:
In `@src/createKindeClient.ts`:
- Around line 114-115: localStorageAdapter is being instantiated unconditionally
via new LocalStorage() even when isUseLocalStorage is false; defer construction
until needed by moving the new LocalStorage() call behind the isUseLocalStorage
check (or lazily instantiate when first used) so that LocalStorage is only
created when isUseLocalStorage is true—update references to localStorageAdapter
accordingly (e.g., in createKindeClient where isUseLocalStorage is computed) to
ensure downstream usages remain gated by isUseLocalStorage.
In `@src/state/store.ts`:
- Around line 37-53: serializeValue currently has redundant branches and calls
JSON.stringify(undefined) which returns undefined (not a string), risking
passing undefined into memoryStorage.setSessionItem; simplify the function to a
single return that guarantees a string by converting undefined to null before
stringifying. Replace the body of serializeValue with a single expression like
JSON.stringify(value === undefined ? null : value) so it always returns a JSON
string for primitives, objects, null, and undefined (as "null") and keep callers
such as memoryStorage.setSessionItem unchanged.
dtoxvanilla1991
left a comment
dtoxvanilla1991
left a comment
There was a problem hiding this comment.
🔬 done. Left some minor comments.
|
|
||
| if (isUseLocalStorage) { | ||
| localStorage.setItem(storageMap.refresh_token, data.refresh_token); | ||
| localStorageAdapter.setSessionItem( |
There was a problem hiding this comment.
Potential compatibility risk: using storageMap.refresh_token (kinde_refresh_token) with LocalStorage session APIs that also apply prefixing can fork key namespaces across upgrade paths. This can make previously persisted refresh tokens unreadable in localStorage mode. Recommend using one canonical logical key model with explicit legacy read-fallback/migration.
| ) => { | ||
| const localStorageRefreshToken = isUseLocalStorage | ||
| ? (localStorage.getItem(storageMap.refresh_token) as string) | ||
| ? (localStorageAdapter.getSessionItem( |
There was a problem hiding this comment.
Same key-model issue here on read path: refresh token lookup may miss tokens written under a different prefixed/legacy key shape. Please ensure read logic checks legacy key and canonical key consistently, then migrates forward to a single canonical storage key.
|
|
||
| if (isUseLocalStorage) { | ||
| localStorage.removeItem(storageMap.refresh_token); | ||
| localStorageAdapter.removeSessionItem(storageMap.refresh_token as any); |
There was a problem hiding this comment.
Deletion path should align with the same canonical key strategy as set/get. If write/read can span legacy and prefixed keys, logout should clear both legacy and canonical keys to avoid stale sessions and inconsistent auth state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/createKindeClient.ts (3)
458-462:⚠️ Potential issue | 🟡 MinorReplace
forEachcallback withfor...ofto satisfy Biome lint.The callback currently returns
merged.set(...), which tripslint/suspicious/useIterableCallbackReturn.♻️ Proposed fix
- Object.entries(authUrlParams as Record<string, string>).forEach( - ([key, value]) => merged.set(key, value) - ); + for (const [key, value] of Object.entries( + authUrlParams as Record<string, string> + )) { + merged.set(key, value); + }#!/bin/bash # Verify remaining forEach callbacks that implicitly return expressions rg -n "Object\.entries\(.+\)\.forEach\(" src/createKindeClient.ts -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/createKindeClient.ts` around lines 458 - 462, The forEach callback that sets URLSearchParams must be changed to a for...of loop to avoid the Biome lint rule "lint/suspicious/useIterableCallbackReturn"; in the block where authUrlParams is handled (check authUrlParams, merged = new URLSearchParams(url.search)), replace Object.entries(authUrlParams).forEach(([key, value]) => merged.set(key, value)) with an explicit for (const [key, value] of Object.entries(authUrlParams)) { merged.set(key, value) } so the callback return is removed and the merged URLSearchParams is updated correctly.
177-181:⚠️ Potential issue | 🟠 MajorUse
StorageKeys.refreshTokeninstead ofstorageMap.refresh_token as anyacross set/get/remove.The
as anycasts weaken key safety and can split refresh-token namespaces across storage paths.♻️ Proposed fix
- localStorageAdapter.setSessionItem( - storageMap.refresh_token as any, - data.refresh_token - ); + localStorageAdapter.setSessionItem( + StorageKeys.refreshToken, + data.refresh_token + ); ... - ? (localStorageAdapter.getSessionItem( - storageMap.refresh_token as any - ) as string) + ? (localStorageAdapter.getSessionItem( + StorageKeys.refreshToken + ) as string) ... - localStorageAdapter.removeSessionItem(storageMap.refresh_token as any); + localStorageAdapter.removeSessionItem(StorageKeys.refreshToken);#!/bin/bash # Verify all refresh-token key usages and unsafe casts rg -n "refresh_token as any|StorageKeys\.refreshToken|setSessionItem\(|getSessionItem\(|removeSessionItem\(" src/createKindeClient.ts -C2 rg -n "StorageKeys" --type=ts -C3Also applies to: 192-194, 529-529
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/createKindeClient.ts` around lines 177 - 181, Replace the unsafe casted key usage storageMap.refresh_token as any with the canonical StorageKeys.refreshToken constant in all localStorageAdapter calls (setSessionItem, getSessionItem, removeSessionItem) so the refresh token uses a single, type-safe key; update occurrences around the set in createKindeClient (the block using localStorageAdapter.setSessionItem), and the corresponding get/remove usages referenced in the review (lines near 192–194 and 529) to pass StorageKeys.refreshToken instead of the casted storageMap value.
101-101:⚠️ Potential issue | 🔴 Critical
isAuthenticatedFromJsUtilsmay miss refresh tokens in local-storage mode.Active storage is fixed to
store, while refresh token writes can go tolocalStorageAdapter; refresh-based auth checks can fail after access-token expiry.🐛 Proposed fix
- const client_id = clientId || 'spa@live'; - setActiveStorage(store); + const client_id = clientId || 'spa@live'; ... const isUseLocalStorage = isDevelopment || is_dangerously_use_local_storage; // Use LocalStorage from `@kinde/js-utils` for persistent storage const localStorageAdapter = new LocalStorage(); + const activeStorage = isUseLocalStorage ? localStorageAdapter : store; + setActiveStorage(activeStorage);- const sessionAccessToken = store.getSessionItem(StorageKeys.accessToken); + const sessionAccessToken = activeStorage.getSessionItem(StorageKeys.accessToken); if (!sessionAccessToken) { const tokenBundle = store.getItem( storageMap.token_bundle ) as KindeState | null; if (tokenBundle?.access_token) { - store.setSessionItem(StorageKeys.accessToken, tokenBundle.access_token); + activeStorage.setSessionItem( + StorageKeys.accessToken, + tokenBundle.access_token + ); } }#!/bin/bash # Verify storage selection and js-utils auth call path rg -n "setActiveStorage|isAuthenticatedFromJsUtils|new LocalStorage|isUseLocalStorage|refresh_token" src/createKindeClient.ts -C3Also applies to: 271-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/createKindeClient.ts` at line 101, The call to setActiveStorage(store) fixes active storage to the in-memory store while refresh tokens may be written to localStorageAdapter, causing isAuthenticatedFromJsUtils to miss refresh tokens after access-token expiry; update the logic around setActiveStorage and calls to isAuthenticatedFromJsUtils so that when local storage mode is enabled (e.g., isUseLocalStorage / new LocalStorage / localStorageAdapter) you call setActiveStorage(localStorageAdapter) before invoking isAuthenticatedFromJsUtils (and revert to store otherwise), and apply the same change in the other occurrence handling refresh-based auth to ensure the active storage matches where refresh tokens are persisted.
🧹 Nitpick comments (2)
src/utils/isTokenValid/isIDTokenValid.test.ts (1)
129-133: ReuseunexpiredPayload()here to avoid duplicated expiry setup.Line 132 reimplements the same future-
explogic already captured by the helper. Reusing it keeps tests consistent.Refactor suggestion
- payload: { - ...idTokenStub, - aud: ['https://account.acme.com', '123456789'], - exp: Math.floor(Date.now() / 1000) + 3600 - } + payload: { + ...unexpiredPayload(), + aud: ['https://account.acme.com', '123456789'] + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/isTokenValid/isIDTokenValid.test.ts` around lines 129 - 133, The test duplicates expiry setup by manually setting exp on the payload; replace the inline exp construction with a call to the existing helper unexpiredPayload() to build the payload (merging or spreading idTokenStub as needed). Locate the failing test where payload: { ...idTokenStub, aud: [...], exp: ... } is used and change it to payload: { ...unexpiredPayload(idTokenStub), aud: [...] } or spread unexpiredPayload() into the payload so the helper supplies the future exp consistently across tests.src/utils/isJWTActive/isJWTActive.test.ts (1)
22-22: Minor: inconsistent verb tense in test description.Line 17 uses "returns false" while line 22 uses "return true". Consider aligning for consistency.
✏️ Suggested fix
- test('return true if provided token is not expired', () => { + test('returns true if provided token is not expired', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/isJWTActive/isJWTActive.test.ts` at line 22, Update the inconsistent test description tense in src/utils/isJWTActive/isJWTActive.test.ts by changing the test name string "return true if provided token is not expired" to match the other tests' tense (e.g., "returns true if provided token is not expired"); ensure the test for the non-expired token and the existing "returns false" test use the same "returns ..." wording so descriptions are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/createKindeClient.ts`:
- Around line 503-517: getUserProfile currently stores a mapped snake_case user
into store.setItem but returns the original camelCase object from
getUserProfileFromJsUtils cast to KindeUser, causing a shape mismatch; update
getUserProfile to construct a new object matching the KindeUser shape (use keys
like id, given_name, family_name, email, picture) from the camelCase user
returned by getUserProfileFromJsUtils, use that same mapped object when calling
store.setItem(storageMap.user, ...), and return that mapped KindeUser object
instead of casting the original; keep references to getUserProfile,
getUserProfileFromJsUtils, storageMap.user and KindeUser when making the change.
---
Duplicate comments:
In `@src/createKindeClient.ts`:
- Around line 458-462: The forEach callback that sets URLSearchParams must be
changed to a for...of loop to avoid the Biome lint rule
"lint/suspicious/useIterableCallbackReturn"; in the block where authUrlParams is
handled (check authUrlParams, merged = new URLSearchParams(url.search)), replace
Object.entries(authUrlParams).forEach(([key, value]) => merged.set(key, value))
with an explicit for (const [key, value] of Object.entries(authUrlParams)) {
merged.set(key, value) } so the callback return is removed and the merged
URLSearchParams is updated correctly.
- Around line 177-181: Replace the unsafe casted key usage
storageMap.refresh_token as any with the canonical StorageKeys.refreshToken
constant in all localStorageAdapter calls (setSessionItem, getSessionItem,
removeSessionItem) so the refresh token uses a single, type-safe key; update
occurrences around the set in createKindeClient (the block using
localStorageAdapter.setSessionItem), and the corresponding get/remove usages
referenced in the review (lines near 192–194 and 529) to pass
StorageKeys.refreshToken instead of the casted storageMap value.
- Line 101: The call to setActiveStorage(store) fixes active storage to the
in-memory store while refresh tokens may be written to localStorageAdapter,
causing isAuthenticatedFromJsUtils to miss refresh tokens after access-token
expiry; update the logic around setActiveStorage and calls to
isAuthenticatedFromJsUtils so that when local storage mode is enabled (e.g.,
isUseLocalStorage / new LocalStorage / localStorageAdapter) you call
setActiveStorage(localStorageAdapter) before invoking isAuthenticatedFromJsUtils
(and revert to store otherwise), and apply the same change in the other
occurrence handling refresh-based auth to ensure the active storage matches
where refresh tokens are persisted.
---
Nitpick comments:
In `@src/utils/isJWTActive/isJWTActive.test.ts`:
- Line 22: Update the inconsistent test description tense in
src/utils/isJWTActive/isJWTActive.test.ts by changing the test name string
"return true if provided token is not expired" to match the other tests' tense
(e.g., "returns true if provided token is not expired"); ensure the test for the
non-expired token and the existing "returns false" test use the same "returns
..." wording so descriptions are consistent.
In `@src/utils/isTokenValid/isIDTokenValid.test.ts`:
- Around line 129-133: The test duplicates expiry setup by manually setting exp
on the payload; replace the inline exp construction with a call to the existing
helper unexpiredPayload() to build the payload (merging or spreading idTokenStub
as needed). Locate the failing test where payload: { ...idTokenStub, aud: [...],
exp: ... } is used and change it to payload: { ...unexpiredPayload(idTokenStub),
aud: [...] } or spread unexpiredPayload() into the payload so the helper
supplies the future exp consistently across tests.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/createKindeClient.tssrc/utils/isJWTActive/isJWTActive.test.tssrc/utils/isTokenValid/isAccessTokenValid.test.tssrc/utils/isTokenValid/isIDTokenValid.test.ts
5 participants
Explain your changes
Migrates the store to use js-utils store and exports all the kinde utils methods on a
/utilspathChecklist
🛟 If you need help, consider asking for advice over in the Kinde community.