Conversation
WalkthroughAdds a Svelte adapter and runtime for OIDC (core API, DI factory, context, provider components, hooks), extends build/packaging to emit Svelte artifacts and exports, and introduces two full Svelte example apps (single- and multi-provider) with lazy OIDC-gated boot, routing, UI components, and env samples. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant E as /src/main.ts
participant O as oidcEarlyInit
participant L as main.lazy (dynamic)
participant A as App (mounted)
B->>E: load /src/main.ts
E->>O: oidcEarlyInit({ freeze fetch/xhr/ws })
O-->>E: { shouldLoadApp }
alt shouldLoadApp == true
E->>L: dynamic import("./main.lazy")
L->>A: mount(App, target=#app)
A-->>B: UI rendered
else
E-->>B: app boot withheld (await auth)
end
sequenceDiagram
autonumber
participant U as User
participant H as Header
participant R as Router
participant C as OIDC API (enforceLogin)
participant IdP as Identity Provider
U->>H: click "Protected"
H->>R: navigate "/protected"
R->>C: pre-hook → enforceLogin(redirect)
alt user logged in
C-->>R: allow navigation
else not logged in
C->>IdP: redirect to auth endpoint
IdP-->>C: redirect back (code/token)
C-->>R: auth success → allow
end
R-->>U: ProtectedPage rendered
sequenceDiagram
autonumber
participant U as User
participant N as NotLoggedInAuthButton
participant D as SelectProviderDialog
participant G as Google OIDC
participant M as Microsoft OIDC
participant IdP as Chosen Provider
U->>N: Click "Login"
N->>D: open selection dialog
D-->>N: provider selected
alt google
N->>G: login({ doesCurrentHrefRequiresAuth: false })
G->>IdP: browser redirect to Google
else microsoft
N->>M: login({ doesCurrentHrefRequiresAuth: false })
M->>IdP: browser redirect to Microsoft
else cancel
N-->>U: no action
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 9
🧹 Nitpick comments (3)
examples/svelte/src/Fallback.svelte (1)
1-3: Consider removing the empty script block.The empty TypeScript script block serves no purpose and can be safely removed to reduce clutter.
Apply this diff:
-<script lang="ts"> - -</script> - <p>Loading...</p>src/svelte/OidcProviderProps.ts (1)
15-16: Consider the fragility of coupling to Svelte's mount signature.Extracting types from
Parameters<typeof mount>is valid but tightly couples these types to Svelte's internalmountfunction signature. If Svelte changes the mount API in a future version, this will break.Consider adding explicit type definitions or a comment documenting the assumed mount signature:
+// Assumes Svelte mount signature: mount(component: Component, options: { target: Element, props?: Record<string, any> }) export type OidcProviderProps< DecodedIdToken extends Record<string, unknown>, AutoLogin extends boolean > = { App: Parameters<typeof mount>[0]; appProps: Parameters<typeof mount>[1]["props"];src/svelte/oidc.context.ts (1)
12-14: Consider adding error handling for missing OIDC context.If
getOidcContextis called outside of an OIDC provider tree, it will returnundefined. This could lead to cryptic errors in consuming components.Consider validating that the context exists:
export const getOidcContext = <DecodedIdToken extends Record<string, unknown>>() => { - return getContext<{ oidc: Oidc<DecodedIdToken> }>(oidcContextKey); + const context = getContext<{ oidc: Oidc<DecodedIdToken> }>(oidcContextKey); + if (!context) { + throw new Error("getOidcContext must be called within an OIDC provider component"); + } + return context; };This provides a clearer error message and fails fast, making debugging easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
examples/svelte/src/assets/svelte.svgis excluded by!**/*.svgexamples/svelte/src/assets/vite.svgis excluded by!**/*.svgexamples/svelte/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
examples/svelte/.env.local.sample(1 hunks)examples/svelte/.gitignore(1 hunks)examples/svelte/README.md(1 hunks)examples/svelte/index.html(1 hunks)examples/svelte/package.json(1 hunks)examples/svelte/src/App.svelte(1 hunks)examples/svelte/src/Fallback.svelte(1 hunks)examples/svelte/src/app.css(1 hunks)examples/svelte/src/components/AutoLogoutWarningOverlay.svelte(1 hunks)examples/svelte/src/components/Header.svelte(1 hunks)examples/svelte/src/components/LoggedInAuthButton.svelte(1 hunks)examples/svelte/src/components/NotLoggedInAuthButton.svelte(1 hunks)examples/svelte/src/env.d.ts(1 hunks)examples/svelte/src/lib/Counter.svelte(1 hunks)examples/svelte/src/main.lazy.ts(1 hunks)examples/svelte/src/main.ts(1 hunks)examples/svelte/src/oidc.ts(1 hunks)examples/svelte/src/pages/ProtectedPage.svelte(1 hunks)examples/svelte/src/pages/PublicPage.svelte(1 hunks)examples/svelte/src/routes.ts(1 hunks)examples/svelte/src/vite-env.d.ts(1 hunks)examples/svelte/svelte.config.js(1 hunks)examples/svelte/tsconfig.app.json(1 hunks)examples/svelte/tsconfig.json(1 hunks)examples/svelte/tsconfig.node.json(1 hunks)examples/svelte/vite.config.ts(1 hunks)package.json(4 hunks)scripts/build.ts(3 hunks)src/mock/svelte.ts(1 hunks)src/svelte/OidcProvider.svelte(1 hunks)src/svelte/OidcProviderLogged.svelte(1 hunks)src/svelte/OidcProviderProps.ts(1 hunks)src/svelte/index.svelte.ts(1 hunks)src/svelte/oidc.context.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/mock/svelte.ts (3)
src/tools/ValueOrAsyncGetter.ts (1)
ValueOrAsyncGetter(1-1)src/mock/oidc.ts (1)
ParamsOfCreateMockOidc(9-28)src/svelte/index.svelte.ts (1)
createSvelteOidc_dependencyInjection(118-395)
examples/svelte/src/main.lazy.ts (1)
examples/react-router-framework/app/root.tsx (1)
App(42-56)
src/svelte/index.svelte.ts (8)
src/tools/tsafe/Param0.ts (1)
Param0(6-12)src/tools/tsafe/Parameters.ts (1)
Parameters(2-3)src/svelte/OidcProviderProps.ts (2)
OidcProviderOidcProps(4-9)OidcProviderProps(11-19)src/tools/ValueOrAsyncGetter.ts (1)
ValueOrAsyncGetter(1-1)src/tools/Deferred.ts (1)
Deferred(1-35)src/svelte/oidc.context.ts (1)
getOidcContext(12-14)src/tools/toFullyQualifiedUrl.ts (1)
toFullyQualifiedUrl(13-58)src/core/Oidc.ts (1)
DecodedIdToken_base(135-142)
examples/svelte/src/oidc.ts (2)
src/keycloak/keycloak-js/Keycloak.ts (2)
init(80-260)oidc(1052-1063)examples/multi-providers/src/oidc.tsx (1)
getOidc(38-59)
src/svelte/OidcProviderProps.ts (1)
src/tools/tsafe/Parameters.ts (1)
Parameters(2-3)
🪛 Biome (2.1.2)
scripts/build.ts
[error] 72-76: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 dotenv-linter (3.3.0)
examples/svelte/.env.local.sample
[warning] 5-5: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
examples/svelte/README.md
38-38: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (19)
scripts/build.ts (1)
322-322: LGTM! Cleanup step correctly removes CJS artifacts.This appropriately removes the CJS-compiled
dist/sveltedirectory while preserving the ESM Svelte sources indist/esm/svelte. Theforce: trueflag ensures no error if the directory doesn't exist.examples/svelte/src/lib/Counter.svelte (1)
1-10: LGTM!The component correctly uses Svelte 5's
$staterune for reactive state management. The implementation is clean and follows Svelte best practices.examples/svelte/index.html (1)
1-13: LGTM!Standard Vite + Svelte HTML entry point with proper meta tags and module script loading.
package.json (4)
51-56: LGTM!The peer dependencies correctly include Angular and Svelte packages with wildcard versions, allowing flexibility for users.
68-76: LGTM!Properly marks all framework peer dependencies as optional, maintaining the library's framework-agnostic nature.
139-151: LGTM!The new Svelte and mock Svelte exports follow the established pattern for framework adapters (matching React and Angular exports).
79-104: Unnecessary Zod 3.x compatibility warning
Zod is only imported under src/vendor/backend and the /examples directory; no.parse/.safeParseorZodErrorusage was found in the core code. Pinning to ^3.x carries no immediate risk.src/mock/svelte.ts (1)
5-10: LGTM!The mock factory correctly follows the dependency injection pattern established in the codebase, delegating to
createSvelteOidc_dependencyInjectionwith the mock OIDC creator. The type parameters are properly constrained.examples/svelte/src/vite-env.d.ts (1)
1-1: LGTM!Standard Vite client types reference, correctly configured.
examples/svelte/tsconfig.json (1)
1-4: LGTM!Standard TypeScript project references configuration, correctly structured.
src/svelte/OidcProviderLogged.svelte (1)
1-10: LGTM!Clean implementation using Svelte 5 syntax. The component correctly sets the OIDC context and renders children.
examples/svelte/src/App.svelte (1)
1-10: LGTM!Clean app structure with proper component composition.
examples/svelte/src/main.ts (1)
1-11: LGTM!The early initialization pattern correctly follows the oidc-spa security-first approach, freezing network APIs before the main app loads to mitigate supply-chain and XSS risks. The conditional lazy loading based on
shouldLoadAppis appropriate.Based on learnings.
examples/svelte/src/components/LoggedInAuthButton.svelte (1)
8-8: Verify thatnameclaim is always present in the ID token.Direct access to
decodedIdToken.nameassumes thenameclaim exists. If the OIDC provider doesn't always include this claim, a runtime error will occur.Consider adding a fallback or validation:
- <span>Hello {decodedIdToken.name}</span> + <span>Hello {decodedIdToken.name ?? 'User'}</span>Or validate the ID token shape with Zod to enforce the presence of required claims. Based on learnings.
examples/svelte/tsconfig.app.json (1)
1-21: LGTM!The TypeScript configuration is well-structured for a Svelte application with appropriate compiler options and includes patterns.
examples/svelte/.env.local.sample (1)
15-15: Verify necessity ofVITE_OIDC_CLIENT_SECRETfor Google OAuth in an SPA.OAuth client secrets should not be used in single-page applications, as they cannot be kept confidential in client-side code. Google OAuth for SPAs typically uses the Authorization Code Flow with PKCE without a client secret.
Confirm whether this is truly needed for the Google configuration or if it should be removed to align with SPA security best practices.
examples/svelte/svelte.config.js (1)
1-8: LGTM!The Svelte configuration correctly uses
vitePreprocessfrom the official Vite plugin, which is the standard approach for integrating Svelte with Vite.examples/svelte/src/components/Header.svelte (1)
1-35: LGTM!The Header component correctly integrates OIDC state and routing. The error handling provides helpful feedback to users, and the conditional rendering of authentication buttons based on login state is appropriate.
examples/svelte/tsconfig.node.json (1)
1-26: LGTM!The Node TypeScript configuration is well-structured with appropriate strict settings for build tooling. The target and module resolution settings align with modern Node.js and bundler requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
scripts/build.ts (2)
72-89: Previous review concerns addressed successfully.The code now properly:
- Wraps the Svelte file handling in a block to prevent variable scope leakage
- Checks for directory existence before calling
readdirSync- Uses explicit path construction for Node.js compatibility
Minor improvement: Change
let filetoconst fileon line 82 since the loop variable is never reassigned.Apply this diff for the minor improvement:
- for (let file of svelteFiles) { + for (const file of svelteFiles) { fs.copyFileSync( pathJoin(srcSvelteDirPath, file.name), pathJoin(distDirPath, "svelte", `${file.name}`)
222-222: Revert unnecessary template interpolation.The change from
test: /\.js$/,totest: ${/\.js$/},is functionally identical (the regex.toString()produces the same string) but adds unnecessary complexity. The original direct literal was clearer.Apply this diff to restore clarity:
- ` test: ${/\.js$/},`, + ` test: /\.js$/,`,examples/svelte/src/components/AutoLogoutWarningOverlay.svelte (1)
10-10: Refactor inline styles into a CSS class.The extensive inline styles reduce maintainability and readability. Consider extracting them into a
<style>block or external stylesheet.Apply this diff:
+<style> + .logout-overlay { + position: fixed; + top: 0; + left: 0; + right: 0; + bottom: 0; + background-color: rgba(0, 0, 0, 0.5); + backdrop-filter: blur(10px); + display: flex; + justify-content: center; + align-items: center; + z-index: 1000; + } +</style> + {#if $secondsLeft} - <div - style="position: fixed; top: 0; left: 0; right: 0; bottom: 0; background-color: rgba(0,0,0,0.5); backdrop-filter: blur(10px); display: flex; justify-content: center; align-items: center; z-index: 1000;" - > + <div class="logout-overlay"> <div>src/svelte/OidcProviderProps.ts (2)
16-17: Consider more explicit typing for mount-derived types.Extracting types via
Parameters<typeof mount>couples these declarations to Svelte's internal API shape. If Svelte adjusts themountsignature in a minor release, or if the extractedpropsproperty resolves toany, type safety could degrade.Consider one of the following approaches:
- Option 1 (recommended): Explicitly define the expected types:
App: Component; appProps?: Record<string, any>;- Option 2: Add runtime or compile-time assertions to verify the extracted types match expectations, or document the assumed
mountsignature more prominently.This improves clarity and reduces fragility when Svelte evolves.
18-18: Consider a discriminated union for initialization state.The current union type
OidcInitializationError | Oidc<DecodedIdToken> | undefinedrequires consumers to perform runtime type checks (e.g.,instanceof, property checks) to discriminate between states.A discriminated union with a
statusorkindfield would improve type safety and reduce boilerplate:oidcOrInitializationError: | { status: "loading" } | { status: "error"; error: OidcInitializationError } | { status: "ready"; oidc: Oidc<DecodedIdToken> };This pattern enables exhaustive switch statements and better IntelliSense support. However, this refactor can be deferred if the current approach is sufficient for your use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/svelte/README.md(1 hunks)examples/svelte/src/Fallback.svelte(1 hunks)examples/svelte/src/app.css(1 hunks)examples/svelte/src/components/AutoLogoutWarningOverlay.svelte(1 hunks)examples/svelte/src/main.lazy.ts(1 hunks)examples/svelte/src/pages/PublicPage.svelte(1 hunks)scripts/build.ts(3 hunks)src/svelte/OidcProviderProps.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/svelte/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/svelte/src/main.lazy.ts
- examples/svelte/src/Fallback.svelte
- examples/svelte/src/pages/PublicPage.svelte
- examples/svelte/src/app.css
🧰 Additional context used
🧬 Code graph analysis (1)
src/svelte/OidcProviderProps.ts (1)
src/tools/tsafe/Parameters.ts (1)
Parameters(2-3)
🔇 Additional comments (4)
scripts/build.ts (1)
329-329: LGTM! Defensive cleanup prevents stale artifacts.The removal of
dist/svelteat the root level is appropriate defensive cleanup. Since Svelte files are copied todist/esm/svelte/during the ESM build, this ensures no stale root-level Svelte directory remains. Theforce: trueflag correctly prevents errors if the directory doesn't exist.examples/svelte/src/components/AutoLogoutWarningOverlay.svelte (2)
14-14: Previous issue resolved.The inconsistent store access flagged in the earlier review has been fixed. Line 14 now correctly uses
{$secondsLeft}with the$prefix.
8-17: Verify the intended UX: no dismiss or extend session action.The overlay displays a countdown but doesn't provide any way for users to dismiss it or extend their session. This may be intentional, but consider whether users should have the option to stay logged in.
If user interaction is needed, you could add a button to extend the session or dismiss the warning. Would you like me to suggest an implementation that includes user controls?
src/svelte/OidcProviderProps.ts (1)
4-9: Clarify ErrorFallback conditional logic
CurrentlyErrorFallbackis only allowed whenAutoLoginistrue, but initialization errors can occur even in manual-login flows. ShouldErrorFallbackalso be available whenAutoLoginisfalse?
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
examples/svelte-multi-providers/src/main.ts (1)
9-11: Consider adding error handling for the dynamic import.The dynamic import of
main.lazylacks error handling. If the module fails to load, the application will silently fail without user feedback.Apply this diff to add error handling:
if (shouldLoadApp) { - import("./main.lazy"); + import("./main.lazy").catch((error) => { + console.error("Failed to load application:", error); + // Optionally display user-facing error UI + }); }examples/svelte-multi-providers/src/components/LoggedInAuthButton.svelte (1)
7-10: Add defensive checks for token fields.Direct access to
decodedIdToken.nameassumes the field exists. WhileuseOidc_assertUserLoggedInguarantees a logged-in state, the token shape depends on the OIDC provider configuration.Apply this diff to handle missing name field:
<div> - <span>Hello {decodedIdToken.name}</span> + <span>Hello {decodedIdToken.name ?? "User"}</span> <button onclick={() => logout({ redirectTo: "home" })}>Logout</button> </div>examples/svelte-multi-providers/src/components/AutoLogoutWarningOverlay.svelte (1)
8-17: Consider adding user interaction options.The overlay displays a countdown but provides no way for the user to acknowledge their presence or dismiss the warning. This could lead to frustration if a user is actively working but the overlay appears.
Consider adding an interaction button:
{#if $secondsLeft} <div style="position: fixed; top: 0; left: 0; right: 0; bottom: 0; background-color: rgba(0,0,0,0.5); backdrop-filter: blur(10px); display: flex; justify-content: center; align-items: center; z-index: 1000;" > <div> <p>Are you still there?</p> <p>You will be logged out in {$secondsLeft}</p> + <button onclick={() => { /* Reset countdown or make user action */ }}> + I'm still here + </button> </div> </div> {/if}Verify whether the OIDC library's
useAutoLogoutWarningCountdownhook provides a method to reset or acknowledge the countdown.examples/svelte-multi-providers/src/components/NotLoggedInAuthButton.svelte (1)
18-18: Consider dynamic button text.The button text "Login with Google or Microsoft" is hardcoded, but
askUserToSelectProvider()suggests the available providers might be configurable. If the provider list can vary, consider making the button text dynamic to reflect the actual available providers.src/svelte/index.svelte.ts (2)
456-467: PreserveautoLogin-conditioned API types without@ts-expect-errorHardcoding
falseerodes type guarantees (e.g., presence ofenforceLogin). Cast the assembled object to the correct conditional type and drop the error suppression.Apply this diff:
- const oidcSvelteApi: OidcSvelteApi<DecodedIdToken, false> = { + const oidcSvelteApi = { initializeOidc, OidcContextProvider, mountOidc, useOidc: useOidc as any, getOidc, enforceLogin - }; - - // @ts-expect-error: We know what we are doing - return oidcSvelteApi; + } as OidcSvelteApi< + DecodedIdToken, + ParamsOfCreateOidc extends { autoLogin?: true | undefined } ? true : false + >; + return oidcSvelteApi;
216-219: Typos in error messagesMinor but user-facing; fix spelling to improve DX.
Apply this diff:
- "You must call initializeOidc only once per provider to intiialize" + "You must call initializeOidc only once per provider to initialize" @@ - "Oidc not inizitialized, you must call initializeOidc or mountOidc" + "OIDC not initialized, you must call initializeOidc or mountOidc" @@ - "Oidc not inizitialized, you must call initializeOidc or mountOidc" + "OIDC not initialized, you must call initializeOidc or mountOidc"Also applies to: 270-273, 309-316
examples/svelte-multi-providers/src/oidc.ts (1)
131-157: Route guard logic looks correct; small UX thoughtGuard returns true when logged in, throws on
preload, otherwise prompts and logs in. Consider returning a redirect boolean whenhistory.back()isn’t desired (e.g., deep-linked protected route). Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
examples/svelte-multi-providers/src/assets/svelte.svgis excluded by!**/*.svgexamples/svelte-multi-providers/src/assets/vite.svgis excluded by!**/*.svgexamples/svelte-multi-providers/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/svelte/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
examples/svelte-multi-providers/.env.local.sample(1 hunks)examples/svelte-multi-providers/.gitignore(1 hunks)examples/svelte-multi-providers/README.md(1 hunks)examples/svelte-multi-providers/index.html(1 hunks)examples/svelte-multi-providers/package.json(1 hunks)examples/svelte-multi-providers/src/App.svelte(1 hunks)examples/svelte-multi-providers/src/app.css(1 hunks)examples/svelte-multi-providers/src/components/AutoLogoutWarningOverlay.svelte(1 hunks)examples/svelte-multi-providers/src/components/Google.svelte(1 hunks)examples/svelte-multi-providers/src/components/Header.svelte(1 hunks)examples/svelte-multi-providers/src/components/LoggedInAuthButton.svelte(1 hunks)examples/svelte-multi-providers/src/components/Microsoft.svelte(1 hunks)examples/svelte-multi-providers/src/components/NotLoggedInAuthButton.svelte(1 hunks)examples/svelte-multi-providers/src/components/SelectProviderDialog.svelte(1 hunks)examples/svelte-multi-providers/src/env.d.ts(1 hunks)examples/svelte-multi-providers/src/main.lazy.ts(1 hunks)examples/svelte-multi-providers/src/main.ts(1 hunks)examples/svelte-multi-providers/src/oidc.ts(1 hunks)examples/svelte-multi-providers/src/pages/ProtectedPage.svelte(1 hunks)examples/svelte-multi-providers/src/pages/PublicPage.svelte(1 hunks)examples/svelte-multi-providers/src/routes.ts(1 hunks)examples/svelte-multi-providers/src/vite-env.d.ts(1 hunks)examples/svelte-multi-providers/svelte.config.js(1 hunks)examples/svelte-multi-providers/tsconfig.app.json(1 hunks)examples/svelte-multi-providers/tsconfig.json(1 hunks)examples/svelte-multi-providers/tsconfig.node.json(1 hunks)examples/svelte-multi-providers/vite.config.ts(1 hunks)examples/svelte/src/App.svelte(1 hunks)examples/svelte/src/env.d.ts(1 hunks)examples/svelte/src/main.lazy.ts(1 hunks)examples/svelte/src/oidc.ts(1 hunks)src/svelte/OidcContextProvider.svelte(1 hunks)src/svelte/OidcProvider.svelte(1 hunks)src/svelte/OidcProviderProps.ts(1 hunks)src/svelte/index.svelte.ts(1 hunks)src/svelte/oidc.context.ts(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- examples/svelte-multi-providers/index.html
- examples/svelte-multi-providers/tsconfig.json
- examples/svelte-multi-providers/src/vite-env.d.ts
- examples/svelte-multi-providers/src/app.css
- examples/svelte-multi-providers/README.md
- examples/svelte-multi-providers/src/pages/PublicPage.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/svelte/src/main.lazy.ts
- examples/svelte/src/App.svelte
🧰 Additional context used
🧬 Code graph analysis (5)
examples/svelte-multi-providers/src/routes.ts (1)
examples/svelte-multi-providers/src/oidc.ts (1)
beforeLoad_protectedRoute(131-158)
examples/svelte-multi-providers/src/oidc.ts (1)
src/svelte/index.svelte.ts (2)
createSvelteOidc(469-474)OidcSvelte(22-24)
src/svelte/index.svelte.ts (6)
src/tools/tsafe/Param0.ts (1)
Param0(6-12)src/svelte/OidcProviderProps.ts (2)
OidcProviderOidcProps(4-9)OidcProviderProps(12-21)src/tools/ValueOrAsyncGetter.ts (1)
ValueOrAsyncGetter(1-1)src/tools/Deferred.ts (1)
Deferred(1-35)src/svelte/oidc.context.ts (2)
setOidcContext(4-11)getOidcContext(13-17)src/tools/toFullyQualifiedUrl.ts (1)
toFullyQualifiedUrl(13-58)
examples/svelte/src/oidc.ts (2)
src/mock/svelte.ts (1)
createMockSvelteOidc(5-10)src/svelte/index.svelte.ts (1)
createSvelteOidc(469-474)
src/svelte/OidcProviderProps.ts (1)
src/tools/tsafe/Parameters.ts (1)
Parameters(2-3)
🪛 dotenv-linter (3.3.0)
examples/svelte-multi-providers/.env.local.sample
[warning] 9-9: [UnorderedKey] The VITE_OIDC_CLIENT_ID_MICROSOFT key should go before the VITE_OIDC_ISSUER_URI_MICROSOFT key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The VITE_OIDC_CLIENT_ID_GOOGLE key should go before the VITE_OIDC_ISSUER_URI_GOOGLE key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The VITE_OIDC_CLIENT_SECRET_GOOGLE key should go before the VITE_OIDC_ISSUER_URI_GOOGLE key
(UnorderedKey)
🪛 Gitleaks (8.28.0)
examples/svelte-multi-providers/.env.local.sample
[high] 15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (21)
examples/svelte-multi-providers/.gitignore (1)
1-24: Gitignore entries look solid.Standard log, build, and editor artifacts are covered, with the VS Code override handled correctly.
examples/svelte-multi-providers/src/env.d.ts (1)
1-10: LGTM!The ambient type declarations follow the standard Vite pattern for typed environment variable access. The structure is consistent with the single-provider example.
src/svelte/OidcContextProvider.svelte (1)
1-15: LGTM with a note on the prop pattern.The component correctly sets the OIDC context and renders children. The generic
DecodedIdTokenparameter enables type-safe token handling.Note: Passing
setOidcContextas a prop (rather than importing it directly) is an uncommon pattern but provides flexibility for testing and dependency injection.examples/svelte-multi-providers/src/App.svelte (1)
1-19: LGTM!The multi-provider composition correctly nests the Google and Microsoft provider contexts, allowing both to be available throughout the component tree. The Router and SelectProviderDialog are appropriately positioned as siblings inside the provider wrappers.
examples/svelte-multi-providers/src/main.lazy.ts (1)
1-12: LGTM!The lazy entry point correctly uses the Svelte 5
mountAPI, provides clear error handling for the missing DOM element, and exports the mounted app instance for HMR and testing support.examples/svelte-multi-providers/src/components/Header.svelte (1)
10-12: Review the absolute positioning choice.The header uses
position: absolutewhich removes it from the document flow and may cause content overlap. Consider whetherposition: fixed(for a sticky header) or normal flow positioning better suits the layout needs.If you're seeing layout issues or content being hidden under the header, try:
<div - style="display: flex; justify-content: space-between; align-items: center; position: absolute; top: 0; left: 0; width: 100%;" + style="display: flex; justify-content: space-between; align-items: center; position: fixed; top: 0; left: 0; width: 100%; background: white; padding: 1rem; box-shadow: 0 2px 4px rgba(0,0,0,0.1); z-index: 100;" >Or remove absolute positioning entirely and let content flow naturally with appropriate padding/margin on the page content.
examples/svelte-multi-providers/svelte.config.js (1)
1-8: LGTM!Standard Svelte configuration using
vitePreprocesswith proper type annotations. This follows the official Svelte + Vite integration pattern.examples/svelte-multi-providers/package.json (2)
7-7: Verify the prepare script logic.The prepare script uses a conditional to avoid overwriting an existing
.env.localfile. The|| trueensures the script doesn't fail if the file exists. This is correct, but ensure that the.env.local.samplefile exists in the repository so the copy can succeed when needed.
18-18: Verified specified package versions exist on npm. No action required.examples/svelte-multi-providers/vite.config.ts (1)
1-7: LGTM!Standard Vite configuration with the Svelte plugin. The use of
defineConfigprovides type safety, and the configuration follows Vite best practices.examples/svelte-multi-providers/src/components/Google.svelte (1)
1-33: LGTM!The component correctly handles the three OIDC initialization states (loading, error, success) with proper type guarding using
instanceofforOidcInitializationError. The context provider wrapping in the success state follows the expected pattern for multi-provider OIDC integration.examples/svelte-multi-providers/src/components/SelectProviderDialog.svelte (1)
1-39: LGTM!The component correctly uses the
evtlibrary for event-driven modal management, properly subscribes/unsubscribes withonDestroycleanup, and handles the dialog lifecycle appropriately. The default behavior of postingundefinedwhen the dialog is closed (e.g., via ESC key) is a good UX pattern.examples/svelte/src/env.d.ts (1)
1-10: LGTM!Standard ambient type declarations for Vite environment variables. This properly extends
ImportMeta.envto provide type safety for the OIDC configuration variables used throughout the Svelte examples.src/svelte/oidc.context.ts (1)
1-17: LGTM!Type-safe wrappers around Svelte's context API that enable sharing OIDC state across components. The use of symbol-based keys prevents naming collisions, and the generic type parameters provide proper type inference for consumers.
src/svelte/OidcProvider.svelte (1)
1-39: LGTM: Initialization flow, fallbacks, and context provisioning are soundProps typing, error branching, and context setup look correct.
examples/svelte-multi-providers/src/routes.ts (1)
6-19: Verify default route path and pre-hook cause
- Is omitting
pathon the first route the intended way to match the root with this router? Some routers require''or'/'.prepasses{ cause: "" }. If you intended to trigger the “preload” branch, use"preload". Otherwise, current logic will prompt login rather than throw the preload sentinel error.examples/svelte/src/oidc.ts (1)
12-32: LGTM: Conditional mock/real OIDC wiringClean split between mock and real OIDC with correct export surface. The note about
autoLoginvsisUserInitiallyLoggedInis helpful.src/svelte/OidcProviderProps.ts (1)
4-21: LGTM: Conditional prop types are well-modeled
AutoLogin-based gating ofErrorFallbackand mount-derivedApp/appPropstypes look good.src/svelte/index.svelte.ts (1)
235-266: Build config check:.svelte.tsrunesThis module uses
$statein.svelte.ts. Ensure your Vite/Svelte plugin is configured to transform runes in TS modules (not only.sveltecomponents). If not, this will fail to compile.Based on learnings
examples/svelte-multi-providers/src/oidc.ts (1)
38-59: LGTM: Multi-provider getOidc flowSequential provider check with a precise not-logged-in shape is clear and ergonomic.
examples/svelte-multi-providers/src/pages/ProtectedPage.svelte (1)
106-107: Fix Svelte event binding: useon:click, notonclickCurrent handler won’t fire in Svelte.
Apply this diff:
- <button onclick={() => renewTokens()}>Renew tokens </button> + <button on:click={() => renewTokens()}>Renew tokens</button>⛔ Skipped due to learnings
Learnt from: luca-peruzzo PR: keycloakify/oidc-spa#104 File: examples/svelte/src/components/NotLoggedInAuthButton.svelte:16-37 Timestamp: 2025-10-06T07:01:46.903Z Learning: In Svelte 5, event handling syntax changed from directives (e.g., `on:click`) to DOM properties (e.g., `onclick`). The old `on:click` syntax is deprecated. When reviewing Svelte 5 code, `onclick` (and other event properties like `onsubmit`, `onchange`, etc.) are the correct syntax, not `on:click`.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
examples/svelte/src/components/NotLoggedInAuthButton.svelte (1)
1-39: LGTM! Optional: simplify the Login button handler.The component correctly implements login/register flows for both Keycloak and Auth0 providers. The Svelte 5
onclicksyntax is correct (as confirmed by learnings).Optionally, Line 16 can be simplified:
- <button onclick={() => login()}>Login</button> + <button onclick={login}>Login</button>This eliminates an unnecessary arrow function wrapper.
src/svelte/index.svelte.ts (1)
401-447: Optional: Add JSDoc to document client-side usage.The
enforceLoginfunction safely accesseslocation.hrefbecause it's designed for client-side route guards only. Based on learnings, this is correct for the intended use case.To make this explicit for future maintainers, consider adding a JSDoc comment:
+ /** + * Enforces user login for protected routes. + * + * **Client-only**: This function should only be called in client-side route guards. + * It is not safe for server-side rendering (SSR) contexts. + * + * @param loaderParams - Parameters containing request URL or location + * @returns Promise resolving to true if login check passes + */ async function enforceLogin(loaderParams: {Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/svelte-multi-providers/src/components/NotLoggedInAuthButton.svelte(1 hunks)examples/svelte/src/components/NotLoggedInAuthButton.svelte(1 hunks)src/svelte/index.svelte.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/svelte-multi-providers/src/components/NotLoggedInAuthButton.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-06T14:55:09.648Z
Learnt from: luca-peruzzo
PR: keycloakify/oidc-spa#104
File: src/svelte/index.svelte.ts:431-433
Timestamp: 2025-10-06T14:55:09.648Z
Learning: In the oidc-spa Svelte adapter, the `enforceLogin` function is designed for client-side route guards only and is not intended for server-side rendering (SSR) contexts. It safely accesses `location.href` because it runs exclusively in browser environments with client-side routers like mateothegreat/svelte5-router.
Applied to files:
src/svelte/index.svelte.ts
📚 Learning: 2025-10-06T07:01:46.903Z
Learnt from: luca-peruzzo
PR: keycloakify/oidc-spa#104
File: examples/svelte/src/components/NotLoggedInAuthButton.svelte:16-37
Timestamp: 2025-10-06T07:01:46.903Z
Learning: In Svelte 5, event handling syntax changed from directives (e.g., `on:click`) to DOM properties (e.g., `onclick`). The old `on:click` syntax is deprecated. When reviewing Svelte 5 code, `onclick` (and other event properties like `onsubmit`, `onchange`, etc.) are the correct syntax, not `on:click`.
Applied to files:
examples/svelte/src/components/NotLoggedInAuthButton.svelte
🧬 Code graph analysis (1)
src/svelte/index.svelte.ts (6)
src/tools/tsafe/Param0.ts (1)
Param0(6-12)src/svelte/OidcProviderProps.ts (2)
OidcProviderOidcProps(4-9)OidcProviderProps(12-21)src/tools/ValueOrAsyncGetter.ts (1)
ValueOrAsyncGetter(1-1)src/tools/Deferred.ts (1)
Deferred(1-35)src/svelte/oidc.context.ts (2)
setOidcContext(4-11)getOidcContext(13-17)src/tools/toFullyQualifiedUrl.ts (1)
toFullyQualifiedUrl(13-58)
🔇 Additional comments (2)
src/svelte/index.svelte.ts (2)
453-459: Type casts are acceptable given the conditional type complexity.The
as anycast on Line 453 and the@ts-expect-erroron Line 458 are necessary workarounds for TypeScript's limitations with conditional types in this context. The API surface is correctly typed for consumers.
15-460: Well-structured OIDC adapter implementation.The overall implementation is solid:
- Clean separation of concerns with dependency injection
- Proper lifecycle management with deferred initialization
- Type-safe API surface with discriminated unions
- Correct handling of auto-login vs manual login scenarios
- Proper cleanup of subscriptions in onMount hooks
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/svelte/index.svelte.ts (1)
265-266: Fix typo in error message."inizitialized" → "initialized".
- "Oidc not inizitialized, you must call initializeOidc or mountOidc" + "Oidc not initialized, you must call initializeOidc or mountOidc"
🧹 Nitpick comments (4)
src/svelte/index.svelte.ts (4)
105-114: Polish doc comments (typos/wording).Minor fixes for clarity.
- * NB: this can be used only when having a single OIDC provider and when usign Svelte without svelte-kit. - * For a generic initialization referes to `initializeOidc` + * NB: this can be used only when having a single OIDC provider and when using Svelte without SvelteKit. + * For a generic initialization refer to `initializeOidc`.
401-447: Document client-only usage for enforceLogin.Clarify SPA-only intent to avoid future SSR misuse.
+ /** + * Client-only route guard that enforces login in SPA contexts. + * Not intended for SSR/SvelteKit loaders. It may access `location.href`. + */ async function enforceLogin(loaderParams: {Based on learnings
143-159: Avoid generic name shadowing the imported ParamsOfCreateOidc.Shadowing hurts readability; rename the generic.
-export function createSvelteOidc_dependencyInjection< - DecodedIdToken extends Record<string, unknown>, - ParamsOfCreateOidc extends { +export function createSvelteOidc_dependencyInjection< + DecodedIdToken extends Record<string, unknown>, + TCreateParams extends { autoLogin?: boolean; } & ( | { - decodedIdTokenSchema: { parse: (data: unknown) => DecodedIdToken } | undefined; + decodedIdTokenSchema: { parse: (data: unknown) => DecodedIdToken } | undefined; } | {} ) >( - paramsOrGetParams: ValueOrAsyncGetter<ParamsOfCreateOidc>, - createOidc: (params: ParamsOfCreateOidc) => Promise<Oidc<DecodedIdToken>> -): OidcSvelteApi< - DecodedIdToken, - ParamsOfCreateOidc extends { autoLogin?: true | undefined } ? true : false + paramsOrGetParams: ValueOrAsyncGetter<TCreateParams>, + createOidc: (params: TCreateParams) => Promise<Oidc<DecodedIdToken>> +): OidcSvelteApi< + DecodedIdToken, + TCreateParams extends { autoLogin?: true | undefined } ? true : false > {Also update downstream conditional types and parameter annotations accordingly in this function body.
449-460: Remove ts-expect-error; return with a safer cast.Preserve runtime shape while avoiding suppression comments.
- const oidcSvelteApi: OidcSvelteApi<DecodedIdToken, false> = { + const oidcSvelteApi = { initializeOidc, OidcContextProvider, mountOidc, - useOidc: useOidc as any, + useOidc: useOidc as any, getOidc, enforceLogin }; - // @ts-expect-error: We know what we are doing - return oidcSvelteApi; + return oidcSvelteApi as unknown as OidcSvelteApi< + DecodedIdToken, + TCreateParams extends { autoLogin?: true | undefined } ? true : false + >;Note: Longer-term, consider shaping the returned API conditionally (omit enforceLogin when autoLogin is true) to align types and runtime more strictly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/svelte/index.svelte.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-06T14:55:09.648Z
Learnt from: luca-peruzzo
PR: keycloakify/oidc-spa#104
File: src/svelte/index.svelte.ts:431-433
Timestamp: 2025-10-06T14:55:09.648Z
Learning: In the oidc-spa Svelte adapter, the `enforceLogin` function is designed for client-side route guards only and is not intended for server-side rendering (SSR) contexts. It safely accesses `location.href` because it runs exclusively in browser environments with client-side routers like mateothegreat/svelte5-router.
Applied to files:
src/svelte/index.svelte.ts
🧬 Code graph analysis (1)
src/svelte/index.svelte.ts (6)
src/tools/tsafe/Param0.ts (1)
Param0(6-12)src/svelte/OidcProviderProps.ts (2)
OidcProviderOidcProps(4-9)OidcProviderProps(12-21)src/tools/ValueOrAsyncGetter.ts (1)
ValueOrAsyncGetter(1-1)src/tools/Deferred.ts (1)
Deferred(1-35)src/svelte/oidc.context.ts (2)
setOidcContext(4-11)getOidcContext(13-17)src/tools/toFullyQualifiedUrl.ts (1)
toFullyQualifiedUrl(13-58)
🔇 Additional comments (1)
src/svelte/index.svelte.ts (1)
243-257: Can you confirm thatvite-plugin-svelteis configured to process.svelte.tsmodules (e.g., via itsextensionsorincludeoption)? Using$stateinsrc/svelte/index.svelte.tsrequires the Svelte compiler to handle that extension. If it isn’t supported, move this logic into a standard.sveltecomponent or use a Svelte store with the mount API.
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores
Build