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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions packages/core/src/features/css-custom-property.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/features/feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
11 changes: 4 additions & 7 deletions packages/core/src/features/st-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
},
});

Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 },
Expand Down
15 changes: 0 additions & 15 deletions packages/core/src/features/st-var.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 1 addition & 10 deletions packages/core/src/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -63,7 +62,6 @@ export class StylableEvaluator {
this.valueHook,
context.diagnostics,
context.passedThrough,
data.cssVarsMapping,
data.args,
data.rootArgument,
data.initialNode
Expand Down Expand Up @@ -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) {
Expand All @@ -108,7 +105,6 @@ export function resolveArgumentsValue(
transformer.replaceValueHook,
diagnostics,
path,
cssVarsMapping,
undefined
);
}
Expand All @@ -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
Expand Down Expand Up @@ -155,7 +150,6 @@ export function processDeclarationValue(
node,
meta,
stVarOverride: variableOverride,
cssVarsMapping,
args,
rootArgument,
initialNode,
Expand Down Expand Up @@ -228,7 +222,6 @@ export function processDeclarationValue(
node,
meta,
stVarOverride: variableOverride,
cssVarsMapping,
args,
rootArgument,
initialNode,
Expand Down Expand Up @@ -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,
Expand All @@ -329,7 +321,6 @@ export function evalDeclarationValue(
valueHook,
diagnostics,
passedThrough,
cssVarsMapping,
args
).outputValue;
}
8 changes: 0 additions & 8 deletions packages/core/src/stylable-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/stylable-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ export class StylableTransformer {
value: decl.value,
meta,
node: decl,
cssVarsMapping: cssVarsMapping.localToGlobal,
}).outputValue;
};

Expand Down Expand Up @@ -354,7 +353,6 @@ export class StylableTransformer {
context: transformContext,
ast,
transformer: this,
cssVarsMapping: cssVarsMapping.localToGlobal,
path,
};
if (this.experimentalSelectorInference) {
Expand Down
126 changes: 115 additions & 11 deletions packages/core/test/features/css-custom-property.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand Down Expand Up @@ -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);
});
});
Expand Down