Skip to content

Commit ce22487

Browse files
committed
chore: Misc minor cleanup unused and duplicative code
1 parent c85d76f commit ce22487

File tree

13 files changed

+273
-274
lines changed

13 files changed

+273
-274
lines changed

src/build/__tests__/css-vars.test.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,7 @@ test('generateReferenceTokenDefaults creates CSS variable declarations', () => {
8787
});
8888

8989
test('resolveTheme with CSS variables returns CSS var() for reference tokens', () => {
90-
const resolved = resolveTheme(testTheme, undefined, {
91-
propertiesMap,
92-
});
90+
const resolved = resolveTheme(testTheme, undefined, propertiesMap);
9391

9492
// Direct token takes precedence over reference token
9593
expect(resolved.colorPrimary500).toBe('#direct-primary-500');
@@ -115,9 +113,7 @@ test('resolveContext with CSS variables preserves var() references', () => {
115113
},
116114
};
117115

118-
const resolved = resolveContext(testTheme, context, undefined, undefined, {
119-
propertiesMap,
120-
});
116+
const resolved = resolveContext(testTheme, context, undefined, undefined, propertiesMap);
121117

122118
// Should resolve to CSS var since colorNeutral900 is a reference token
123119
expect(resolved.colorSecondary).toBe('var(--color-neutral-900)');
@@ -157,6 +153,4 @@ test('createBuildDeclarations with secondary theme generates reference token CSS
157153
// Should contain reference tokens from both primary and secondary themes
158154
expect(css).toContain('--color-primary-500');
159155
expect(css).toContain('[data-theme="dark"]');
160-
// TODO: Reference tokens from secondary themes are not yet included in CSS generation
161-
// expect(css).toContain('--color-error-100');
162156
});

src/shared/declaration/index.ts

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
3-
import { mergeInPlace, Override, Theme, ResolveOptions } from '../theme';
3+
import { mergeInPlace, Override, Theme } from '../theme';
44
import { flattenReferenceTokens, collectReferencedTokens } from '../theme/utils';
55
import type { PropertiesMap, SelectorCustomizer } from './interfaces';
66
import { RuleCreator } from './rule';
@@ -35,35 +35,58 @@ function createMinimalTheme(base: Theme, override: Override): Theme {
3535
return mergeInPlace(minimalTheme, override);
3636
}
3737

38+
function collectAllRequiredTokens(
39+
themes: Theme[],
40+
initialTokens: string[]
41+
): { referenceTokens: string[]; referencedTokens: string[] } {
42+
const referenceTokens: string[] = [];
43+
themes.forEach((theme) => referenceTokens.push(...Object.keys(flattenReferenceTokens(theme))));
44+
45+
const alreadyIncluded = new Set([...initialTokens, ...referenceTokens]);
46+
const referencedTokens: string[] = [];
47+
48+
themes.forEach((theme) => {
49+
const referenced = collectReferencedTokens(theme, [...initialTokens, ...referenceTokens]);
50+
referenced.forEach((token) => {
51+
if (!alreadyIncluded.has(token)) {
52+
referencedTokens.push(token);
53+
alreadyIncluded.add(token);
54+
}
55+
});
56+
});
57+
58+
return { referenceTokens, referencedTokens };
59+
}
60+
61+
function addMissingTokensToTheme(theme: Theme, tokens: string[], sourceTheme: Theme): void {
62+
tokens.forEach((token) => {
63+
if (!(token in theme.tokens) && token in sourceTheme.tokens) {
64+
theme.tokens[token] = sourceTheme.tokens[token];
65+
}
66+
});
67+
}
68+
3869
export function createOverrideDeclarations(
3970
base: Theme,
4071
override: Override,
4172
propertiesMap: PropertiesMap,
4273
selectorCustomizer: SelectorCustomizer
4374
): string {
4475
const minimalTheme = createMinimalTheme(base, override);
45-
let usedTokens = Object.keys(minimalTheme.tokens);
46-
47-
const allReferencedTokens = collectReferencedTokens(base, usedTokens);
48-
usedTokens = [...usedTokens, ...allReferencedTokens];
76+
const initialTokens = Object.keys(minimalTheme.tokens);
4977

78+
const { referencedTokens } = collectAllRequiredTokens([base], initialTokens);
5079
// Add referenced tokens to minimalTheme so they get output in root
5180
// The transformer will remove them from mode/context selectors since they're unchanged
52-
allReferencedTokens.forEach((token) => {
53-
if (!(token in minimalTheme.tokens) && token in base.tokens) {
54-
minimalTheme.tokens[token] = base.tokens[token];
55-
}
56-
});
81+
addMissingTokensToTheme(minimalTheme, referencedTokens, base);
5782

83+
const usedTokens = [...initialTokens, ...referencedTokens];
5884
const ruleCreator = new RuleCreator(
5985
new Selector(selectorCustomizer),
6086
new UsedPropertyRegistry(propertiesMap, usedTokens)
6187
);
62-
const stylesheetCreator = new SingleThemeCreator(minimalTheme, ruleCreator, base, { propertiesMap });
63-
const stylesheet = stylesheetCreator.create();
64-
const transformer = new MinimalTransformer();
65-
const minimal = transformer.transform(stylesheet);
66-
return minimal.toString();
88+
const stylesheet = new SingleThemeCreator(minimalTheme, ruleCreator, base, propertiesMap).create();
89+
return new MinimalTransformer().transform(stylesheet).toString();
6790
}
6891

6992
export function createBuildDeclarations(
@@ -74,38 +97,13 @@ export function createBuildDeclarations(
7497
used: string[]
7598
): string {
7699
const themes = [primary, ...secondary];
77-
// Add reference tokens (from referenceTokens object)
78-
const allReferenceTokens: string[] = [];
79-
themes.forEach((theme: Theme) => {
80-
const referenceTokens = flattenReferenceTokens(theme);
81-
allReferenceTokens.push(...Object.keys(referenceTokens));
82-
});
83-
84-
// Collect referenced leaf tokens (like colorWhite, colorGreyOpaque70)
85-
const allReferencedTokens: string[] = [];
86-
const alreadyIncluded = new Set([...used, ...allReferenceTokens]);
87-
88-
themes.forEach((theme: Theme) => {
89-
const referenced = collectReferencedTokens(theme, [...used, ...allReferenceTokens]);
90-
referenced.forEach((token) => {
91-
if (!alreadyIncluded.has(token)) {
92-
allReferencedTokens.push(token);
93-
alreadyIncluded.add(token);
94-
}
95-
});
96-
});
97-
98-
// Add allReferencedTokens to effectiveUsed so they get created in primary rule
99-
// The transformer will remove them from mode rules since they're unchanged
100-
const effectiveUsed = [...used, ...allReferenceTokens, ...allReferencedTokens];
100+
const { referenceTokens, referencedTokens } = collectAllRequiredTokens(themes, used);
101+
const usedTokens = [...used, ...referenceTokens, ...referencedTokens];
101102

102103
const ruleCreator = new RuleCreator(
103104
new Selector(selectorCustomizer),
104-
new UsedPropertyRegistry(propertiesMap, effectiveUsed)
105+
new UsedPropertyRegistry(propertiesMap, usedTokens)
105106
);
106-
const stylesheetCreator = new MultiThemeCreator([primary, ...secondary], ruleCreator, { propertiesMap });
107-
const stylesheet = stylesheetCreator.create();
108-
const transformer = new MinimalTransformer();
109-
const minimal = transformer.transform(stylesheet);
110-
return minimal.toString();
107+
const stylesheet = new MultiThemeCreator(themes, ruleCreator, propertiesMap).create();
108+
return new MinimalTransformer().transform(stylesheet).toString();
111109
}

src/shared/declaration/multi.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,8 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33
import { isGlobalSelector } from '../styles/selector';
4-
import {
5-
defaultsReducer,
6-
modeReducer,
7-
OptionalState,
8-
reduce,
9-
resolveContext,
10-
resolveTheme,
11-
Theme,
12-
ResolveOptions,
13-
} from '../theme';
14-
import { flattenReferenceTokens } from '../theme/utils';
4+
import { defaultsReducer, modeReducer, OptionalState, reduce, resolveContext, resolveTheme, Theme } from '../theme';
5+
import type { PropertiesMap } from './interfaces';
156
import { AbstractCreator } from './abstract';
167
import type { StylesheetCreator } from './interfaces';
178
import { RuleCreator, SelectorConfig } from './rule';
@@ -26,13 +17,13 @@ import { compact } from './utils';
2617
export class MultiThemeCreator extends AbstractCreator implements StylesheetCreator {
2718
themes: Theme[];
2819
ruleCreator: RuleCreator;
29-
options?: ResolveOptions;
20+
propertiesMap?: PropertiesMap;
3021

31-
constructor(themes: Theme[], ruleCreator: RuleCreator, options?: ResolveOptions) {
22+
constructor(themes: Theme[], ruleCreator: RuleCreator, propertiesMap?: PropertiesMap) {
3223
super();
3324
this.themes = themes;
3425
this.ruleCreator = ruleCreator;
35-
this.options = options;
26+
this.propertiesMap = propertiesMap;
3627
}
3728

3829
create(): Stylesheet {
@@ -51,7 +42,7 @@ export class MultiThemeCreator extends AbstractCreator implements StylesheetCrea
5142
if (!globalThemes.length) {
5243
// If there is no root theme, all themes are scoped by their root selector. No interference.
5344
const stylesheets = this.themes.map((theme) =>
54-
new SingleThemeCreator(theme, this.ruleCreator, undefined, this.options).create()
45+
new SingleThemeCreator(theme, this.ruleCreator, undefined, this.propertiesMap).create()
5546
);
5647
const result = new Stylesheet();
5748
stylesheets.forEach((stylesheet) => {
@@ -62,7 +53,7 @@ export class MultiThemeCreator extends AbstractCreator implements StylesheetCrea
6253

6354
const [globalTheme] = globalThemes;
6455

65-
const stylesheet = new SingleThemeCreator(globalTheme, this.ruleCreator, undefined, this.options).create();
56+
const stylesheet = new SingleThemeCreator(globalTheme, this.ruleCreator, undefined, this.propertiesMap).create();
6657
const secondaries = this.getThemesWithout(globalTheme);
6758
secondaries.forEach((secondary) => {
6859
this.appendRulesForSecondary(stylesheet, globalTheme, secondary);
@@ -72,7 +63,7 @@ export class MultiThemeCreator extends AbstractCreator implements StylesheetCrea
7263
}
7364

7465
appendRulesForSecondary(stylesheet: Stylesheet, primary: Theme, secondary: Theme) {
75-
const secondaryResolution = resolveTheme(secondary, undefined, this.options);
66+
const secondaryResolution = resolveTheme(secondary, undefined, this.propertiesMap);
7667
const defaults = reduce(secondaryResolution, secondary, defaultsReducer());
7768

7869
const rootRule = this.ruleCreator.create({ global: [secondary.selector] }, defaults);
@@ -96,7 +87,7 @@ export class MultiThemeCreator extends AbstractCreator implements StylesheetCrea
9687

9788
MultiThemeCreator.forEachContext(secondary, (context) => {
9889
const contextResolution = reduce(
99-
resolveContext(secondary, context, undefined, undefined, this.options),
90+
resolveContext(secondary, context, undefined, undefined, this.propertiesMap),
10091
secondary,
10192
defaultsReducer()
10293
);
@@ -130,7 +121,7 @@ export class MultiThemeCreator extends AbstractCreator implements StylesheetCrea
130121
MultiThemeCreator.forEachContextWithinOptionalModeState(secondary, (context, mode, state) => {
131122
const optionalState = mode.states[state] as OptionalState;
132123
const contextResolution = reduce(
133-
resolveContext(secondary, context, undefined, undefined, this.options),
124+
resolveContext(secondary, context, undefined, undefined, this.propertiesMap),
134125
secondary,
135126
modeReducer(mode, state)
136127
);

src/shared/declaration/single.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import {
99
resolveContext,
1010
resolveTheme,
1111
Theme,
12-
ResolveOptions,
1312
} from '../theme';
13+
import type { PropertiesMap } from './interfaces';
1414
import Stylesheet from './stylesheet';
1515
import { AbstractCreator } from './abstract';
1616
import type { StylesheetCreator } from './interfaces';
@@ -22,15 +22,15 @@ export class SingleThemeCreator extends AbstractCreator implements StylesheetCre
2222
baseTheme?: Theme;
2323
resolution: FullResolution;
2424
ruleCreator: RuleCreator;
25-
options?: ResolveOptions;
25+
propertiesMap?: PropertiesMap;
2626

27-
constructor(theme: Theme, ruleCreator: RuleCreator, baseTheme?: Theme, options?: ResolveOptions) {
27+
constructor(theme: Theme, ruleCreator: RuleCreator, baseTheme?: Theme, propertiesMap?: PropertiesMap) {
2828
super();
2929
this.theme = theme;
3030
this.baseTheme = baseTheme;
31-
this.resolution = resolveTheme(theme, this.baseTheme, options);
31+
this.propertiesMap = propertiesMap;
32+
this.resolution = resolveTheme(theme, this.baseTheme, propertiesMap);
3233
this.ruleCreator = ruleCreator;
33-
this.options = options;
3434
}
3535

3636
create(): Stylesheet {
@@ -53,7 +53,7 @@ export class SingleThemeCreator extends AbstractCreator implements StylesheetCre
5353

5454
SingleThemeCreator.forEachContext(this.theme, (context) => {
5555
const contextResolution = reduce(
56-
resolveContext(this.theme, context, this.baseTheme, this.resolution, this.options),
56+
resolveContext(this.theme, context, this.baseTheme, this.resolution, this.propertiesMap),
5757
this.theme,
5858
defaultsReducer(),
5959
this.baseTheme
@@ -73,7 +73,7 @@ export class SingleThemeCreator extends AbstractCreator implements StylesheetCre
7373

7474
SingleThemeCreator.forEachContextWithinOptionalModeState(this.theme, (context, mode, state) => {
7575
const contextResolution = reduce(
76-
resolveContext(this.theme, context, this.baseTheme, this.resolution, this.options),
76+
resolveContext(this.theme, context, this.baseTheme, this.resolution, this.propertiesMap),
7777
this.theme,
7878
modeReducer(mode, state),
7979
this.baseTheme

src/shared/declaration/stylesheet.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ export class Rule {
8989
}
9090
return rule;
9191
}
92+
93+
isModeRule(): boolean {
94+
return !!this.media;
95+
}
9296
}
9397

9498
export class Declaration {

src/shared/declaration/transformer.ts

Lines changed: 39 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
import { entries } from '../utils';
44
import type Stylesheet from './stylesheet';
55
import { Declaration } from './stylesheet';
6-
import { isGlobalSelector } from '../styles/selector';
6+
import { getFirstSelector, isGlobalSelector } from '../styles/selector';
7+
import { getReferencedVar } from './utils';
78

89
export interface Transformer {
910
transform(stylesheet: Stylesheet): Stylesheet;
@@ -46,56 +47,45 @@ export class MinimalTransformer implements Transformer {
4647
// However, for mode rules (which have media queries), we should skip tokens that have
4748
// identical values to their parent, even if referenced variables are overridden. These can inherit
4849
// properly via natural css variable cascade rules.
49-
const selector = rule.toString().split('{')[0].trim();
50-
const firstSelector = selector.split(/[\s.:[\]]/)[0];
51-
const isModeRule = !!rule.media;
52-
if (!isGlobalSelector(firstSelector)) {
53-
// Helper to check if a variable or any variable it references is overridden
54-
const isVariableOverriddenRecursive = (varName: string, visited = new Set<string>()): boolean => {
55-
if (visited.has(varName)) return false;
56-
visited.add(varName);
57-
58-
// Check if this variable itself is overridden
59-
if (varName in ruleValue && varName in resolvedParent && ruleValue[varName] !== resolvedParent[varName]) {
60-
return true;
61-
}
62-
63-
// Check if this variable references another variable that's overridden
64-
if (varName in ruleValue) {
65-
const varValue = ruleValue[varName];
66-
const refMatch = varValue.match(/var\((--[^)]+)\)/);
67-
if (refMatch) {
68-
const referencedVar = refMatch[1];
69-
if (isVariableOverriddenRecursive(referencedVar, visited)) {
70-
return true;
71-
}
72-
}
73-
}
74-
75-
return false;
76-
};
77-
78-
Object.keys(ruleValue).forEach((property) => {
79-
const value = ruleValue[property];
80-
// Check if this is a CSS variable reference
81-
const match = value.match(/var\((--[^)]+)\)/);
82-
if (match) {
83-
const referencedVar = match[1];
84-
// If the referenced variable (or any variable it references) is overridden
85-
if (isVariableOverriddenRecursive(referencedVar)) {
86-
// For mode rules, only output if value actually differs from resolved parent
87-
// For context rules, always output to ensure correct resolution
88-
if (isModeRule && property in resolvedParent && ruleValue[property] === resolvedParent[property]) {
89-
return;
90-
}
91-
if (!(property in diff) && property in ruleValue) {
92-
diff[property] = ruleValue[property];
93-
}
94-
}
95-
}
96-
});
50+
51+
const firstSelector = getFirstSelector(rule.selector);
52+
const isModeRule = rule.isModeRule();
53+
54+
if (isGlobalSelector(firstSelector)) {
55+
rule.clear();
56+
entries(diff).forEach(([property, value]) => rule.appendDeclaration(new Declaration(property, value)));
57+
if (rule.size() === 0) {
58+
stylesheet.removeRule(rule);
59+
}
60+
return;
9761
}
9862

63+
const isOverridden = (varName: string, visited = new Set<string>()): boolean => {
64+
if (visited.has(varName)) return false;
65+
visited.add(varName);
66+
67+
const isDirectlyOverridden =
68+
varName in ruleValue && varName in resolvedParent && ruleValue[varName] !== resolvedParent[varName];
69+
70+
if (isDirectlyOverridden) return true;
71+
72+
const referencedVar = varName in ruleValue ? getReferencedVar(ruleValue[varName]) : null;
73+
return referencedVar ? isOverridden(referencedVar, visited) : false;
74+
};
75+
76+
Object.keys(ruleValue).forEach((property) => {
77+
const referencedVar = getReferencedVar(ruleValue[property]);
78+
if (!referencedVar || !isOverridden(referencedVar)) return;
79+
// For mode rules, only output if value actually differs from resolved parent
80+
// For context rules, always output to ensure correct resolution
81+
const canInherit = isModeRule && ruleValue[property] === resolvedParent[property];
82+
if (canInherit) return;
83+
84+
if (!(property in diff)) {
85+
diff[property] = ruleValue[property];
86+
}
87+
});
88+
9989
rule.clear();
10090
entries(diff).forEach(([property, value]) => rule.appendDeclaration(new Declaration(property, value)));
10191
if (rule.size() === 0) {

0 commit comments

Comments
 (0)