Refactor branding preference fetching logic#406
Merged
Conversation
kavindadimuthu
force-pushed
the
fix/branding-issue
branch
2 times, most recently
from
ac11887 to
45dafe3
Compare
brionmario
reviewed
Mar 25, 2026
brionmario
approved these changes
Mar 26, 2026
kavindadimuthu
force-pushed
the
fix/branding-issue
branch
from
8f71e34 to
1d26f79
Compare
kavindadimuthu
force-pushed
the
fix/branding-issue
branch
from
1d26f79 to
f0cf50d
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR makes branding an explicit opt-in (default false), updates the JSDoc and changeset to reflect that default, and adds richer warning logging when the branding-preference API call fails; the React provider is updated to only fetch/apply branding when opt-in is true. ChangesBranding Preference Default and Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🧹 Nitpick comments (1)
packages/javascript/src/api/getBrandingPreference.ts (1)
161-161: ⚡ Quick winRemove the
as anytype assertion.The type assertion bypasses TypeScript's type safety.
identifyPlatformexpects a config object with abaseUrlproperty, so you should pass a properly typed object:♻️ Proposed fix
- const platform: Platform = identifyPlatform({baseUrl} as any); + const platform: Platform = identifyPlatform({baseUrl});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/javascript/src/api/getBrandingPreference.ts` at line 161, The code uses an unsafe cast when calling identifyPlatform; remove the "as any" and pass a properly typed config object instead—e.g., construct an object with the baseUrl property typed to the expected config interface and call identifyPlatform(config) so the platform variable is assigned via identifyPlatform({ baseUrl }) without any type assertion; update any surrounding variable or parameter types so baseUrl matches the identifyPlatform config type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/src/api/getBrandingPreference.ts`:
- Around line 183-185: The logger.warn call in getBrandingPreference.ts contains
React-specific guidance referencing `<AsgardeoProvider>`; update the message to
be framework-agnostic by removing the component reference and instead
instructing users to update the SDK configuration (e.g., set
preferences.theme.inheritFromBranding to false) or consult the
platformConsoleGuidance; modify the string built with errorDescription and
platformConsoleGuidance so it reads something like "To stop fetching branding
preferences, set preferences.theme.inheritFromBranding to false in your SDK
configuration" while keeping the existing errorDescription and
platformConsoleGuidance variables.
---
Nitpick comments:
In `@packages/javascript/src/api/getBrandingPreference.ts`:
- Line 161: The code uses an unsafe cast when calling identifyPlatform; remove
the "as any" and pass a properly typed config object instead—e.g., construct an
object with the baseUrl property typed to the expected config interface and call
identifyPlatform(config) so the platform variable is assigned via
identifyPlatform({ baseUrl }) without any type assertion; update any surrounding
variable or parameter types so baseUrl matches the identifyPlatform config type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26aa7525-73cd-4f21-88bf-863c010a94a1
📒 Files selected for processing (4)
.changeset/cold-games-lead.mdpackages/javascript/src/api/getBrandingPreference.tspackages/javascript/src/models/config.tspackages/react/src/contexts/Asgardeo/AsgardeoProvider.tsx
Comment on lines
+183
to
+185
| logger.warn( | ||
| `[BrandingError] ${errorDescription} To resolve this issue, please ${platformConsoleGuidance}. If you want to suppress this warning and stop fetching branding preferences, set \`<AsgardeoProvider>\` -> \`preferences\` -> \`theme\` -> \`inheritFromBranding\` to false.`, | ||
| ); |
There was a problem hiding this comment.
Avoid React-specific guidance in the JavaScript package.
The warning message references <AsgardeoProvider>, which is a React-specific component. Since this is in the @asgardeo/javascript package (not @asgardeo/react), the guidance could be confusing or incorrect for developers using this SDK in non-React contexts (vanilla JS, Vue, Angular, etc.).
Consider making the guidance framework-agnostic:
💡 Proposed fix to use framework-agnostic guidance
logger.warn(
- `[BrandingError] ${errorDescription} To resolve this issue, please ${platformConsoleGuidance}. If you want to suppress this warning and stop fetching branding preferences, set \`<AsgardeoProvider>\` -> \`preferences\` -> \`theme\` -> \`inheritFromBranding\` to false.`,
+ `[BrandingError] ${errorDescription} To resolve this issue, please ${platformConsoleGuidance}. If you want to suppress this warning and stop fetching branding preferences, set the \`preferences.theme.inheritFromBranding\` configuration option to false.`,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.warn( | |
| `[BrandingError] ${errorDescription} To resolve this issue, please ${platformConsoleGuidance}. If you want to suppress this warning and stop fetching branding preferences, set \`<AsgardeoProvider>\` -> \`preferences\` -> \`theme\` -> \`inheritFromBranding\` to false.`, | |
| ); | |
| logger.warn( | |
| `[BrandingError] ${errorDescription} To resolve this issue, please ${platformConsoleGuidance}. If you want to suppress this warning and stop fetching branding preferences, set the \`preferences.theme.inheritFromBranding\` configuration option to false.`, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/javascript/src/api/getBrandingPreference.ts` around lines 183 - 185,
The logger.warn call in getBrandingPreference.ts contains React-specific
guidance referencing `<AsgardeoProvider>`; update the message to be
framework-agnostic by removing the component reference and instead instructing
users to update the SDK configuration (e.g., set
preferences.theme.inheritFromBranding to false) or consult the
platformConsoleGuidance; modify the string built with errorDescription and
platformConsoleGuidance so it reads something like "To stop fetching branding
preferences, set preferences.theme.inheritFromBranding to false in your SDK
configuration" while keeping the existing errorDescription and
platformConsoleGuidance variables.
kavindadimuthu
force-pushed
the
fix/branding-issue
branch
from
f0cf50d to
118e96e
Compare
🦋 Changeset detectedThe changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. |
brionmario
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request updates the branding preferences logic in the SDK to ensure branding is only fetched and applied when explicitly enabled, rather than by default. The changes clarify the documentation and adjust the code to require
inheritFromBrandingto be set totruefor branding to be fetched.Branding preferences logic update:
inheritFromBrandingproperty in theThemePreferencesinterface to clarify that branding is only fetched when explicitly enabled, and defaults tofalse.AsgardeoProviderto only fetch branding whenpreferences.theme.inheritFromBrandingistrue, rather than when it is notfalse.enabledprop for branding inAsgardeoProviderto requirepreferences.theme.inheritFromBrandingto betrue.Related Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit
Documentation
Improvements
Chores