Skip to content

Commit d8ec11c

Browse files
mmalerbammalerba
committed
fix(material/theming): disambiguate token names in extend-theme functions
Avoids token name collisions between namespaces by assigning string prefixes to token in namespaces that would otherwise have a collision
1 parent e0d46a4 commit d8ec11c

File tree

11 files changed

+200
-48
lines changed

11 files changed

+200
-48
lines changed

‎src/material/button/_button-theme.scss

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -417,14 +417,38 @@
417417
@return token-utils.extend-theme(
418418
$theme,
419419
sass-utils.list-of(
420-
tokens-mdc-filled-button.$prefix,
421-
tokens-mdc-outlined-button.$prefix,
422-
tokens-mdc-protected-button.$prefix,
423-
tokens-mdc-text-button.$prefix,
424-
tokens-mat-filled-button.$prefix,
425-
tokens-mat-outlined-button.$prefix,
426-
tokens-mat-protected-button.$prefix,
427-
tokens-mat-text-button.$prefix
420+
(
421+
namespace: tokens-mdc-filled-button.$prefix,
422+
prefix: 'filled-',
423+
),
424+
(
425+
namespace: tokens-mdc-outlined-button.$prefix,
426+
prefix: 'outlined-',
427+
),
428+
(
429+
namespace: tokens-mdc-protected-button.$prefix,
430+
prefix: 'protected-',
431+
),
432+
(
433+
namespace: tokens-mdc-text-button.$prefix,
434+
prefix: 'text-',
435+
),
436+
(
437+
namespace: tokens-mat-filled-button.$prefix,
438+
prefix: 'filled-',
439+
),
440+
(
441+
namespace: tokens-mat-outlined-button.$prefix,
442+
prefix: 'outlined-',
443+
),
444+
(
445+
namespace: tokens-mat-protected-button.$prefix,
446+
prefix: 'protected-',
447+
),
448+
(
449+
namespace: tokens-mat-text-button.$prefix,
450+
prefix: 'text-',
451+
)
428452
),
429453
$overrides
430454
);

‎src/material/button/_fab-theme.scss

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,19 @@
194194
$theme,
195195
sass-utils.list-of(
196196
tokens-mdc-fab.$prefix,
197-
tokens-mdc-fab-small.$prefix,
198-
tokens-mdc-extended-fab.$prefix,
197+
(
198+
namespace: tokens-mdc-fab-small.$prefix,
199+
prefix: 'small-',
200+
),
201+
(
202+
namespace: tokens-mdc-extended-fab.$prefix,
203+
prefix: 'extended-',
204+
),
199205
tokens-mat-fab.$prefix,
200-
tokens-mat-fab-small.$prefix
206+
(
207+
namespace: tokens-mat-fab-small.$prefix,
208+
prefix: 'small-',
209+
)
201210
),
202211
$overrides
203212
);

