[progress] Show runtime errors only once#48591
Conversation
Deploy previewhttps://deploy-preview-48591--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new errorOnce utility in @mui/utils and adopts it in LinearProgress and CircularProgress to avoid repeating the same dev-time warnings/errors across renders (notably in React Strict Mode), addressing console spam reported for progress components.
Changes:
- Add
@mui/utils/errorOnce(with reset support) and export it from@mui/utils. - Replace repeated
console.warn/console.errorcalls inLinearProgressandCircularProgresswitherrorOnce. - Update component tests to reset the
errorOncecache and to assert single-emission logging behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mui-utils/src/index.ts | Exports the new errorOnce utility (and its reset alias) from the root utils entrypoint. |
| packages/mui-utils/src/errorOnce/index.ts | Adds the subpath entrypoint for @mui/utils/errorOnce. |
| packages/mui-utils/src/errorOnce/errorOnce.ts | Implements dev-only “log once” behavior with an optional key and a reset function. |
| packages/mui-utils/src/errorOnce/errorOnce.test.ts | Adds unit tests for errorOnce behavior and reset semantics. |
| packages/mui-material/src/LinearProgress/LinearProgress.js | Uses errorOnce to dedupe dev warnings/errors for invalid props. |
| packages/mui-material/src/LinearProgress/LinearProgress.test.js | Updates expectations for deduped logs and resets cache between cases. |
| packages/mui-material/src/CircularProgress/CircularProgress.js | Uses errorOnce to dedupe dev warnings/errors for invalid props. |
| packages/mui-material/src/CircularProgress/CircularProgress.test.js | Updates expectations for deduped logs and resets cache between cases. |
Comments suppressed due to low confidence (4)
packages/mui-material/src/LinearProgress/LinearProgress.test.js:7
strictModeDoubleLoggingSuppressedis imported but no longer used in this test file. This will fail lint/type-check (unused import) and should be removed.
import {
createRenderer,
screen,
strictModeDoubleLoggingSuppressed,
} from '@mui/internal-test-utils';
packages/mui-material/src/CircularProgress/CircularProgress.test.js:7
strictModeDoubleLoggingSuppressedis imported but no longer used in this test file. This will fail lint/type-check (unused import) and should be removed.
import {
createRenderer,
strictModeDoubleLoggingSuppressed,
screen,
} from '@mui/internal-test-utils';
packages/mui-material/src/LinearProgress/LinearProgress.test.js:7
strictModeDoubleLoggingSuppressedis imported but no longer used in this test file. This will fail lint/type-check (unused import) and should be removed.
import {
createRenderer,
screen,
strictModeDoubleLoggingSuppressed,
} from '@mui/internal-test-utils';
packages/mui-material/src/CircularProgress/CircularProgress.test.js:7
strictModeDoubleLoggingSuppressedis imported but no longer used in this test file. This will fail lint/type-check (unused import) and should be removed.
import {
createRenderer,
strictModeDoubleLoggingSuppressed,
screen,
} from '@mui/internal-test-utils';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| } | ||
| } | ||
| errorOnce( |
There was a problem hiding this comment.
Moving the NODE_ENV check would cause a bit more work here as the logic to compute the arguments are no longer skipped since they are evaluated before errorOnce runs
Might be easier to keep the check at all the call sites?
Codex tried to pitch this alternative API to avoid this issue but I'm not convinced:
function errorOnce(
key: string,
level: ErrorOnceLevel,
getMessage: () => string | null | false,
) {
...
}There was a problem hiding this comment.
or maybe the message could be string | () => string. we might over-engineer this, let me see how it will look without the helper and do the checks on site.
e7d9827 to
91a464a
Compare
Fixes #48562.