Uh oh!
There was an error while loading. Please reload this page.
Add nextjs migration logic - #42
Conversation
Reviewer's GuideImplements SuperTokens migration support for React and Next.js Rownd providers by normalizing SuperTokens app info, wiring a sign-in completed event listener to call a migration endpoint for new users, extending provider props to accept SuperTokens config, and adding full-stack example apps (React and Next.js) demonstrating the migration flow with a SuperTokens + Express backend. Sequence diagram for Rownd-to-SuperTokens migration on new user sign-insequenceDiagram
actor User
participant Frontend as React_or_Next_client
participant RowndHub as Rownd_hub_events
participant STBackend as SuperTokens_backend
User->>Frontend: requestSignIn()
Frontend->>RowndHub: requestSignIn()
RowndHub-->>Frontend: events.sign_in_completed(detail.user_type)
alt [detail.user_type == 'new_user']
Frontend->>Frontend: handleSignInCompleted(event)
Frontend->>Frontend: syncUserToSuperTokens(accessToken, appInfo)
Frontend->>STBackend: POST /auth/plugin/rownd/migrate
STBackend-->>Frontend: 2xx/4xx response
else [detail.user_type != 'new_user']
Frontend->>Frontend: (no migration call)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
sign_in_completedevent handling logic (includingaccessTokenRef/supertokensAppInfoRefusage andsyncUserToSuperTokensinvocation) is duplicated betweenReactRowndProviderand the Next.js client; consider extracting a shared helper/hook to avoid divergence over time. - In the
sign_in_completedhandlers you cast toCustomEvent<{ user_type?: string }>without a runtime guard; if these events can be triggered externally, consider checkingevent instanceof CustomEvent(and thatdetailis an object) before accessingdetail.user_typeto avoid potential runtime issues. syncUserToSuperTokensalready catches and logs its own errors, so chaining.catch(() => {})at the call sites in React effects is redundant and can be removed for clarity.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- The `sign_in_completed` event handling logic (including `accessTokenRef`/`supertokensAppInfoRef` usage and `syncUserToSuperTokens` invocation) is duplicated between `ReactRowndProvider` and the Next.js client; consider extracting a shared helper/hook to avoid divergence over time.
- In the `sign_in_completed` handlers you cast to `CustomEvent<{ user_type?: string }>` without a runtime guard; if these events can be triggered externally, consider checking `event instanceof CustomEvent` (and that `detail` is an object) before accessing `detail.user_type` to avoid potential runtime issues.
-`syncUserToSuperTokens` already catches and logs its own errors, so chaining `.catch(() => {})` at the call sites in React effects is redundant and can be removed for clarity.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
0274a67 to
db15799Compare
mhamann
left a comment
There was a problem hiding this comment.
Clean refactor — the inline SuperTokens-migration block in ReactRowndProvider is now a useSuperTokensMigration hook, reused by both the React provider and the Next.js client. The defensive event-type checks (instanceof CustomEvent, typeof detail === 'object') are a real improvement over the previous inline version. Tests cover the new wiring in both providers.
Correctness
- Minor regression — lost rejection swallow on
syncUserToSuperTokens. The old code wrotesyncUserToSuperTokens(...).catch(() => {}); the new code doesvoid syncUserToSuperTokens(...).syncUserToSuperTokensconstructsnew URL()outside itstryblock, so a malformedapiDomain(e.g. missing protocol) throws synchronously and bubbles as an unhandled promise rejection. See line comment onuseSuperTokensMigration.ts:60.
Design / Nits
Three small suggestions
useEffect(..., [supertokens])re-runs whenever a parent passes a new object literal — and the example inlayout.tsxdoes exactly that. See line comment onuseSuperTokensMigration.ts:46.useRefis still imported insrc/context/ReactRowndProvider.tsx(line 1) but no longer referenced after the extraction. (Couldn't anchor — unchanged line — flagging here.)- The Next.js example's
layout.tsxhard-codesappKey="<APP_KEY>"and the README doesn't call out the substitution. See line comment onlayout.tsx:19.
Test coverage
Good positive/negative cases for user_type and event-shape. Not covered: early returns when accessToken is null or supertokens is undefined, and the cleanup (removeEventListener on unmount). Optional, low priority.
Landing recommendation: Safe to merge after the rejection-swallow regression on line 60 is addressed (one-line fix).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary by Sourcery
Add SuperTokens migration support triggered from Rownd sign-in flows and provide example apps demonstrating Rownd-to-SuperTokens lazy user migration for React and Next.js.
New Features:
Enhancements:
Documentation:
Chores: