Refactor Knob component styles for performance - #5416
MaddipatlaChetan24 wants to merge 1 commit into
Conversation
Refactor styles in Knob component to use static objects instead of inline styles, improving performance by avoiding unnecessary re-creation on each render.
|
can you please check this !! |
|
Thanks for the PR. Before we review it, please note that "check this" isn't a submission we can accept. It hands the verification work to maintainers, and our review time is limited. We welcome AI-assisted contributions, but you are the author and must stand behind every line. Before requesting review, please: Run it. Set up the dev environment, run the full test suite, and confirm the change works as intended. Once you can confirm all of the above in the PR description, we're happy to take a look. Until then, we'll mark this as a draft. |
Fixes #1, Fixes #2
Description
Refactors two style overrides in the
Knobcomponent to use static,module-level objects instead of inline object/function literals that
were being re-created on every render.
RadioGroupRoot's style was written as a function(
({ $theme }) => ({...})) but never actually referenced$themeinits return value — it was a constant disguised as a function. Replaced
with a plain module-level
RADIO_GROUP_ROOT_OVERRIDE_STYLEobject,which baseui's
overrides.<Component>.styleaccepts directly. Thisremoves both the per-render function allocation and the function call
baseui previously had to make to resolve it.
Checkbox'sLabeloverride (style: { fontWeight: 500 }) wasalready a static shape but was being re-allocated as a new object
literal on every render. Hoisted to a module-level
CHECKBOX_LABEL_OVERRIDE_STYLEconstant so the same object referenceis reused across renders.
Both changes are behavior-identical — same resolved styles, same visual
output — just without the unnecessary re-allocation.
Left the two per-
Radiooverrides inside the enum options.map()unchanged. They do use
$themeand can't become plain objects, andwhile they could in principle also be hoisted to module scope, doing so
risks losing baseui's contextual type inference for the
$themeparameter (which is currently inferred from the JSX prop's expected
type) and could introduce a type error without knowing this project's
exact
tsconfig/baseui override type setup. Flagging as a possiblefollow-up rather than guessing at a fix that might not compile.
Scope
Patch: Bug Fix