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

packages/core/src/features/css-custom-property.ts

@@ -1,4 +1,4 @@
-import { createFeature, FeatureContext } from './feature';
+import { createFeature, FeatureContext, FeatureTransformContext } from './feature';
 import * as STSymbol from './st-symbol';
 import type { ImportSymbol } from './st-import';
 import {
@@ -177,12 +177,7 @@ export const hooks = createFeature<{
         for (const [localVarName, localSymbol] of Object.entries(
             STSymbol.getAllByType(meta, `cssVar`)
         )) {
-            const resolve = resolvedSymbols.cssVar[localVarName] || {
-                // fallback to local namespace
-                _kind: `css`,
-                symbol: localSymbol,
-                meta,
-            };
+            const resolve = resolveFinalSymbol(meta, localSymbol, resolvedSymbols);
             customPropsMapping.localToGlobal[localVarName] = getTransformedName(resolve);
             if (resolve.meta === meta) {
                 customPropsMapping.locals.add(localVarName);
@@ -209,12 +204,16 @@ export const hooks = createFeature<{
     transformDeclaration({ decl, resolved }) {
         decl.prop = resolved.localToGlobal[decl.prop] || decl.prop;
     },
-    transformValue({ node, data: { cssVarsMapping } }) {
+    transformValue({ node, data: { meta }, context: { getResolvedSymbols } }) {
         const { value } = node;
         const varWithPrefix = node.nodes[0]?.value || ``;
         if (validateCustomPropertyName(varWithPrefix)) {
-            if (cssVarsMapping && cssVarsMapping[varWithPrefix]) {
-                node.nodes[0].value = cssVarsMapping[varWithPrefix];
+            const resolvedSymbols = getResolvedSymbols(meta);
+            const localSymbol = STSymbol.get(meta, varWithPrefix, `cssVar`);
+            if (localSymbol) {
+                node.nodes[0].value = getTransformedName(
+                    resolveFinalSymbol(meta, localSymbol, resolvedSymbols)
+                );
             }
         }
         // handle default values - ToDo: check if required
@@ -235,6 +234,21 @@ export function get(meta: StylableMeta, name: string): CSSVarSymbol | undefined
     return STSymbol.get(meta, name, `cssVar`);
 }
 
+function resolveFinalSymbol(
+    meta: StylableMeta,
+    localSymbol: CSSVarSymbol,
+    resolvedSymbols: ReturnType<FeatureTransformContext['getResolvedSymbols']>
+) {
+    return (
+        resolvedSymbols.cssVar[localSymbol.name] || {
+            // fallback to local namespace
+            _kind: `css`,
+            symbol: localSymbol,
+            meta,
+        }
+    );
+}
+
 function addCSSProperty({
     context,
     node,

packages/core/src/features/feature.ts

@@ -98,7 +98,6 @@ export interface FeatureHooks<T extends NodeTypes = NodeTypes> {
         context: FeatureTransformContext;
         ast: postcss.Root;
         transformer: StylableTransformer;
-        cssVarsMapping: Record<string, string>;
         path: string[];
     }) => void;
 }

packages/core/src/features/st-mixin.ts

@@ -122,8 +122,8 @@ export const hooks = createFeature({
         }
         return isMarker;
     },
-    transformLastPass({ context, ast, transformer, cssVarsMapping, path }) {
-        ast.walkRules((rule) => appendMixins(context, transformer, rule, cssVarsMapping, path));
+    transformLastPass({ context, ast, transformer, path }) {
+        ast.walkRules((rule) => appendMixins(context, transformer, rule, path));
     },
 });
 
@@ -212,7 +212,6 @@ function appendMixins(
     context: FeatureTransformContext,
     transformer: StylableTransformer,
     rule: postcss.Rule,
-    cssPropertyMapping: Record<string, string>,
     path: string[] = []
 ) {
     const [decls, mixins] = collectRuleMixins(context, rule);
@@ -221,7 +220,7 @@ function appendMixins(
     }
     for (const mixin of mixins) {
         if (mixin.valid) {
-            appendMixin(context, { transformer, mixDef: mixin, rule, path, cssPropertyMapping });
+            appendMixin(context, { transformer, mixDef: mixin, rule, path });
         }
     }
     for (const mixinDecl of decls) {
@@ -359,7 +358,6 @@ interface ApplyMixinContext {
     mixDef: AnalyzedMixin & { valid: true };
     rule: postcss.Rule;
     path: string[];
-    cssPropertyMapping: Record<string, string>;
 }
 
 function appendMixin(context: FeatureTransformContext, config: ApplyMixinContext) {
@@ -475,8 +473,7 @@ function handleCSSMixin(
                 context.diagnostics,
                 mixDef.data.originDecl,
                 stVarOverride,
-                config.path,
-                config.cssPropertyMapping
+                config.path
             );
             collectOptionalArgs(
                 { meta: resolved.meta, resolver: context.resolver },

packages/core/src/features/st-var.ts

@@ -271,7 +271,6 @@ export function parseVarsFromExpr(expr: string) {
 
 function collectVarSymbols(context: FeatureContext, rule: postcss.Rule) {
     rule.walkDecls((decl) => {
-        collectUrls(context.meta, decl); // ToDo: remove
         warnOnDeprecatedCustomValues(context, decl);
 
         // check type annotation
@@ -319,20 +318,6 @@ function warnOnDeprecatedCustomValues(context: FeatureContext, decl: postcss.Dec
     );
 }
 
-// ToDo: remove after moving :vars removal to end of analyze.
-// url collection should pickup vars value during general decls walk
-function collectUrls(meta: StylableMeta, decl: postcss.Declaration) {
-    processDeclarationFunctions(
-        decl,
-        (node) => {
-            if (node.type === 'url') {
-                meta.urls.push(node.url);
-            }
-        },
-        false
-    );
-}
-
 function evaluateValueCall(
     context: FeatureTransformContext,
     parsedNode: ParsedValue,

packages/core/src/functions.ts

@@ -23,7 +23,6 @@ export interface EvalValueData {
     node?: postcss.Node;
     meta: StylableMeta;
     stVarOverride?: Record<string, string> | null;
-    cssVarsMapping?: Record<string, string>;
     args?: string[];
     rootArgument?: string;
     initialNode?: postcss.Node;
@@ -63,7 +62,6 @@ export class StylableEvaluator {
             this.valueHook,
             context.diagnostics,
             context.passedThrough,
-            data.cssVarsMapping,
             data.args,
             data.rootArgument,
             data.initialNode
@@ -94,8 +92,7 @@ export function resolveArgumentsValue(
     diagnostics: Diagnostics,
     node: postcss.Node,
     variableOverride?: Record<string, string>,
-    path?: string[],
-    cssVarsMapping?: Record<string, string>
+    path?: string[]
 ) {
     const resolvedArgs = {} as Record<string, string>;
     for (const k in options) {
@@ -108,7 +105,6 @@ export function resolveArgumentsValue(
             transformer.replaceValueHook,
             diagnostics,
             path,
-            cssVarsMapping,
             undefined
         );
     }
@@ -125,7 +121,6 @@ export function processDeclarationValue(
     valueHook?: replaceValueHook,
     diagnostics: Diagnostics = new Diagnostics(),
     passedThrough: string[] = [],
-    cssVarsMapping: Record<string, string> = {},
     args: string[] = [],
     rootArgument?: string,
     initialNode?: postcss.Node
@@ -155,7 +150,6 @@ export function processDeclarationValue(
                         node,
                         meta,
                         stVarOverride: variableOverride,
-                        cssVarsMapping,
                         args,
                         rootArgument,
                         initialNode,
@@ -228,7 +222,6 @@ export function processDeclarationValue(
                         node,
                         meta,
                         stVarOverride: variableOverride,
-                        cssVarsMapping,
                         args,
                         rootArgument,
                         initialNode,
@@ -312,7 +305,6 @@ export function evalDeclarationValue(
     valueHook?: replaceValueHook,
     diagnostics?: Diagnostics,
     passedThrough: string[] = [],
-    cssVarsMapping?: Record<string, string>,
     args: string[] = [],
     getResolvedSymbols: (meta: StylableMeta) => MetaResolvedSymbols = createSymbolResolverWithCache(
         resolver,
@@ -329,7 +321,6 @@ export function evalDeclarationValue(
         valueHook,
         diagnostics,
         passedThrough,
-        cssVarsMapping,
         args
     ).outputValue;
 }

packages/core/src/stylable-processor.ts

@@ -51,15 +51,7 @@ export class StylableProcessor implements FeatureContext {
             }
         });
 
-        const isStylable = this.meta.type === 'stylable';
         root.walkDecls((decl) => {
-            const parent = decl.parent as postcss.ChildNode;
-            if (parent.type === 'rule' && parent.selector === ':vars' && isStylable) {
-                // ToDo: remove once
-                // - custom property definition is allowed in var value
-                // - url collection is removed from st-var
-                return;
-            }
             CSSClass.hooks.analyzeDeclaration({ context: this, decl });
             CSSCustomProperty.hooks.analyzeDeclaration({ context: this, decl });
             CSSContains.hooks.analyzeDeclaration({ context: this, decl });

packages/core/src/stylable-transformer.ts

@@ -314,7 +314,6 @@ export class StylableTransformer {
                 value: decl.value,
                 meta,
                 node: decl,
-                cssVarsMapping: cssVarsMapping.localToGlobal,
             }).outputValue;
         };
 
@@ -354,7 +353,6 @@ export class StylableTransformer {
             context: transformContext,
             ast,
             transformer: this,
-            cssVarsMapping: cssVarsMapping.localToGlobal,
             path,
         };
         if (this.experimentalSelectorInference) {

packages/core/test/features/css-custom-property.spec.ts

@@ -757,22 +757,86 @@ describe(`features/css-custom-property`, () => {
 
             shouldReportNoDiagnostics(meta);
         });
-        it(`should NOT define property as var value (change in v5)`, () => {
-            // ToDo: in the future property should be able to be defined in var value
-            const { sheets } = testStylableCore(`
-                :vars {
-                    myVar: var(--color);
-                }
+        it(`should define property as var value`, () => {
+            const { sheets } = testStylableCore({
+                './origin.st.css': `
+                    :vars {
+                        x: var(--x);
+                    }
+                `,
+                './entry.st.css': `
+                    @st-import [x as importedVar] from './origin.st.css';
+                    :vars {
+                        y: var(--y);
+                        z: value(y) value(importedVar);
+                    }
 
-                .root {
-                    /* @decl prop: var(--color) */
-                    prop: value(myVar);
-                }
-            `);
+                    .root {
+                        --x: context property does not override property from origin;
+
+                        /* @decl(local) prop: var(--entry-y) */
+                        prop: value(y);
+
+                        /* @decl(imported) prop: var(--origin-x) */
+                        prop: value(importedVar);
+
+                        /* @decl(mix) prop: var(--entry-y) var(--origin-x) */
+                        prop: value(z);
+                    }
+                `,
+            });
+
+            const { meta, exports } = sheets['/entry.st.css'];
+
+            shouldReportNoDiagnostics(meta);
+
+            // symbols
+            expect(CSSCustomProperty.get(meta, `--y`), `--y symbol`).to.eql({
+                _kind: `cssVar`,
+                name: `--y`,
+                global: false,
+                alias: undefined,
+            });
+
+            // JS exports
+            expect(exports.vars, `JS export`).to.eql({ y: `--entry-y`, x: `--entry-x` });
+        });
+        it(`should preserve string value with custom property`, () => {
+            const { sheets } = testStylableCore({
+                './origin.st.css': `
+                    :vars {
+                        x: 'var(--x)';
+                    }
+                `,
+                './entry.st.css': `
+                    @st-import [x as importedVar] from './origin.st.css';
+                    :vars {
+                        y: "var(--y)";
+                        z: value(y) value(importedVar);
+                    }
+
+                    .root {
+                        /* @decl(local) prop: var(--y) */
+                        prop: value(y);
+
+                        /* @decl(imported) prop: var(--x) */
+                        prop: value(importedVar);
+
+                        /* @decl(mix) prop: var(--y) var(--x) */
+                        prop: value(z);
+                    }
+                `,
+            });
 
             const { meta } = sheets['/entry.st.css'];
 
             shouldReportNoDiagnostics(meta);
+
+            // symbols
+            expect(CSSCustomProperty.get(meta, `--y`), `--y symbol`).to.eql(undefined);
+
+            // JS exports
+            expect(exports.vars, `JS export`).to.eql(undefined);
         });
     });
     describe(`st-formatter`, () => {
@@ -886,6 +950,46 @@ describe(`features/css-custom-property`, () => {
 
             const { meta } = sheets['/entry.st.css'];
 
+            shouldReportNoDiagnostics(meta);
+        });
+        it('should namespace custom-props within build vars', () => {
+            const { sheets } = testStylableCore({
+                './mix.st.css': `
+                    :vars {
+                        x: var(--x);
+                    }
+                    .mix {
+                        val: value(x);
+                    }
+                `,
+                './entry.st.css': `
+                    @st-import [mix as importedMix] from './mix.st.css';
+                    :vars {
+                        x: var(--y);
+                    }
+                    .localMix {
+                        val: value(x);
+                    }
+
+                    /* @rule(local) .entry__root { val: var(--entry-y); } */
+                    .root {
+                        -st-mixin: localMix;
+                    }
+
+                    /* @rule(imported) .entry__root { val: var(--mix-x); } */
+                    .root {
+                        -st-mixin: importedMix;
+                    }
+
+                    /* @rule(with local override) .entry__root { val: var(--entry-y); } */
+                    .root {
+                        -st-mixin: importedMix(x value(x));
+                    }
+                `,
+            });
+
+            const { meta } = sheets['/entry.st.css'];
+
             shouldReportNoDiagnostics(meta);
         });
     });