Skip to content
Open
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
45 changes: 45 additions & 0 deletions packages/mui-system/src/cssVars/cssVarsParser.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from 'chai';
import { vi } from 'vitest';
import cssVarsParser, { assignNestedKeys, walkObjectDeep } from './cssVarsParser';

describe('cssVarsParser', () => {
Expand Down Expand Up @@ -405,4 +406,48 @@ describe('cssVarsParser', () => {
expect(css).to.deep.equal({});
expect(vars).to.deep.equal({});
});

it('skips string values containing "//" to avoid stylis comment corruption', () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
const { css, vars } = cssVarsParser({
palette: {
primary: {
main: '#000',
},
},
backgroundImage: 'url(https://example.com/image.png)',
});
expect(css).to.not.have.property('--backgroundImage');
expect(vars).to.not.have.property('backgroundImage');
expect(css).to.deep.equal({ '--palette-primary-main': '#000' });
expect(errorSpy.mock.calls[0][0]).to.include('backgroundImage');
expect(errorSpy.mock.calls[0][0]).to.include('//');
errorSpy.mockRestore();
});

it('skips only the URL value and preserves other variables', () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
const { css, vars } = cssVarsParser({
palette: {
primary: {
main: '#ff5252',
image: 'url(https://example.com/image.png)',
dark: '#b71c1c',
},
},
});
expect(css).to.deep.equal({
'--palette-primary-main': '#ff5252',
'--palette-primary-dark': '#b71c1c',
});
expect(vars).to.deep.equal({
palette: {
primary: {
main: 'var(--palette-primary-main)',
dark: 'var(--palette-primary-dark)',
},
},
});
errorSpy.mockRestore();
});
});
10 changes: 10 additions & 0 deletions packages/mui-system/src/cssVars/cssVarsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,16 @@ export default function cssVarsParser<T extends Record<string, any>>(
theme,
(keys, value: string | number | object, arrayKeys) => {
if (typeof value === 'string' || typeof value === 'number') {
if (typeof value === 'string' && value.includes('//')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldSkipGeneratingVar doesn't seem to silence this new dev warning which contradicts the advice in the warning

// Stylis (the CSS preprocessor used by Emotion) treats `//` as a single-line comment,
// which corrupts the entire :root {} block when a URL value is serialized as a CSS variable.
if (process.env.NODE_ENV !== 'production') {
console.error(
`MUI: The theme key "${keys.join('.')}" value ("${value}") contains "//" which is treated as a comment by the CSS preprocessor and cannot be used as a CSS variable value. Use \`shouldSkipGeneratingVar\` to exclude this key from CSS variable generation.`,
);
}
return;
}
if (!shouldSkipGeneratingVar || !shouldSkipGeneratingVar(keys, value)) {
// only create css & var if `shouldSkipGeneratingVar` return false
const cssVar = `--${prefix ? `${prefix}-` : ''}${keys.join('-')}`;
Expand Down
1 change: 1 addition & 0 deletions skills/material-ui-theming/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
- `InitColorSchemeScript`: place before any rendered content to prevent the initial color-scheme flash. App Router: inside `<body>` before `{children}` in `app/layout.tsx`. Pages Router: in `_document.tsx` before `<Main />`. See [Preventing SSR flickering](https://mui.com/material-ui/customization/css-theme-variables/configuration.md#preventing-ssr-flickering).
- Trade-offs: larger HTML (both schemes' variables), possible FCP impact; benefits include less JavaScript work on scheme switch and better SSR dark experience. See [Overview](https://mui.com/material-ui/customization/css-theme-variables/overview.md).
- `CssVarsProvider` is superseded by `ThemeProvider` with the same capabilities. Use `ThemeProvider`.
- **URL values and `//` in theme strings**: Stylis (the CSS preprocessor used by Emotion) treats `//` as a single-line comment. Any theme string value containing `//` — such as a `backgroundImage` URL like `https://…` — will corrupt the generated `:root {}` block and make all subsequent `--mui-*` CSS variables undefined. Use `shouldSkipGeneratingVar` to exclude these keys from CSS variable generation: `createTheme({ cssVariables: { shouldSkipGeneratingVar: (keys) => keys.includes('backgroundImage') } })`. A dev-mode `console.error` is emitted if such a value is detected.

Check warning on line 84 in skills/material-ui-theming/AGENTS.md

View workflow job for this annotation

GitHub Actions / test-dev (ubuntu-latest)

[vale] reported by reviewdog 🐶 [Google.Will] Avoid using 'will'. Raw Output: {"message": "[Google.Will] Avoid using 'will'.", "location": {"path": "skills/material-ui-theming/AGENTS.md", "range": {"start": {"line": 84, "column": 221}}}, "severity": "WARNING"}

---

Expand Down
Loading