fix: migrate auth views to Apsara v1 and add /client subpath export#1581
fix: migrate auth views to Apsara v1 and add /client subpath export#1581rohanchkrabrty merged 8 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
📝 WalkthroughWalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 8 minutes and 42 seconds.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: 10
🧹 Nitpick comments (9)
web/sdk/react/views-new/auth/components/auth-header.module.css (1)
11-15: Consider preventing logo stretching for non-square assets.With fixed dimensions, non-square logos can appear distorted. Adding
object-fitmakes this more resilient.Proposed tweak
.logo { border-radius: var(--rs-space-3); width: 80px; height: 80px; + object-fit: contain; }web/sdk/react/views-new/auth/components/oidc-button.tsx (1)
13-31: Props fromHTMLProps<HTMLButtonElement>are not forwarded to the Button.The component type accepts
HTMLProps<HTMLButtonElement>, but onlyonClickandproviderare destructured and used. Other props likeclassName,disabled,aria-*, and customdata-test-idpassed by callers (e.g.,sign-up-view.tsxline 72 passesdata-test-id="frontier-sdk-signup-page-oidc-btn") are silently ignored.Additionally, the hardcoded
data-test-id="frontier-sdk-oidc-logo-btn"on line 19 conflicts with caller-provided test IDs.♻️ Spread remaining props to the Button
-export const OIDCButton = ({ onClick, provider }: OIDCButtonProps) => ( +export const OIDCButton = ({ onClick, provider, ...rest }: OIDCButtonProps) => ( <Button variant="outline" color="neutral" className={styles.button} onClick={onClick} - data-test-id="frontier-sdk-oidc-logo-btn" + data-test-id="frontier-sdk-oidc-logo-btn" + {...rest} >web/sdk/react/views-new/auth/sign-up/sign-up-view.tsx (1)
66-75: Use strategy name as key instead of array index.Using
indexas key can cause issues if the strategies list changes order or items are added/removed. Strategy names are unique identifiers and should be used as keys.♻️ Use strategy name as key
{filteredOIDC.map((s, index) => { return ( <OIDCButton - key={index} + key={s.name} onClick={() => clickHandler(s.name)} provider={s.name || ''} data-test-id="frontier-sdk-signup-page-oidc-btn" /> ); })}web/sdk/react/views-new/auth/sign-in/sign-in-view.tsx (2)
68-77: Use strategy name as key instead of array index.Same issue as in
sign-up-view.tsx- usingindexas key can cause rendering issues.♻️ Use strategy name as key
{filteredOIDC.map((s, index) => { return ( <OIDCButton - key={index} + key={s.name} onClick={() => clickHandler(s.name)} provider={s.name || ''} data-test-id="frontier-sdk-oidc-btn" /> ); })}
41-57: Consider extracting shared authentication logic.
SignInViewandSignUpViewshare nearly identical code for:
- Strategy querying and filtering (lines 32-35, 59-62)
- The
clickHandlerimplementation (lines 41-57)- OIDC button rendering pattern
Consider extracting this into a shared hook (e.g.,
useAuthStrategies) to reduce duplication and ensure consistent behavior.web/sdk/react/views-new/auth/subscribe/subscribe-view.tsx (1)
26-42: Phone number validation regex is overly permissive.The regex
/^[+\d\s\-()]+$/allows strings composed entirely of whitespace, parentheses, or dashes (e.g.,"---","( )"), which are not valid phone numbers. Consider adding a check that at least one digit is present.🔧 Suggested refinement
.test( 'digits-only', 'Must contain only numbers with country code', value => { if (!value?.trim()) return true; - return /^[+\d\s\-()]+$/.test(value); + return /^[+\d\s\-()]+$/.test(value) && /\d/.test(value); } )web/sdk/react/views-new/auth/components/magic-link-form.tsx (2)
19-22:childrenprop is declared but never used.The
MagicLinkFormPropstype declareschildren?: ReactNode, but the component never renders or references it. Either remove the unused prop or implement its intended usage.🔧 Remove unused prop
type MagicLinkFormProps = { open?: boolean; - children?: ReactNode; };
74-77: Usewindow.location.hrefinstead of@ts-ignore.Assigning directly to
window.locationrequires a@ts-ignorebecause TypeScript expects aLocationobject. Usewindow.location.hreffor a cleaner redirect without suppressing type errors.🔧 Proposed fix
- // `@ts-ignore` - window.location = `${ + window.location.href = `${ config.redirectMagicLinkVerify }?${searchParams.toString()}`;web/sdk/react/client.ts (1)
1-2: Consider CSS import order: normalize before style.Typically, CSS normalization/reset should be imported before component styles to ensure consistent base styling. Consider swapping the order so
normalize.cssloads first.🔧 Suggested order
-import '@raystack/apsara-v1/style.css'; -import '@raystack/apsara-v1/normalize.css'; +import '@raystack/apsara-v1/normalize.css'; +import '@raystack/apsara-v1/style.css';
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e96a7534-b936-4b12-aa33-f7c0f1ae26cb
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
web/apps/client-demo/package.jsonweb/apps/client-demo/src/App.tsxweb/apps/client-demo/src/config/frontier.tsweb/apps/client-demo/src/contexts/auth/provider.tsxweb/apps/client-demo/src/pages/Home.tsxweb/apps/client-demo/src/pages/Login.tsxweb/apps/client-demo/src/pages/MagiclinkVerify.tsxweb/apps/client-demo/src/pages/Settings.tsxweb/apps/client-demo/src/pages/Signup.tsxweb/apps/client-demo/src/pages/Subscribe.tsxweb/apps/client-demo/src/pages/Updates.tsxweb/apps/client-demo/src/pages/settings/Billing.tsxweb/apps/client-demo/src/pages/settings/General.tsxweb/apps/client-demo/src/pages/settings/Members.tsxweb/apps/client-demo/src/pages/settings/PatDetails.tsxweb/apps/client-demo/src/pages/settings/Pats.tsxweb/apps/client-demo/src/pages/settings/Plans.tsxweb/apps/client-demo/src/pages/settings/Preferences.tsxweb/apps/client-demo/src/pages/settings/Profile.tsxweb/apps/client-demo/src/pages/settings/ProjectDetails.tsxweb/apps/client-demo/src/pages/settings/Projects.tsxweb/apps/client-demo/src/pages/settings/Security.tsxweb/apps/client-demo/src/pages/settings/ServiceAccountDetails.tsxweb/apps/client-demo/src/pages/settings/ServiceAccounts.tsxweb/apps/client-demo/src/pages/settings/Sessions.tsxweb/apps/client-demo/src/pages/settings/TeamDetails.tsxweb/apps/client-demo/src/pages/settings/Teams.tsxweb/apps/client-demo/src/pages/settings/Tokens.tsxweb/sdk/package.jsonweb/sdk/react/client.tsweb/sdk/react/contexts/FrontierProvider.tsxweb/sdk/react/index.tsweb/sdk/react/views-new/auth/components/auth-container.module.cssweb/sdk/react/views-new/auth/components/auth-container.tsxweb/sdk/react/views-new/auth/components/auth-header.module.cssweb/sdk/react/views-new/auth/components/auth-header.tsxweb/sdk/react/views-new/auth/components/magic-link-form.module.cssweb/sdk/react/views-new/auth/components/magic-link-form.tsxweb/sdk/react/views-new/auth/components/oidc-button.module.cssweb/sdk/react/views-new/auth/components/oidc-button.tsxweb/sdk/react/views-new/auth/index.tsweb/sdk/react/views-new/auth/magic-link-verify/magic-link-verify-view.module.cssweb/sdk/react/views-new/auth/magic-link-verify/magic-link-verify-view.tsxweb/sdk/react/views-new/auth/sign-in/sign-in-view.module.cssweb/sdk/react/views-new/auth/sign-in/sign-in-view.tsxweb/sdk/react/views-new/auth/sign-up/sign-up-view.module.cssweb/sdk/react/views-new/auth/sign-up/sign-up-view.tsxweb/sdk/react/views-new/auth/subscribe/subscribe-view.module.cssweb/sdk/react/views-new/auth/subscribe/subscribe-view.tsxweb/sdk/react/views-new/auth/updates/updates-view.module.cssweb/sdk/react/views-new/auth/updates/updates-view.tsxweb/sdk/tsup.config.ts
💤 Files with no reviewable changes (1)
- web/sdk/react/index.ts
Coverage Report for CI Build 25158355293Coverage remained the same at 41.963%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b40236b-eff8-4705-a6f5-5dfa72ff4bc8
📒 Files selected for processing (25)
web/sdk/react/client.tsweb/sdk/react/components/auth-container/auth-container.module.cssweb/sdk/react/components/auth-container/auth-container.tsxweb/sdk/react/components/auth-container/index.tsweb/sdk/react/components/auth-header/auth-header.module.cssweb/sdk/react/components/auth-header/auth-header.tsxweb/sdk/react/components/auth-header/index.tsweb/sdk/react/components/auth-oidc-button/auth-oidc-button.module.cssweb/sdk/react/components/auth-oidc-button/auth-oidc-button.tsxweb/sdk/react/components/auth-oidc-button/index.tsweb/sdk/react/contexts/FrontierProvider.tsxweb/sdk/react/index.tsweb/sdk/react/views-new/auth/magic-link-verify/index.tsweb/sdk/react/views-new/auth/magic-link-verify/magic-link-verify-view.tsxweb/sdk/react/views-new/auth/magic-link/index.tsweb/sdk/react/views-new/auth/magic-link/magic-link-view.module.cssweb/sdk/react/views-new/auth/magic-link/magic-link-view.tsxweb/sdk/react/views-new/auth/sign-in/index.tsweb/sdk/react/views-new/auth/sign-in/sign-in-view.tsxweb/sdk/react/views-new/auth/sign-up/index.tsweb/sdk/react/views-new/auth/sign-up/sign-up-view.tsxweb/sdk/react/views-new/auth/subscribe/index.tsweb/sdk/react/views-new/auth/updates/index.tsweb/sdk/react/views-new/auth/updates/updates-view.tsxweb/sdk/shared/types.ts
💤 Files with no reviewable changes (1)
- web/sdk/react/index.ts
✅ Files skipped from review due to trivial changes (11)
- web/sdk/react/components/auth-oidc-button/auth-oidc-button.module.css
- web/sdk/react/views-new/auth/sign-in/index.ts
- web/sdk/react/components/auth-container/index.ts
- web/sdk/react/components/auth-container/auth-container.module.css
- web/sdk/react/components/auth-header/index.ts
- web/sdk/react/views-new/auth/updates/index.ts
- web/sdk/react/views-new/auth/magic-link/index.ts
- web/sdk/react/components/auth-header/auth-header.module.css
- web/sdk/react/views-new/auth/magic-link/magic-link-view.module.css
- web/sdk/react/views-new/auth/sign-up/index.ts
- web/sdk/react/components/auth-oidc-button/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/sdk/react/views-new/auth/sign-up/sign-up-view.tsx
- web/sdk/react/views-new/auth/sign-in/sign-in-view.tsx
- web/sdk/react/views-new/auth/magic-link-verify/magic-link-verify-view.tsx
- web/sdk/react/contexts/FrontierProvider.tsx
3 participants
Summary
views-new/auth, organized as per-view folders with a sharedauth/components/directory@raystack/frontier/clientsubpath export for v1-only views/components, with separate tsup entries to keep/react(legacy) and/client(new) bundles fully independentviews-new/*and v1 component exports out ofreact/index.tsso the legacy entry only exposes the old API/client, and to use Apsara v1 directly (no alias) — includingDropdownMenu→Menu,toast→toastManager, and addingToast.Providerwindow.locationfor redirects) so they work outside any router context