Conversation
…ST nodes
The var() rewriter only walked plain string values in style objects, so var()
calls embedded inside a TemplateLiteral AST node (e.g. inside a styleFn body
like `flexShrink: \`var(--x, ${expr})\``) were never passed to
adapter.resolveValue. When the adapter resolves a var, the default value is now
dropped — the resolved StyleX token supersedes the runtime fallback.
Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com>
Contributor
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dd6ab4c0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Strip placeholder sentinels from var() fallbacks before forwarding to adapter.resolveValue. Adapters that inspect ctx.fallback were receiving synthetic '__SC_TPL_EXPR_*' markers when the var()'s default contained a template interpolation, which could mis-resolve dynamic style-function values. - Apply varsToDrop to every rewritten bucket (extraStyleObjects, variantBuckets) and styleFnDecls, not just the base styleObj. When the adapter returns dropDefinition: true for a var defined inside a non-base bucket, the local '--name: ...' definition was previously left behind, contradicting the adapter contract. Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com>
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
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.
var(--x, default)calls embedded inside AST nodes (e.g. style function template literals likeflexShrink: \var(--x, ${expr})`) were never passed toadapter.resolveValue, so the originalvar(...)` text was kept as-is. The walker now recurses into AST template literals, calls the adapter, and drops the default value when the adapter resolves the var.