‎src/material/card/_card-theme.scss

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,19 @@
8787
/// @param {Map} $overrides The token values to override in the theme.
8888
@function extend-theme($theme, $overrides: ()) {
8989
@return token-utils.extend-theme(
90-
$theme,
91-
sass-utils.list-of(
92-
tokens-mdc-elevated-card.$prefix,
93-
tokens-mdc-outlined-card.$prefix,
94-
tokens-mat-card.$prefix
90+
$theme,
91+
sass-utils.list-of(
92+
(
93+
namespace: tokens-mdc-elevated-card.$prefix,
94+
prefix: 'elevated-',
9595
),
96-
$overrides
96+
(
97+
namespace: tokens-mdc-outlined-card.$prefix,
98+
prefix: 'outlined-',
99+
),
100+
tokens-mat-card.$prefix
101+
),
102+
$overrides
97103
);
98104
}
99105

‎src/material/chips/_chips-theme.scss

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
@use 'sass:color';
2+
@use '../core/style/sass-utils';
23
@use '../core/tokens/m2/mdc/chip' as tokens-mdc-chip;
34
@use '../core/tokens/m2/mat/chip' as tokens-mat-chip;
45
@use '../core/tokens/token-utils';

‎src/material/core/_core-theme.scss

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,30 @@
101101
@return token-utils.extend-theme(
102102
$theme,
103103
sass-utils.list-of(
104-
tokens-mat-app.$prefix,
105-
tokens-mat-ripple.$prefix,
106-
tokens-mat-option.$prefix,
107-
tokens-mat-optgroup.$prefix,
108-
tokens-mat-full-pseudo-checkbox.$prefix,
109-
tokens-mat-minimal-pseudo-checkbox.$prefix
104+
(
105+
namespace: tokens-mat-app.$prefix,
106+
prefix: 'app-',
107+
),
108+
(
109+
namespace: tokens-mat-ripple.$prefix,
110+
prefix: 'ripple-',
111+
),
112+
(
113+
namespace: tokens-mat-option.$prefix,
114+
prefix: 'option-',
115+
),
116+
(
117+
namespace: tokens-mat-optgroup.$prefix,
118+
prefix: 'optgroup-',
119+
),
120+
(
121+
namespace: tokens-mat-full-pseudo-checkbox.$prefix,
122+
prefix: 'pseudo-checkbox-full-',
123+
),
124+
(
125+
namespace: tokens-mat-minimal-pseudo-checkbox.$prefix,
126+
prefix: 'pseudo-checkbox-minimal-',
127+
)
110128
),
111129
$overrides
112130
);

‎src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,14 @@
7575
@return token-utils.extend-theme(
7676
$theme,
7777
sass-utils.list-of(
78-
tokens-mat-full-pseudo-checkbox.$prefix,
79-
tokens-mat-minimal-pseudo-checkbox.$prefix
78+
(
79+
namespace: tokens-mat-full-pseudo-checkbox.$prefix,
80+
prefix: 'full-',
81+
),
82+
(
83+
namespace: tokens-mat-minimal-pseudo-checkbox.$prefix,
84+
prefix: 'minimal-',
85+
)
8086
),
8187
$overrides
8288
);

‎src/material/core/theming/tests/m3-theme.spec.ts

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,77 @@ describe('M3 theme', () => {
262262
);
263263
});
264264

