fix(clerk-js): CSS Variables #6100
Conversation
🦋 Changeset detectedLatest commit: 94cb7b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis change refactors the Clerk JS UI theming system to use CSS custom properties (variables) for colors, spacing, typography, opacity, and borders. It introduces new color manipulation utilities, replaces hardcoded values with dynamic CSS variables, and updates components to use new color mixing functions. Several obsolete utilities are removed, and a new changeset is added. Changes
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tmilewski
force-pushed
the
alexcarpenter/color-mix
branch
from
a0006a9 to
fedf717
Compare
tmilewski
force-pushed
the
alexcarpenter/color-mix
branch
from
fedf717 to
de26343
Compare
…n and update related functions to use it
tmilewski
force-pushed
the
alexcarpenter/color-mix
branch
from
de26343 to
f7ebfcc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx (1)
167-173: 💡 Verification agent🧩 Analysis chain
backgroundColorusescolor-mixwithout a fallback – older browsers will render a solid grayThe OKLCH
color-mix()function is still behind flags in Safari ≤16 and absent in Firefox ≤113/Chromium ≤111. If it fails, the browser will ignorebackgroundColorand fall back totransparent, leaving the card visually broken.sx={t => ({ - backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), + /* Fallback for browsers without color-mix */ + background: t.colors.$colorBackground, + /* Modern browsers */ + backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), })}Consider adding a caniuse comment or a feature-query (
@supports (background-color: color-mix(in oklch, black, white)) { … }) to progressively enhance.
🌐 Web query:
CSS color-mix() browser compatibility caniuse💡 Result:
The CSS
color-mix()function, which allows blending two colors in a specified color space, has been supported in all modern browsers since mid-2023. (developer.mozilla.org) According to Can I Use, the global usage ofcolor-mix()is approximately 91.98%. (caniuse.com)Here's a summary of browser support:
- Chrome: Supported from version 111 onwards.
- Firefox: Supported from version 113 onwards.
- Safari: Supported from version 16.2 onwards.
- Edge: Supported from version 111 onwards.
- Opera: Supported from version 97 onwards.
For browsers that do not support
color-mix(), it's advisable to provide fallback colors to ensure consistent styling across all platforms. (developer.mozilla.org)Citations:
- 1: https://developer.mozilla.org/en-US/blog/color-palettes-css-color-mix/?utm_source=openai
- 2: https://caniuse.com/mdn-css_types_color_color-mix?utm_source=openai
- 3: https://developer.mozilla.org/en-US/blog/color-palettes-css-color-mix/?utm_source=openai
Add a solid‐color fallback for
color-mixThe CSS
color-mix()function is supported in most browsers (Chrome ≥ 111, Firefox ≥ 113, Safari ≥ 16.2)[1][2], but older versions will ignorebackgroundColorand fall back totransparent, breaking the card’s styling. Please add a legacy fallback and/or feature query:sx={t => ({ + /* Fallback for unsupported browsers */ + background: t.colors.$colorBackground, /* Modern browsers */ backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), })}Optional enhancement using a feature query:
@supports (background-color: color-mix(in oklch, var(--color-background), var(--neutral-alpha-50))) { /* Use mixed background here */ }Citations:
♻️ Duplicate comments (5)
packages/clerk-js/src/ui/elements/Card/CardFooter.tsx (1)
54-55: Samecolor-mixfallback concern as inPopoverCard– see previous comment for a drop-in snippet.packages/clerk-js/src/ui/elements/Disclosure.tsx (1)
170-171:color-mixfallback – same cross-browser concern applies here.packages/clerk-js/src/ui/elements/Navbar.tsx (1)
150-153: 🛠️ Refactor suggestionSame
color-mixsupport concern
backgroundColor: colorMix(...)has the same browser-support caveat mentioned for the PricingTable card. Add a solid-color fallback or a feature query.No change required here if you address it globally.
packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx (1)
57-60: Add fallback for unsupportedcolor-mixSame compatibility story as earlier – guards or fallbacks will avoid washed-out highlight rows on unsupported browsers.
packages/clerk-js/src/ui/elements/Drawer.tsx (1)
300-303: Header background – see earlier comment oncolor-mixA single fallback declaration will keep header styling consistent with the body even when the browser drops
color-mix.
🧹 Nitpick comments (16)
packages/clerk-js/src/ui/utils/css.ts (1)
1-1: Validate variable names to guard against malformed CSS/injection
clerkCssVarblindly interpolates whatever string it receives. A stray space, semicolon, or malicious payload would yield invalid CSS or could even leak into additional declarations (var(--clerk-foo);background:red)).
Lightweight runtime validation in dev-mode keeps production fast while catching mistakes early.-export const clerkCssVar = (name: string, defaultValue: string) => `var(--clerk-${name}, ${defaultValue})`; +export const clerkCssVar = (name: string, defaultValue: string) => { + if (process.env.NODE_ENV !== 'production' && !/^[a-z0-9-]+$/i.test(name)) { + console.warn(`[clerkCssVar] Invalid CSS variable name: "${name}"`); + } + return `var(--clerk-${name}, ${defaultValue})`; +};packages/clerk-js/src/ui/foundations/opacity.ts (1)
4-6: Expose these tokens to consumers and reuse them across componentsThe literals
'24%' | '50%' | '62%'are now the single source of truth for opacity. Ensure downstream components (e.g.Th.tsx) referenceopacity.inactiveinstead of re-hard-coding'62%'so theme tweaks propagate automatically and avoid silent drift.
No action needed here, just flagging for follow-up.packages/clerk-js/src/ui/primitives/Th.tsx (1)
14-14: Reuse the opacity token instead of hard-coding'62%'To keep visual consistency with the new
opacity.inactivetoken:- color: transparentize(theme.colors.$colorText, '62%'), + color: transparentize(theme.colors.$colorText, theme.opacity?.$inactive ?? '62%'),(if
theme.opacityisn’t yet part of the styled-system typings, consider exporting it similarly tofontSizes/spaceso consumers can rely on the same source of truth).packages/clerk-js/src/ui/elements/PopoverCard.tsx (1)
84-85:color-mix()lacks full browser support – add a graceful fallbackUsing
colorMix()directly is clean, but Safari ≤16.3 and all Firefox versions behind a flag will ignore the rule and render the raw token. Consider a@supportsguard or a fallback colour:- backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), + backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), + '@supports not (color: color-mix(in oklch, #000, #fff))': { + backgroundColor: t.colors.$neutralAlpha50, + },packages/clerk-js/src/ui/elements/Card/CardFooter.tsx (1)
3-4: Path alias vs relative import – be consistentHere we import
colorMixvia@/ui/utils/colorMix, while other files use relative paths (../utils/colorMix). In large monorepos mixed styles often break IDE resolution or Jest module-mapper configs. Standardise on one form in all UI packages.packages/clerk-js/sandbox/template.html (1)
10-30: Nice baseline variables – but move them to a dedicated CSS fileEmbedding a 120-line variable sheet directly in the HTML bloats the template and prevents browser caching. A standalone
base.css(or the existing tailwind entry point) keeps concerns separated and will be cached by the CDN.No functional bug, just maintainability.
packages/clerk-js/src/ui/elements/Navbar.tsx (1)
320-322: Consistency:backgroundColorvsbackgroundIn
PricingTableMatrixyou usebackground, here you usebackgroundColor. Prefer one style across components to avoid diff noise and make future refactors simpler.packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx (1)
57-60: UsebackgroundColorinstead ofbackgroundunless you intend to support multiple layersYou only supply a single color value, so
backgroundColoris semantically clearer and avoids overriding other background layers (e.g. images) that a consumer might inject.-const highlightBackgroundColor: ThemableCssProp = t => ({ - background: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha25), +const highlightBackgroundColor: ThemableCssProp = t => ({ + backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha25), })packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (1)
330-333: Same compatibility caveat applies toConnectionIconConsider a similar fallback strategy for the icon’s background to avoid it rendering transparent on browsers lacking
color-mixsupport.packages/clerk-js/src/ui/foundations/sizes.ts (1)
9-11: Numeric dance incalcDynamicSpaceUnitsis hard to follow
((${num} / 0.5) * 0.125)reduces tonum * 0.25. Dropping the noise improves readability and avoids operator-precedence mistakes:-const calcDynamicSpaceUnits = (num: number) => `calc(${dynamicSpaceUnitsDefaultVar} * ((${num} / 0.5) * 0.125))`; +const calcDynamicSpaceUnits = (num: number) => `calc(${dynamicSpaceUnitsDefaultVar} * ${num * 0.25})`;Same byte size after minification, much clearer at review time.
packages/clerk-js/src/ui/elements/Drawer.tsx (2)
152-156: Backdrop relies oncolor-mix()– add solid fallbackSame browser-support caveat: if
transparentizeyieldscolor-mix(), the whole backdrop might go missing. Layer an opaquergba()fallback underneath:backgroundColor: [ transparentize(t.colors.$colorBackground, '28%'), 'rgba(0,0,0,0.28)', ].join(',')
517-518: Confirmation sheet background – same compatibility issueApply the same fallback strategy here to avoid inconsistent UI.
packages/clerk-js/src/ui/customizables/parseVariables.ts (1)
48-49: Secondary text default depends oncolor-mix– provide legacy rgba pathIf
colorTextSecondaryis auto-derived, browsers withoutcolor-mixwill inheritinherit. Pre-compute an HSLA fallback or document min-browser requirements.packages/clerk-js/src/ui/foundations/colors.ts (1)
76-78: Use generated scale instead of ad-hoc lightening forprimaryHover.
primaryHoveris derived withlighten(..., '25%'), but there’s already a full lightness scale (primary600,primary700, …) generated right above. Picking a shade from that scale (e.g.primary600) keeps the hover colour in sync with any customprimaryoverrides and avoids the TODO.- primaryHover: lighten(clerkCssVar('primary', '#2F3037'), '25%'), + // Hover = one step lighter than 500 + primaryHover: primaryScale.primary600,packages/clerk-js/src/ui/utils/colorMix.ts (2)
41-49: Doc-string is out of sync with implementation.The JSDoc still lists
percentage/colorTintparams, butcolorMixnow accepts just two colours. Update the comment to prevent confusion for consumers and IDE tooling.
57-60: Consider validating the percentage argument.
transparentize(andlighten/darken) rely on a free-formPercentagestring. A quick runtime guard (0–100 %, ends with%) would fail fast on typos like'105%'or'25'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.changeset/warm-pens-tickle.md(1 hunks)packages/clerk-js/sandbox/app.ts(1 hunks)packages/clerk-js/sandbox/template.html(1 hunks)packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx(3 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx(3 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx(3 hunks)packages/clerk-js/src/ui/customizables/parseVariables.ts(2 hunks)packages/clerk-js/src/ui/elements/Card/CardFooter.tsx(2 hunks)packages/clerk-js/src/ui/elements/Disclosure.tsx(2 hunks)packages/clerk-js/src/ui/elements/Drawer.tsx(6 hunks)packages/clerk-js/src/ui/elements/Navbar.tsx(3 hunks)packages/clerk-js/src/ui/elements/PopoverCard.tsx(2 hunks)packages/clerk-js/src/ui/foundations/borders.ts(1 hunks)packages/clerk-js/src/ui/foundations/colors.ts(1 hunks)packages/clerk-js/src/ui/foundations/opacity.ts(1 hunks)packages/clerk-js/src/ui/foundations/sizes.ts(2 hunks)packages/clerk-js/src/ui/foundations/typography.ts(1 hunks)packages/clerk-js/src/ui/primitives/Badge.tsx(0 hunks)packages/clerk-js/src/ui/primitives/Th.tsx(2 hunks)packages/clerk-js/src/ui/styledSystem/common.ts(1 hunks)packages/clerk-js/src/ui/utils/colorMix.ts(1 hunks)packages/clerk-js/src/ui/utils/colors.ts(1 hunks)packages/clerk-js/src/ui/utils/css.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/primitives/Badge.tsx
🧰 Additional context used
🧬 Code Graph Analysis (14)
packages/clerk-js/src/ui/elements/Card/CardFooter.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/elements/PopoverCard.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/primitives/Th.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
transparentize(57-59)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/elements/Drawer.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (2)
transparentize(57-59)colorMix(47-49)
packages/clerk-js/src/ui/foundations/typography.ts (1)
packages/clerk-js/src/ui/utils/css.ts (1)
clerkCssVar(1-1)
packages/clerk-js/src/ui/elements/Disclosure.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/foundations/opacity.ts (1)
packages/clerk-js/src/ui/utils/css.ts (1)
clerkCssVar(1-1)
packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/foundations/sizes.ts (1)
packages/clerk-js/src/ui/utils/css.ts (1)
clerkCssVar(1-1)
packages/clerk-js/src/ui/elements/Navbar.tsx (1)
packages/clerk-js/src/ui/utils/colorMix.ts (1)
colorMix(47-49)
packages/clerk-js/src/ui/foundations/borders.ts (1)
packages/clerk-js/src/ui/utils/css.ts (1)
clerkCssVar(1-1)
packages/clerk-js/src/ui/customizables/parseVariables.ts (2)
packages/types/src/appearance.ts (1)
Theme(705-738)packages/clerk-js/src/ui/utils/colorMix.ts (4)
createColorMixLightnessScale(154-206)createAlphaScaleWithTransparentize(108-118)lighten(67-69)transparentize(57-59)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/clerk-js/sandbox/app.ts (1)
348-349: Removing the initialupdateVariables()call may regress saved-theme rehydrationPreviously this call applied variables stored in
sessionStorageright afterClerk.load().
With it commented out, users who refreshed the page will not see their saved colour/spacing overrides until they manually change a control, effectively breaking the sandbox UX.Please verify if this behaviour is intentional or gate the call behind a flag instead of removing it entirely.
packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx (1)
7-8: Import looks good, but keep an eye on tree-shaking
colorMixis the only named import added from@/ui/utils/colorMix. The current file uses it once, so no dead-code concerns.packages/clerk-js/src/ui/styledSystem/common.ts (1)
101-105:toStringcast removal is safe if theme tokens are typed asstringIf
t.colors.$dangerAlpha200is alreadystring, the cast was redundant – 👍. Just ensure the theme type guarantees this; otherwise TypeScript may regress.packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx (1)
6-8: Import OK
colorMixreplaces the removed gradient helper. No issues.packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (1)
16-17: Import looks good but verify tree-shaking side-effects
colorMixis imported directly from@/ui/utils/colorMix; the util itself does not appear to have any side-effects, so no concerns here.packages/clerk-js/src/ui/foundations/typography.ts (1)
4-8: Nestedvar()fallbacks – check browser supportPatterns like
var(--clerk-font-weight-normal, var(--clerk-font-weight, 400))work in modern Chromium but only landed in Firefox 117 and are still unsupported in legacy Safari. If broad compatibility is a requirement, collapse to a single fallback.
No action needed if the target browsers are green-field.
packages/clerk-js/src/ui/foundations/sizes.ts (1)
72-81: Radii scale assumesborderRadiusnumeric – guard against keywordsIf a theme sets
borderRadius: "none", you already coerce to0, nice.
However keywords like"revert"or"unset"would pass through and break the math below.
Consider validatingnumericValuebefore multiplying or warn the integrator.packages/clerk-js/src/ui/elements/Drawer.tsx (1)
490-494: Gradient overlay mixes color strings – verify resulting CSS
linear-gradient(to bottom, ${transparentize(...)} , ${t.colors.$colorBackground})If
transparentizereturns acolor-mix()string, the gradient will embed another functional color, which is valid CSS but fails on browsers without support. Confirm visual degradation is acceptable or provide a simple rgba fallback.
| const borderWidthDefaultVar = clerkCssVar('border-width', '1px'); | ||
| export const borderWidths = Object.freeze({ | ||
| normal: '1px', | ||
| heavy: '2px', | ||
| normal: clerkCssVar('border-width-normal', borderWidthDefaultVar), | ||
| heavy: clerkCssVar('border-width-heavy', `calc(${borderWidthDefaultVar} * 2)`), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
heavy should derive from the normal width token
If a user overrides --clerk-border-width-normal, the current fallback keeps heavy tied to the base default (1px), not the overridden normal width. Deriving from the normal token keeps them in sync:
- heavy: clerkCssVar('border-width-heavy', `calc(${borderWidthDefaultVar} * 2)`),
+ heavy: clerkCssVar(
+ 'border-width-heavy',
+ `calc(${clerkCssVar('border-width-normal', borderWidthDefaultVar)} * 2)`
+ ),📝 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.
| const borderWidthDefaultVar = clerkCssVar('border-width', '1px'); | |
| export const borderWidths = Object.freeze({ | |
| normal: '1px', | |
| heavy: '2px', | |
| normal: clerkCssVar('border-width-normal', borderWidthDefaultVar), | |
| heavy: clerkCssVar('border-width-heavy', `calc(${borderWidthDefaultVar} * 2)`), | |
| }); | |
| const borderWidthDefaultVar = clerkCssVar('border-width', '1px'); | |
| export const borderWidths = Object.freeze({ | |
| normal: clerkCssVar('border-width-normal', borderWidthDefaultVar), | |
| heavy: clerkCssVar( | |
| 'border-width-heavy', | |
| `calc(${clerkCssVar('border-width-normal', borderWidthDefaultVar)} * 2)` | |
| ), | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/foundations/borders.ts around lines 8 to 12, the
'heavy' border width is currently calculated using the base default border width
instead of the 'normal' border width token. To fix this, update the 'heavy'
value to derive from the 'normal' token variable so that if the user overrides
'--clerk-border-width-normal', the 'heavy' width stays in sync by multiplying
the 'normal' token by 2.
| --- | ||
| '@clerk/clerk-js': minor | ||
| --- | ||
|
|
||
| [TODO] |
There was a problem hiding this comment.
Populate the changeset before merging
[TODO] will reach the changelog verbatim, leaving consumers without any context for the minor release. Replace it with a concise summary of the CSS-variable migration and colour-utility refactor.
🤖 Prompt for AI Agents
In the file .changeset/warm-pens-tickle.md at lines 1 to 5, replace the
placeholder text "[TODO]" with a concise summary describing the CSS-variable
migration and colour-utility refactor. This summary should clearly explain the
changes made in this minor release to provide useful context for consumers in
the changelog.
| // TODO(Colors): This is a temporary fix to allow for custom colors to be passed in as variables | ||
| return str as unknown as ParsedResult; | ||
| const prefix = str.substr(0, 3).toLowerCase(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
parse now returns a plain string – downstream logic will crash
parse is expected to return {model, value} but now returns the input string cast to ParsedResult.
toHslaColor immediately destructures that object (line 282). At runtime model and value become undefined, so the switch on model falls through and returns undefined, breaking every helper (lighten, makeSolid, …) that relies on a valid HSLA colour.
- // TODO(Colors): This is a temporary fix to allow for custom colors to be passed in as variables
- return str as unknown as ParsedResult;
+ // Quick-exit for CSS variables *only*; keep original parser for real colours
+ if (str.trim().startsWith('var(')) {
+ /* Treat CSS variables as opaque – most callers will forward the string
+ to the style layer without numeric manipulation. */
+ // @ts-ignore – runtime consumers must guard before numeric ops
+ return {model: 'var', value: [0, 0, 0, 1]} as ParsedResult;
+ }
+ // Fallback to the original, validated parserIf numeric manipulation of a CSS variable is still attempted you’ll need a wider refactor, but the above prevents immediate runtime errors while preserving validation for literal colours.
📝 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.
| // TODO(Colors): This is a temporary fix to allow for custom colors to be passed in as variables | |
| return str as unknown as ParsedResult; | |
| const prefix = str.substr(0, 3).toLowerCase(); | |
| // Quick-exit for CSS variables *only*; keep original parser for real colours | |
| if (str.trim().startsWith('var(')) { | |
| /* Treat CSS variables as opaque – most callers will forward the string | |
| to the style layer without numeric manipulation. */ | |
| // @ts-ignore – runtime consumers must guard before numeric ops | |
| return {model: 'var', value: [0, 0, 0, 1]} as ParsedResult; | |
| } | |
| // Fallback to the original, validated parser | |
| const prefix = str.substr(0, 3).toLowerCase(); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 260-260: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/utils/colors.ts around lines 258 to 260, the parse
function incorrectly returns the input string cast as ParsedResult instead of an
object with model and value properties. This causes downstream code, such as
toHslaColor at line 282, to fail when destructuring. Fix this by returning an
object with the expected shape { model, value } from parse, ensuring model and
value are properly assigned to prevent runtime errors in color helper functions.
| sx={t => ({ | ||
| padding: t.space.$3, | ||
| background: common.mergedColorsBackground( | ||
| utils.colors.setAlpha(t.colors.$colorBackground, 1), | ||
| t.colors.$neutralAlpha50, | ||
| ), | ||
| backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), | ||
| })} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Color-mix may break on non-Chromium browsers – add fallback
backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50) generates a color-mix() expression, which is still behind flags in Safari 17 and completely unsupported in Firefox ≤ 126. Users on those browsers will get no background color.
backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50),
+backgroundColor: [
+ colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50),
+ t.colors.$neutralAlpha50, // fallback for browsers without color-mix
+].join(','),This keeps the new look while preserving graceful degradation.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for occurrences of `colorMix` in the codebase
grep -R "colorMix" -n .Length of output: 65620
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display first 200 lines of the colorMix utility to verify its output
sed -n '1,200p' packages/clerk-js/src/ui/utils/colorMix.tsLength of output: 5964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show lines 1-200 of the OAuthConsent.tsx for context
sed -n '1,200p' packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxLength of output: 7269
Add a solid fallback before using colorMix for unsupported browsers
To avoid an invalid background-color in browsers that don’t yet support color-mix(), declare the plain alpha background first, then override with your mixed color. For example, in packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx around line 123:
sx={t => ({
padding: t.space.$3,
- backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50),
+ // fallback for browsers without color-mix
+ backgroundColor: t.colors.$neutralAlpha50,
+ backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50),
})}Please apply the same pattern wherever colorMix(…, t.colors.$neutralAlpha50) (or other mixes) is used, e.g.:
• packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx
• packages/clerk-js/src/ui/components/PricingTable/PricingTableMatrix.tsx
• packages/clerk-js/src/ui/elements/Card/CardFooter.tsx
• packages/clerk-js/src/ui/elements/Disclosure.tsx
• packages/clerk-js/src/ui/elements/Drawer.tsx
• packages/clerk-js/src/ui/elements/Navbar.tsx
• packages/clerk-js/src/ui/elements/PopoverCard.tsx
Consider extracting a helper or theme token if this becomes a widespread pattern.
📝 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.
| sx={t => ({ | |
| padding: t.space.$3, | |
| background: common.mergedColorsBackground( | |
| utils.colors.setAlpha(t.colors.$colorBackground, 1), | |
| t.colors.$neutralAlpha50, | |
| ), | |
| backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), | |
| })} | |
| sx={t => ({ | |
| padding: t.space.$3, | |
| + // fallback for browsers without color-mix | |
| + backgroundColor: t.colors.$neutralAlpha50, | |
| backgroundColor: colorMix(t.colors.$colorBackground, t.colors.$neutralAlpha50), | |
| })} |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx around
lines 121 to 124, the backgroundColor uses colorMix which is unsupported in some
browsers. Fix this by first setting backgroundColor to the solid alpha color
t.colors.$neutralAlpha50 as a fallback, then override it with the colorMix
expression. Apply this pattern to ensure graceful degradation and consider
creating a helper or theme token if this pattern is used frequently across the
codebase.
There was a problem hiding this comment.
This package only supports browsers released in the last two years (since May 8 2023), so all of our supported browsers support color-mix().
There was a problem hiding this comment.
@dstaley Thank you for the clarification! I appreciate you letting me know about the package's browser support policy. That makes perfect sense - if you're only supporting browsers from the last two years, then color-mix() support is indeed guaranteed and no fallbacks are needed.
✏️ Learnings added
Learnt from: dstaley
PR: clerk/javascript#6100
File: packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx:121-124
Timestamp: 2025-06-16T17:08:58.363Z
Learning: The @clerk/clerk-js package only supports browsers released in the last two years (since May 8, 2023), so modern CSS features like color-mix() are fully supported across all target browsers without requiring fallbacks.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const lineHeightsDefaultVar = clerkCssVar('line-height', 'normal'); | ||
| const lineHeights = Object.freeze({ | ||
| normal: 'normal', | ||
| extraSmall: '1.33333', | ||
| small: '1.38462', | ||
| medium: '1.41176', | ||
| large: '1.45455', | ||
| normal: clerkCssVar('line-height-normal', lineHeightsDefaultVar), | ||
| extraSmall: clerkCssVar('line-height-xs', `calc(${lineHeightsDefaultVar} * 1.33333)`), // 1.33333 | ||
| small: clerkCssVar('line-height-sm', `calc(${lineHeightsDefaultVar} * 1.38462)`), // 1.38462 | ||
| medium: clerkCssVar('line-height-md', `calc(${lineHeightsDefaultVar} * 1.41176)`), // 1.41176 | ||
| large: clerkCssVar('line-height-lg', `calc(${lineHeightsDefaultVar} * 1.45455)`), // 1.45455 | ||
| } as const); |
There was a problem hiding this comment.
calc() with non-numeric fallback will be invalid CSS
lineHeightsDefaultVar falls back to normal. When the root variable is not set, expressions like
calc(var(--clerk-line-height, normal) * 1.33333) evaluate to calc(normal * 1.33333) → invalid.
Two options:
-const lineHeightsDefaultVar = clerkCssVar('line-height', 'normal');
+// Use numeric 1 as a safe multiplicative base when no custom value is supplied
+const lineHeightsDefaultVar = clerkCssVar('line-height', '1');or compute the multipliers in JS instead of CSS.
Without a fix, many text blocks will inherit line-height: normal rather than scaled values.
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/foundations/typography.ts around lines 10 to 17, the
use of calc() with a fallback of 'normal' in lineHeightsDefaultVar causes
invalid CSS when the root variable is unset. To fix this, replace the fallback
'normal' with a numeric value like '1' so that calc() always operates on
numbers, or alternatively, perform the line-height multiplications in JavaScript
before assigning the values to avoid invalid CSS expressions.
| primaryHover: primaryBase ? lighten(primaryBase, '90%') : undefined, | ||
| colorTextOnPrimaryBackground: variables.colorTextOnPrimaryBackground, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
lighten(..., '90%') hardly lightens
lighten(color, '90%') keeps 90 % of the base color and only 10 % white, so the result is barely lighter. If the intent was “90 % lighter”, flip the numbers:
-primaryHover: primaryBase ? lighten(primaryBase, '90%') : undefined,
+primaryHover: primaryBase ? lighten(primaryBase, '10%') : undefined,or clarify with a named helper (lightenBy(percentageOfWhite)) to avoid future confusion.
📝 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.
| primaryHover: primaryBase ? lighten(primaryBase, '90%') : undefined, | |
| colorTextOnPrimaryBackground: variables.colorTextOnPrimaryBackground, | |
| primaryHover: primaryBase ? lighten(primaryBase, '10%') : undefined, | |
| colorTextOnPrimaryBackground: variables.colorTextOnPrimaryBackground, |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/customizables/parseVariables.ts around lines 44 to
45, the lighten function is called with '90%' which results in only a slight
lightening because it retains 90% of the base color. To fix this, invert the
percentage to '10%' to achieve a 90% lighter color or refactor the code to use a
named helper function like lightenBy that clearly indicates the percentage of
white added, improving clarity and correctness.
| * @param colorTint - The color to mix the color with | ||
| * @returns The mixed color | ||
| */ | ||
| export function colorMix(colorOne: string, colorTwo: string) { |
There was a problem hiding this comment.
we could use CSS.supports("color", "color-mix(in srgb, red, blue)") to check for browser support, falling back to a JS implementation of color-mix in unsupported browsers.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores