#169 #170 #32 - #171
Conversation
📝 WalkthroughWalkthroughRefactors multiple modules to store shared coordination state on window-scoped globals instead of module-scoped variables, updates early-init to accept an Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Consumer
participant CreateOidc as createOidc module
participant WindowGlobal as window["__oidc-spa:globalContext:createOidc"]
participant OtherTabs as Other tabs
App->>CreateOidc: registerExports_earlyInit({ exports, isMicroFrontendSetup })
CreateOidc->>WindowGlobal: initialize/set dEarlyInitResolved & actual_exposedForMicroFrontendSetup
CreateOidc->>WindowGlobal: resolve dEarlyInitResolved
App->>CreateOidc: call createOidc()
CreateOidc->>WindowGlobal: await dEarlyInitResolved (or warn after 3s)
WindowGlobal-->>CreateOidc: return actual early-init values
CreateOidc->>CreateOidc: use getGlobalContextActual() for shared state (logout, tokens, flags)
CreateOidc->>OtherTabs: notify via BroadcastChannel / write window-shared globals
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 Tip You can get early access to new features in CodeRabbit.Enable the |
1847d45 to
a325403
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/core/createOidc.ts`:
- Around line 287-291: The module currently mutates
window["__oidc-spa:globalContext:createOidc"] at import time which triggers
"window is not defined" in SSR; change this to a lazy browser-only getter used
by createOidc that first checks typeof window !== "undefined" and only then
initializes the singleton (creating dEarlyInitResolved = new Deferred() and
actual = undefined) and returns the globalContext; update any other similar
window-backed globals in this PR to the same pattern so no window access occurs
during module evaluation.
- Around line 407-413: The new unconditional await on
globalContext.dEarlyInitResolved.pr inside createOidc() causes a silent infinite
hang when oidcEarlyInit() was never called; revert to the original behavior by
not blocking indefinitely: if dEarlyInitResolved.getState().hasResolved is
false, do not await forever—instead race the promise with the existing 3-second
setup warning path (or start the same timeout used later) so the warning fires
if early init is missing; locate symbols dEarlyInitResolved, globalContext,
createOidc(), and oidcEarlyInit() and implement a timed wait or conditional that
preserves the 3s warning instead of awaiting the promise unconditionally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 831c229e-6746-49ff-bc11-ba53e9d3ca48
📒 Files selected for processing (8)
examples/tanstack-start/src/routeTree.gen.tssrc/core/createOidc.tssrc/core/earlyInit.tssrc/core/evtIsUserActive.tssrc/core/loginOrGoToAuthServer.tssrc/core/loginPropagationToOtherTabs.tssrc/core/logoutPropagationToOtherTabs.tssrc/core/ongoingLoginOrRefreshProcesses.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/createOidc.ts (1)
327-329: Consider a more descriptive error message.The current message
"oidc-spa: Wrong micro frontend setup"doesn't help developers understand what went wrong. This condition triggers whenoidcEarlyInit()is called multiple times across different bundles without proper MFE configuration.💡 Suggested improvement
if (globalContext.actual !== undefined) { - throw new Error("oidc-spa: Wrong micro frontend setup"); + throw new Error( + "oidc-spa: oidcEarlyInit() was called multiple times. " + + "If you are using a micro-frontend setup, ensure each MFE bundle " + + "calls oidcEarlyInit({ isMicroFrontendSetup: true })." + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/createOidc.ts` around lines 327 - 329, The thrown error in the globalContext check is too vague; update the Error thrown where globalContext.actual is checked in createOidc (the block reached when oidcEarlyInit() is called multiple times) to a more descriptive message that explains the cause and how to fix it (e.g., indicate that oidcEarlyInit() was invoked by multiple bundles / MFEs and recommend ensuring a single initialization or correct MFE wiring); modify the Error string thrown in that branch to include this actionable context and reference to oidcEarlyInit so developers can quickly identify the misconfiguration.
🤖 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/core/loginOrGoToAuthServer.ts`:
- Around line 21-25: The module currently reads
window["__oidc-spa:globalContext:loginOrGoToAuthServer"] at import time which
breaks SSR; change this to a lazy runtime guard: create a function (e.g.,
getLoginOrGoToAuthServerGlobalContext) that checks typeof window !== "undefined"
before accessing window, and if undefined returns a safe fallback (or
initializes the global using createStatefulEvt only when window exists), and
replace the direct globalContext const with a call to that function so
createStatefulEvt and the window key
("__oidc-spa:globalContext:loginOrGoToAuthServer") are only accessed/executed at
runtime in the browser.
---
Nitpick comments:
In `@src/core/createOidc.ts`:
- Around line 327-329: The thrown error in the globalContext check is too vague;
update the Error thrown where globalContext.actual is checked in createOidc (the
block reached when oidcEarlyInit() is called multiple times) to a more
descriptive message that explains the cause and how to fix it (e.g., indicate
that oidcEarlyInit() was invoked by multiple bundles / MFEs and recommend
ensuring a single initialization or correct MFE wiring); modify the Error string
thrown in that branch to include this actionable context and reference to
oidcEarlyInit so developers can quickly identify the misconfiguration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3e05610-2f56-473b-91a5-b6ab43879572
📒 Files selected for processing (9)
examples/tanstack-start/src/routeTree.gen.tspackage.jsonsrc/core/createOidc.tssrc/core/earlyInit.tssrc/core/evtIsUserActive.tssrc/core/loginOrGoToAuthServer.tssrc/core/loginPropagationToOtherTabs.tssrc/core/logoutPropagationToOtherTabs.tssrc/core/ongoingLoginOrRefreshProcesses.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/core/earlyInit.ts
- src/core/evtIsUserActive.ts
- src/core/ongoingLoginOrRefreshProcesses.ts
- examples/tanstack-start/src/routeTree.gen.ts
| window["__oidc-spa:globalContext:loginOrGoToAuthServer"] ??= { | ||
| evtHasLoginBeenCalled: createStatefulEvt(() => false) | ||
| }; | ||
|
|
||
| const globalContext = window["__oidc-spa:globalContext:loginOrGoToAuthServer"]; |
There was a problem hiding this comment.
Window access at module evaluation time breaks SSR.
Same issue as the other files in this PR - immediate window access at import time causes errors in SSR/Node.js environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/loginOrGoToAuthServer.ts` around lines 21 - 25, The module currently
reads window["__oidc-spa:globalContext:loginOrGoToAuthServer"] at import time
which breaks SSR; change this to a lazy runtime guard: create a function (e.g.,
getLoginOrGoToAuthServerGlobalContext) that checks typeof window !== "undefined"
before accessing window, and if undefined returns a safe fallback (or
initializes the global using createStatefulEvt only when window exists), and
replace the direct globalContext const with a call to that function so
createStatefulEvt and the window key
("__oidc-spa:globalContext:loginOrGoToAuthServer") are only accessed/executed at
runtime in the browser.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/createOidc.ts`:
- Around line 328-345: registerExports_earlyInit currently throws whenever
globalContext.actual_exposedForMicroFrontendSetup is already set, preventing
multiple micro-frontend bundles from calling earlyInit with
isMicroFrontendSetup: true; change the logic so it only errors when there is a
real mode mismatch (existing value !== incoming isMicroFrontendSetup) but allows
repeated registrations when incoming is true and the existing value is also the
micro-frontend marker. Specifically, in registerExports_earlyInit check
globalContext.actual_exposedForMicroFrontendSetup: if undefined, set it as
before; if already set, only throw when its boolean/mode differs from the new
isMicroFrontendSetup flag; otherwise treat it as idempotent (reuse existing
globalContext_actual_moduleScoped and assign exports_earlyInit and resolve
dEarlyInitResolved as currently done).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c20480aa-1d4d-4b68-ac3d-9a6be2ad06f2
📒 Files selected for processing (2)
scripts/start-example.shsrc/core/createOidc.ts
✅ Files skipped from review due to trivial changes (1)
- scripts/start-example.sh
| export function registerExports_earlyInit(params: { | ||
| exports: Exports_earlyInit; | ||
| isMicroFrontendSetup: boolean; | ||
| }): void { | ||
| const { exports, isMicroFrontendSetup } = params; | ||
|
|
||
| if (globalContext.actual_exposedForMicroFrontendSetup !== undefined) { | ||
| throw new Error("oidc-spa: Wrong micro frontend setup"); | ||
| } | ||
|
|
||
| globalContext.actual_exposedForMicroFrontendSetup = isMicroFrontendSetup | ||
| ? globalContext_actual_moduleScoped | ||
| : "not a micro frontend setup"; | ||
|
|
||
| globalContext_actual_moduleScoped.exports_earlyInit = exports; | ||
|
|
||
| globalContext.dEarlyInitResolved.resolve(); | ||
| } |
There was a problem hiding this comment.
Allow repeated isMicroFrontendSetup: true registrations.
Every earlyInit() call is forwarded here, so after the first micro-frontend sets actual_exposedForMicroFrontendSetup, the next bundle that does earlyInit({ isMicroFrontendSetup: true }) hits Lines 334-336 and throws "Wrong micro frontend setup". That makes the new MFE mode effectively single-bundle only. Repeated MFE registrations should be idempotent and reuse the already-exposed shared context; only real mode mismatches should error.
💡 Minimal fix
export function registerExports_earlyInit(params: {
exports: Exports_earlyInit;
isMicroFrontendSetup: boolean;
}): void {
const { exports, isMicroFrontendSetup } = params;
+ const existing = globalContext.actual_exposedForMicroFrontendSetup;
- if (globalContext.actual_exposedForMicroFrontendSetup !== undefined) {
- throw new Error("oidc-spa: Wrong micro frontend setup");
+ if (existing !== undefined) {
+ if (existing !== "not a micro frontend setup" && isMicroFrontendSetup) {
+ return;
+ }
+
+ throw new Error("oidc-spa: Wrong micro frontend setup");
}
globalContext.actual_exposedForMicroFrontendSetup = isMicroFrontendSetup
? globalContext_actual_moduleScoped
: "not a micro frontend setup";
globalContext_actual_moduleScoped.exports_earlyInit = exports;
globalContext.dEarlyInitResolved.resolve();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/createOidc.ts` around lines 328 - 345, registerExports_earlyInit
currently throws whenever globalContext.actual_exposedForMicroFrontendSetup is
already set, preventing multiple micro-frontend bundles from calling earlyInit
with isMicroFrontendSetup: true; change the logic so it only errors when there
is a real mode mismatch (existing value !== incoming isMicroFrontendSetup) but
allows repeated registrations when incoming is true and the existing value is
also the micro-frontend marker. Specifically, in registerExports_earlyInit check
globalContext.actual_exposedForMicroFrontendSetup: if undefined, set it as
before; if already set, only throw when its boolean/mode differs from the new
isMicroFrontendSetup flag; otherwise treat it as idempotent (reuse existing
globalContext_actual_moduleScoped and assign exports_earlyInit and resolve
dEarlyInitResolved as currently done).
Add minimal support for micro frontend setups.
When enabling this option, you must use the manual setup. The Vite plugin is not compatible with this configuration.
See: #169 #170 #32
Summary by CodeRabbit
Refactor
New Features
Chores