Skip to content

refactor: replace computed with functions in styles#547

Merged
Brentlok merged 1 commit into
mainfrom
replace-computed-with-functions
May 22, 2026
Merged

refactor: replace computed with functions in styles#547
Brentlok merged 1 commit into
mainfrom
replace-computed-with-functions

Conversation

@Brentlok

@Brentlok Brentlok commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Fixed CSS variable cascading in inline style definitions to ensure variables are properly resolved before style application.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR refactors CSS variable resolution throughout the Uniwind bundler and runtime, shifting from lazy Object.defineProperty getters and this[...] property access to eager function-based evaluation where variables are invoked as Var(vars) functions. Changes span type definitions, CSS processors, the runtime store, native parsers, and supporting utilities.

Changes

Variable Resolution Function Refactoring

Layer / File(s) Summary
Variable type contracts
src/core/types.ts
Introduces Vars and Var types representing variable resolver functions. Updates Style.entries and GenerateStyleSheetsCallback return shape to use typed variable functions instead of untyped records.
CSS processor variable resolution
src/bundler/css-processor/{color,css,units,var}.ts, src/bundler/css-processor/{utils,addMetaToStylesTemplate}.ts
Updates Color.processColor, CSS.getProcessedValue, Units.processLength, and processVar to reference variables via vars[...] lookups with optional invocation (?.(vars)), replacing direct this[...] property access. Adjusts extractVarsFromString and shouldBeSerialized to recognize vars[...] patterns. Changes property/value serialization to accept vars parameter.
Native CSS compiler template output
src/bundler/css-compiler/compileNativeCSS.ts
Changes generated template serialization from get "<key>"() { return value } accessors to vars => value expressions for property entries and currentColor.
Runtime store variable resolution
src/core/native/store.ts
Refactors UniwindStoreBuilder.reinit to use Object.create prototype-based variable cloning instead of cloneWithAccessors, and Object.assign for merging platform/common/theme variables. Updates resolveStyles to collect resolver functions in resultGetters and eagerly evaluate them via Object.fromEntries(...Var(vars)), replacing lazy Object.defineProperty getter patterns on the result object.
CSS variable store assignment
src/core/config/config.native.ts
Changes updateCSSVariables to assign computed values directly to UniwindStore.vars[theme][varName] instead of defining lazy getters, causing eager evaluation at assignment time.
Native color parsing utility
src/core/native/native-utils.ts
Removes cloneWithAccessors helper. Adds new parseColor(type, color) function that parses via culori, formats to 3/6/8-digit hex depending on alpha, returns original input if parsing yields undefined, and falls back to '#000000' on error.
Native property parsers
src/core/native/parsers/{textShadow,transforms}.ts
Replaces Object.defineProperty-based property descriptors with direct property assignments in parseTextShadowMutation and parseTransformsMutation, simplifying mutation logic.
Variable access hook refactoring
src/hooks/useCSSVariable/getVariableValue.native.ts
Refactors getVariableValue from expression body to block body for clarity; logic unchanged—selects appropriate vars map and invokes stored variable function with vars parameter.
Variable cascade test and utility
tests/test.css, tests/native/styles-parsing/colors.test.tsx
Adds @utility var-cascade-after that references a CSS variable before defining it. Adds test case verifying inline variable cascade resolution behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • uni-stack/uniwind#535: Modifies how Processor.vars/scopedVars are serialized and referenced, directly impacting variable dependency detection patterns.
  • uni-stack/uniwind#502: Both PRs modify updateCSSVariables logic in config.native.ts, affecting how scoped theme CSS variables are applied and when notifications fire.
  • uni-stack/uniwind#523: Introduces the compileNativeCSS template serialization that this PR refactors from accessor getter shapes to vars => value expressions.

Suggested reviewers

  • jpudysz

Poem

🐰 From getters lazy to functions swift,
vars now flows through every shift,
No more this[...], now vars[name]?.(vars) sings,
Eager evaluation spreads its wings!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: replace computed with functions in styles' accurately describes the main objective of the pull request—replacing computed properties/getters with function-based approach throughout the styling system.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch replace-computed-with-functions

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/uniwind/tests/test.css`:
- Line 44: The new `@utility` at-rule is tripping the scss/at-rule-no-unknown
linter; either add a targeted inline suppression immediately above the `@utility`
usage (for example a single-line comment to disable scss/at-rule-no-unknown for
the next line) or whitelist this at-rule in the stylelint config by adding
"utility" to the ignoreAtRules list for the scss/at-rule-no-unknown rule so the
`@utility` declaration is allowed; locate the offending token "`@utility`" in the
test file and apply one of these two fixes.
🪄 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: a6170e16-9a10-4b94-bc33-f3a9e2c38227

📥 Commits

Reviewing files that changed from the base of the PR and between 2880232 and fd4cc18.

📒 Files selected for processing (16)
  • packages/uniwind/src/bundler/css-compiler/compileNativeCSS.ts
  • packages/uniwind/src/bundler/css-processor/addMetaToStylesTemplate.ts
  • packages/uniwind/src/bundler/css-processor/color.ts
  • packages/uniwind/src/bundler/css-processor/css.ts
  • packages/uniwind/src/bundler/css-processor/units.ts
  • packages/uniwind/src/bundler/css-processor/utils.ts
  • packages/uniwind/src/bundler/css-processor/var.ts
  • packages/uniwind/src/core/config/config.native.ts
  • packages/uniwind/src/core/native/native-utils.ts
  • packages/uniwind/src/core/native/parsers/textShadow.ts
  • packages/uniwind/src/core/native/parsers/transforms.ts
  • packages/uniwind/src/core/native/store.ts
  • packages/uniwind/src/core/types.ts
  • packages/uniwind/src/hooks/useCSSVariable/getVariableValue.native.ts
  • packages/uniwind/tests/native/styles-parsing/colors.test.tsx
  • packages/uniwind/tests/test.css
💤 Files with no reviewable changes (1)
  • packages/uniwind/src/core/native/native-utils.ts

Comment thread packages/uniwind/tests/test.css
@Brentlok Brentlok merged commit 327fe5b into main May 22, 2026
2 checks passed
@Brentlok Brentlok deleted the replace-computed-with-functions branch May 22, 2026 09:48
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🚀 This pull request is included in v1.8.0. See Release v1.8.0 for release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant