mui / mui/material-ui

[system] Improve error message on dynamic theme values?

Open
#28,573 0 comments 9 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

scope: system type: new feature waiting for 👍
Dominant language
JavaScript
Stars
99.1k
Forks
32.5k
Avg merge
2d 17h
Merged PRs (30d)
106

Description

https://github.com/mui-org/material-ui/issues/28482 surfaced a pain with the v5-migration: the error message when using <Button color="default"> is unclear. This prop was supported in v4 and no longer in v5.

Going forward, it seems that developers will have the same pain when using custom themes. Consider the following case:

import * as React from "react";
import Button from "@mui/material/Button";

export default function BasicButtons() {
  return (
    <Button color="my-custom-value" variant="text">
      Text
    </Button>
  );
}

https://codesandbox.io/s/basicbuttons-material-demo-forked-qe7te?file=/demo.js:0-212

It fails with TypeError: Cannot read properties of undefined (reading 'main'). The why it fails, nor how to solve it requires developers to research, it's not the best DX.
Some might argue that. TypeScript is enough for this problem, however, it depends on the parent theme context the component is mounted in.

One possible solution

One idea:

diff --git a/packages/mui-material/src/Button/Button.js b/packages/mui-material/src/Button/Button.js
index 4393cb5d6a..ad54c68178 100644
--- a/packages/mui-material/src/Button/Button.js
+++ b/packages/mui-material/src/Button/Button.js
@@ -73,7 +73,7 @@ const ButtonRoot = styled(ButtonBase, {
     ];
   },
 })(
-  ({ theme, ownerState }) => ({
+  ({ theme, themeGetter, ownerState }) => ({
     ...theme.typography.button,
     minWidth: 64,
     padding: '6px 16px',
@@ -173,7 +173,7 @@ const ButtonRoot = styled(ButtonBase, {
     ...(ownerState.variant === 'outlined' &&
       ownerState.color !== 'inherit' && {
         color: theme.palette[ownerState.color].main,
-        border: `1px solid ${alpha(theme.palette[ownerState.color].main, 0.5)}`,
+        border: `1px solid ${alpha(themeGetter(`theme.palette.${ownerState.color}.main`), 0.5)}`,
       }),
     ...(ownerState.variant === 'contained' && {
       color: theme.palette.getContrastText(theme.palette.grey[300]),

that would return a clear error message?

Error: MUI: The MuiButton's Root component tries to access theme.palette.my-custom-value.main but the value is missing. Update your theme to include it.

Alternatives

?

Benchmark

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with packages/mui-material/src/Button/Button.js, especially the ButtonRoot styles that read theme.palette[ownerState.color].main. Review how the dynamic color value reaches that access and compare the proposed diagnostic with the theme-ui benchmark. Done means an unsupported custom Button color produces a clear actionable error instead of the undefined-property TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react
Domain
developer-experience, frontend
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.