Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core)!: normalize build-var value #2921

Merged
merged 4 commits into from
Oct 26, 2023
Merged

Conversation

idoros
Copy link
Collaborator

@idoros idoros commented Oct 24, 2023

This PR modifies the evaluation method for the Stylable build-vars value to ensure its consistency with other values.

Please note, this introduces a breaking change. We had initially deferred this adjustment until version 6. However, to streamline edge cases, we've decided to alter the behavior now.

Previous Behavior

In the past, the build-var value wasn't assessed in the same manner as other values. This discrepancy led to symbols defined within it being overlooked, especially when referencing a custom property.

For instance, using buildVar: var(--dynamicVar) wouldn't define the --dynamicVar symbol. As a result, the evaluation of the build-var would retain a global, un-namespaced custom property. Complicating matters, if this property was defined outside the build-var, it would be namespaced. This behavior can be preserved (shown below).

Another case that is being deprecated/fixed is when evaluating a build-var, it would pickup custom-properties from the evaluated context. That was an unintentional/undocumented/weird behavior that we don't think should be supported, have seen no cases of anyone using it, and unlike the global/undeclared behavior that can be preserved, this "fix" doesn't have any alternatives.

Retaining the Old Behavior

For legacy code that sets a build-var with a global custom property and wishes to maintain the previous behavior, there are two solutions:

Enclose the value in quotes:

:vars {
    buildVar: "var(--dynamicVar)";
}

Define a global custom property:

@property st-global(--dynamicVar);
:vars {
    buildVar: var(--dynamicVar);
}

Notice that a deprecation info diagnostic is introduced in stylable@5 to report unknown custom-properties in build-vars: #2922

@idoros idoros added feature New syntax feature or behavior core Processing and transforming logic tech debt Updates, upgrades, stale code and work-arounds labels Oct 24, 2023
@idoros idoros requested review from barak007 and tomrav October 24, 2023 09:13
@idoros idoros self-assigned this Oct 24, 2023
@idoros idoros force-pushed the ido/normalize-build-var-value branch from d6f941f to 5010d5a Compare October 24, 2023 11:34
@idoros idoros merged commit 3caedca into master Oct 26, 2023
12 checks passed
@idoros idoros deleted the ido/normalize-build-var-value branch October 26, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Processing and transforming logic feature New syntax feature or behavior tech debt Updates, upgrades, stale code and work-arounds
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants