Conversation
📝 WalkthroughWalkthroughCentralizes color and style derivation into an enhanced useStyle hook (now receiving component props), removes useUniwindAccent, and adds data-attribute selector support across style resolution and build-time metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useStyle
participant UniwindStore
participant Resolver
Component->>useStyle: call useStyle(className, props, state)
useStyle->>UniwindStore: getStyles(className, props, state)
UniwindStore->>Resolver: resolveStyles(classNames, componentProps, state)
Resolver->>Resolver: parse rules, evaluate selectors (including dataAttributes)
alt dataAttributes mismatch
Resolver-->>UniwindStore: skip style
else match or no dataAttributes
Resolver-->>UniwindStore: include style, accumulate dependencySum & hasDataAttributes
end
UniwindStore-->>useStyle: StylesResult (styles, dependencies, dependencySum, hasDataAttributes)
useStyle-->>Component: computed style (includes accentColor)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 4
🤖 Fix all issues with AI agents
In `@packages/uniwind/src/core/native/store.ts`:
- Around line 10-16: The StylesResult type is missing the hasDataAttributes
field which resolveStyles returns and callers (e.g., code reading emptyState)
expect; update the StylesResult type to include hasDataAttributes: boolean and
then add hasDataAttributes: false to the emptyState constant so the shape
matches resolveStyles and consumers can safely access that property. Ensure you
update the type declaration named StylesResult and the constant emptyState
accordingly.
- Around line 238-256: The validateDataAttributes function returns early when a
boolean attribute matches, skipping remaining checks; change the logic in
validateDataAttributes to avoid returning true inside the loop: for each
attribute (in function validateDataAttributes) evaluate boolean cases and only
return false when a mismatch is detected, otherwise continue to the next
attribute, and after the loop return true; ensure the checks for
expectedAttributeValue === 'true' and 'false' return false on mismatch (not true
on match) and do not prematurely exit the loop when matched.
In `@packages/uniwind/src/core/types.ts`:
- Line 20: The TypeScript type for Style currently forces the dataAttributes
property (even if nullable) which can break downstream consumers; update the
Style definition to make dataAttributes optional (change the declaration for
dataAttributes on the Style type to be optional, e.g. dataAttributes?:
Record<string,string> | null) so callers that omit it won’t error, and if this
is a public API change ensure you document or version-bump accordingly; locate
the Style type in the file where dataAttributes is declared in
packages/uniwind/src/core/types.ts and adjust that property.
In `@packages/uniwind/src/metro/processor/processor.ts`:
- Around line 181-185: The code stores attribute values with surrounding quotes
causing mismatches in validation; in the block handling selector.type ===
'attribute' and selector.operation?.operator === 'equal' (inside the processor
function where dataAttributes is set), stop wrapping selector.operation.value in
quotes and assign the raw value instead (e.g., dataAttributes[selector.name] =
selector.operation.value) so it matches what validateDataAttributes() in
store.ts expects when comparing boolean/string props.
🧹 Nitpick comments (2)
packages/uniwind/src/components/native/Pressable.tsx (1)
18-31: Consider extracting the state object to reduce duplication.The state object
{ isDisabled: Boolean(props.disabled), isPressed: true }partially duplicates the one created at lines 10-12. Minor improvement would be to extract the base state.♻️ Optional refactor
export const Pressable = copyComponentProperties(RNPressable, (props: PressableProps) => { + const baseState = { isDisabled: Boolean(props.disabled) } const style = useStyle( props.className, props, - { - isDisabled: Boolean(props.disabled), - }, + baseState, ) return ( <RNPressable {...props} style={state => { if (state.pressed) { return [ UniwindStore.getStyles( props.className, props, - { isDisabled: Boolean(props.disabled), isPressed: true }, + { ...baseState, isPressed: true }, ).styles, typeof props.style === 'function' ? props.style(state) : props.style, ] } return [style, typeof props.style === 'function' ? props.style(state) : props.style] }} /> ) })packages/uniwind/src/metro/processor/processor.ts (1)
182-185: Only theequaloperator is supported for attribute selectors.Other CSS attribute selector operators (
~=,|=,^=,$=,*=) are not handled. If this is intentional (data attributes typically use exact matching), consider adding a brief comment for clarity.📝 Optional: Add clarifying comment
- if (selector.type === 'attribute' && selector.operation?.operator === 'equal') { + // Only exact-match attribute selectors are supported (e.g., [data-active="true"]) + if (selector.type === 'attribute' && selector.operation?.operator === 'equal') { dataAttributes ??= {} dataAttributes[selector.name] = `"${selector.operation.value}"` }
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/uniwind/src/core/native/store.ts (1)
86-124: Add a guard to preventdependencySumbit-shift overflow ifStyleDependencyenum grows.Currently safe with enum values 1–9, but
dependencySum |= 1 << depwill silently collide if any enum value reaches 31+ (causing shifts beyond the 32-bit boundary). Since different dependency sets would then map to the same sum, the React dependency array wouldn't detect changes, breaking re-subscriptions.Consider either:
- Asserting that
StyleDependencyvalues stay ≤ 30- Switching to a stable string key (e.g., bitmask-based hash or Set serialization) if further growth is planned
Also applies to: 229–234
This was referenced Feb 10, 2026
Merged
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
1 participant
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.
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.