265+
it('should allow accessing a namespace with a prefix', () => {
266+
const css = transpile(`
267+
@use '../../tokens/token-utils';
268+
269+
$theme: token-utils.extend-theme($theme,
270+
(
271+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'),
272+
(mat, full-pseudo-checkbox)
273+
),
274+
(
275+
minimal-selected-checkmark-color: magenta,
276+
selected-checkmark-color: cyan
277+
)
278+
);
279+
280+
html {
281+
@include mat.pseudo-checkbox-theme($theme);
282+
}
283+
`);
284+
285+
expect(css).toContain('--mat-minimal-pseudo-checkbox-selected-checkmark-color: magenta');
286+
expect(css).toContain('--mat-full-pseudo-checkbox-selected-checkmark-color: cyan');
287+
});
288+
289+
it('should not allow accessing a prefixed namespace without its prefix', () => {
290+
expect(() =>
291+
transpile(`
292+
@use '../../tokens/token-utils';
293+
294+
$theme: token-utils.extend-theme($theme,
295+
(
296+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'),
297+
),
298+
(
299+
selected-checkmark-color: magenta
300+
)
301+
);
302+
303+
html {
304+
@include mat.pseudo-checkbox-theme($theme);
305+
}
306+
`),
307+
).toThrowError(
308+
/Error extending theme: Unrecognized token `selected-checkmark-color`. Allowed tokens are:.* minimal-selected-checkmark-color/,
309+
);
310+
});
311+
312+
it('should detect name collisions that remain after prefixes are applied', () => {
313+
expect(() =>
314+
transpile(`
315+
@use '../../tokens/token-utils';
316+
317+
$theme: token-utils.extend-theme($theme,
318+
(
319+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'both-'),
320+
(namespace: (mat, full-pseudo-checkbox), prefix: 'both-')
321+
),
322+
(
323+
both-selected-checkmark-color: magenta
324+
)
325+
);
326+
327+
html {
328+
@include mat.pseudo-checkbox-theme($theme);
329+
}
330+
`),
331+
).toThrowError(
332+
/Error extending theme: Ambiguous token name `both-selected-checkmark-color` exists in multiple namespaces/,
333+
);
334+
});
335+
265336
it('should not error when calling component extend-theme functions', () => {
266337
// Ensures that no components have issues with ambiguous token names.
267338
expect(() =>
@@ -307,7 +378,9 @@ describe('M3 theme', () => {
307378
$theme: mat.tooltip-extend-theme($theme, ());
308379
$theme: mat.tree-extend-theme($theme, ());
309380
310-
@include mat.all-component-themes($theme);
381+
html {
382+
@include mat.all-component-themes($theme);
383+
}
311384
`),
312385
).not.toThrow();
313386
});

‎src/material/core/tokens/_token-utils.scss

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,11 @@ $_system-fallbacks: m3-system.create-system-fallbacks();
255255

256256
// Returns a theme config with the given tokens overridden.
257257
/// @param {Map} $theme The material theme for an application.
258+
/// @param {List} $extend-namespaces The namespaces to extend in the theme. Each item can be either:
259+
/// 1. A list, representing a namespace to match token names against.
260+
/// 2. A map, representing a namespace to match tokens agaisnt + a prefix string that the token
261+
/// name must start with to match this namespace. The prefix will be stripped before matching
262+
/// the token name.
258263
/// @param {Map} $overrides The token values to override in the theme.
259264
@function extend-theme($theme, $extend-namespaces, $overrides: ()) {
260265
$internals: _mat-theming-internals-do-not-access;
@@ -268,18 +273,25 @@ $_system-fallbacks: m3-system.create-system-fallbacks();
268273
// Determine which system and namespace each shorthand token belongs to.
269274
$seen-tokens: ();
270275
@each $namespace in $extend-namespaces {
276+
$prefix: '';
277+
// Unpack namespaces that come with a prefix.
278+
@if meta.type-of($namespace) == 'map' {
279+
$prefix: map.get($namespace, prefix);
280+
$namespace: map.get($namespace, namespace);
281+
}
271282
$namespace-found: false;
272283
@each $system in $systems {
273284
$tokens: map.get($theme, $internals, $system, $namespace);
274285
@if ($tokens != null) {
275286
$namespace-found: true;
276287
@each $name, $value in $tokens {
277-
@if map.has-key($seen-tokens, $name) {
278-
@error #{'Error extending theme: Ambiguous token name `'}#{$name}#{
279-
'` exists in multiple namespaces: `('}#{list.nth(map.get($seen-tokens, $name), 1)}#{
280-
')` and `('}#{$namespace}#{')`'};
288+
$prefixed-name: $prefix + $name;
289+
@if map.has-key($seen-tokens, $prefixed-name) {
290+
@error #{'Error extending theme: Ambiguous token name `'}#{$prefixed-name}#{
291+
'` exists in multiple namespaces: `('}#{
292+
list.nth(map.get($seen-tokens, $prefixed-name), 1)}#{')` and `('}#{$namespace}#{')`'};
281293
}
282-
$seen-tokens: map.set($seen-tokens, $name, ($namespace, $system));
294+
$seen-tokens: map.set($seen-tokens, $prefixed-name, ($namespace, $system, $name));
283295
}
284296
}
285297
}
@@ -322,5 +334,6 @@ $_system-fallbacks: m3-system.create-system-fallbacks();
322334
$namespace: list.append($namespace, $variant);
323335
}
324336
$system: list.nth($token-info, 2);
325-
@return map.set($theme, $internals, $system, $namespace, $token, $value);
337+
$token-name: list.nth($token-info, 3);
338+
@return map.set($theme, $internals, $system, $namespace, $token-name, $value);
326339
}

‎src/material/form-field/_form-field-theme.scss

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,14 @@
157157
@return token-utils.extend-theme(
158158
$theme,
159159
sass-utils.list-of(
160-
tokens-mdc-filled-text-field.$prefix,
161-
tokens-mdc-outlined-text-field.$prefix,
160+
(
161+
namespace: tokens-mdc-filled-text-field.$prefix,
162+
prefix: 'filled',
163+
),
164+
(
165+
namespace: tokens-mdc-outlined-text-field.$prefix,
166+
prefix: 'outlined',
167+
),
162168
tokens-mat-form-field.$prefix
163169
),
164170
$overrides

‎src/material/paginator/_paginator-theme.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
@mixin density($theme) {
4545
$density-scale: inspection.get-theme-density($theme);
4646
$form-field-density: if(
47-
(meta.type-of($density-scale) == 'number' and $density-scale >= -4) or $density-scale == maximum,
47+
(meta.type-of($density-scale) == 'number' and $density-scale >= -4) or
48+
($density-scale == maximum),
4849
-4,
4950
$density-scale
5051
);

0 commit comments

Comments
 (0)