Skip to content

Commit cea28c3

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 3eb85a4 commit cea28c3

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
@@ -134,12 +134,30 @@ $_has-inserted-loaded-marker: false;
134134
@return token-utils.extend-theme(
135135
$theme,
136136
sass-utils.list-of(
137-
tokens-mat-app.$prefix,
138-
tokens-mat-ripple.$prefix,
139-
tokens-mat-option.$prefix,
140-
tokens-mat-optgroup.$prefix,
141-
tokens-mat-full-pseudo-checkbox.$prefix,
142-
tokens-mat-minimal-pseudo-checkbox.$prefix
137+
(
138+
namespace: tokens-mat-app.$prefix,
139+
prefix: 'app-',
140+
),
141+
(
142+
namespace: tokens-mat-ripple.$prefix,
143+
prefix: 'ripple-',
144+
),
145+
(
146+
namespace: tokens-mat-option.$prefix,
147+
prefix: 'option-',
148+
),
149+
(
150+
namespace: tokens-mat-optgroup.$prefix,
151+
prefix: 'optgroup-',
152+
),
153+
(
154+
namespace: tokens-mat-full-pseudo-checkbox.$prefix,
155+
prefix: 'pseudo-checkbox-full-',
156+
),
157+
(
158+
namespace: tokens-mat-minimal-pseudo-checkbox.$prefix,
159+
prefix: 'pseudo-checkbox-minimal-',
160+
)
143161
),
144162
$overrides
145163
);

‎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
@@ -278,6 +278,77 @@ describe('M3 theme', () => {
278278
);
279279
});
280280

281+
it('should allow accessing a namespace with a prefix', () => {
282+
const css = transpile(`
283+
@use '../../tokens/token-utils';
284+
285+
$theme: token-utils.extend-theme($theme,
286+
(
287+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'),
288+
(mat, full-pseudo-checkbox)
289+
),
290+
(
291+
minimal-selected-checkmark-color: magenta,
292+
selected-checkmark-color: cyan
293+
)
294+
);
295+
296+
html {
297+
@include mat.pseudo-checkbox-theme($theme);
298+
}
299+
`);
300+
301+
expect(css).toContain('--mat-minimal-pseudo-checkbox-selected-checkmark-color: magenta');
302+
expect(css).toContain('--mat-full-pseudo-checkbox-selected-checkmark-color: cyan');
303+
});
304+
305+
it('should not allow accessing a prefixed namespace without its prefix', () => {
306+
expect(() =>
307+
transpile(`
308+
@use '../../tokens/token-utils';
309+
310+
$theme: token-utils.extend-theme($theme,
311+
(
312+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'),
313+
),
314+
(
315+
selected-checkmark-color: magenta
316+
)
317+
);
318+
319+
html {
320+
@include mat.pseudo-checkbox-theme($theme);
321+
}
322+
`),
323+
).toThrowError(
324+
/Error extending theme: Unrecognized token `selected-checkmark-color`. Allowed tokens are:.* minimal-selected-checkmark-color/,
325+
);
326+
});
327+
328+
it('should detect name collisions that remain after prefixes are applied', () => {
329+
expect(() =>
330+
transpile(`
331+
@use '../../tokens/token-utils';
332+
333+
$theme: token-utils.extend-theme($theme,
334+
(
335+
(namespace: (mat, minimal-pseudo-checkbox), prefix: 'both-'),
336+
(namespace: (mat, full-pseudo-checkbox), prefix: 'both-')
337+
),
338+
(
339+
both-selected-checkmark-color: magenta
340+
)
341+
);
342+
343+
html {
344+
@include mat.pseudo-checkbox-theme($theme);
345+
}
346+
`),
347+
).toThrowError(
348+
/Error extending theme: Ambiguous token name `both-selected-checkmark-color` exists in multiple namespaces/,
349+
);
350+
});
351+
281352
it('should not error when calling component extend-theme functions', () => {
282353
// Ensures that no components have issues with ambiguous token names.
283354
expect(() =>
@@ -323,7 +394,9 @@ describe('M3 theme', () => {
323394
$theme: mat.tooltip-extend-theme($theme, ());
324395
$theme: mat.tree-extend-theme($theme, ());
325396
326-
@include mat.all-component-themes($theme);
397+
html {
398+
@include mat.all-component-themes($theme);
399+
}
327400
`),
328401
).not.toThrow();
329402
});

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,11 @@ $_component-prefix: null;
431431

432432
/// Returns a theme config with the given tokens overridden.
433433
/// @param {Map} $theme The material theme for an application.
434+
/// @param {List} $extend-namespaces The namespaces to extend in the theme. Each item can be either:
435+
/// 1. A list, representing a namespace to match token names against.
436+
/// 2. A map, representing a namespace to match tokens agaisnt + a prefix string that the token
437+
/// name must start with to match this namespace. The prefix will be stripped before matching
438+
/// the token name.
434439
/// @param {Map} $overrides The token values to override in the theme.
435440
@function extend-theme($theme, $extend-namespaces, $overrides: ()) {
436441
$internals: _mat-theming-internals-do-not-access;
@@ -444,18 +449,25 @@ $_component-prefix: null;
444449
// Determine which system and namespace each shorthand token belongs to.
445450
$seen-tokens: ();
446451
@each $namespace in $extend-namespaces {
452+
$prefix: '';
453+
// Unpack namespaces that come with a prefix.
454+
@if meta.type-of($namespace) == 'map' {
455+
$prefix: map.get($namespace, prefix);
456+
$namespace: map.get($namespace, namespace);
457+
}
447458
$namespace-found: false;
448459
@each $system in $systems {
449460
$tokens: map.get($theme, $internals, $system, $namespace);
450461
@if ($tokens != null) {
451462
$namespace-found: true;
452463
@each $name, $value in $tokens {
453-
@if map.has-key($seen-tokens, $name) {
454-
@error #{'Error extending theme: Ambiguous token name `'}#{$name}#{
455-
'` exists in multiple namespaces: `('}#{list.nth(map.get($seen-tokens, $name), 1)}#{
456-
')` and `('}#{$namespace}#{')`'};
464+
$prefixed-name: $prefix + $name;
465+
@if map.has-key($seen-tokens, $prefixed-name) {
466+
@error #{'Error extending theme: Ambiguous token name `'}#{$prefixed-name}#{
467+
'` exists in multiple namespaces: `('}#{
468+
list.nth(map.get($seen-tokens, $prefixed-name), 1)}#{')` and `('}#{$namespace}#{')`'};
457469
}
458-
$seen-tokens: map.set($seen-tokens, $name, ($namespace, $system));
470+
$seen-tokens: map.set($seen-tokens, $prefixed-name, ($namespace, $system, $name));
459471
}
460472
}
461473
}
@@ -498,5 +510,6 @@ $_component-prefix: null;
498510
$namespace: list.append($namespace, $variant);
499511
}
500512
$system: list.nth($token-info, 2);
501-
@return map.set($theme, $internals, $system, $namespace, $token, $value);
513+
$token-name: list.nth($token-info, 3);
514+
@return map.set($theme, $internals, $system, $namespace, $token-name, $value);
502515
}

‎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)