Fix <Notification> causes undoable notifications to be commited twice in MUI v6 - #11345
Conversation
| const Mui6Snackbar = ReactModule.forwardRef<any, any>((props, ref) => { | ||
| const wasOpen = ReactModule.useRef(props.open); | ||
|
|
||
| ReactModule.useEffect(() => { | ||
| if (wasOpen.current && !props.open) { | ||
| props.slotProps?.transition?.onExited?.(null); | ||
| props.TransitionProps?.onExited?.(null); | ||
| } | ||
| wasOpen.current = props.open; | ||
| }, [props.open, props.slotProps, props.TransitionProps]); | ||
|
|
||
| ReactModule.useEffect(() => { | ||
| if (!props.open) return; | ||
|
|
||
| document.addEventListener('click', props.onClose); | ||
| return () => document.removeEventListener('click', props.onClose); | ||
| }, [props.open, props.onClose]); | ||
|
|
||
| if (!props.open) return null; | ||
|
|
||
| return ReactModule.createElement( | ||
| 'div', | ||
| { ref }, | ||
| props.children ?? props.message, | ||
| props.action, | ||
| ReactModule.createElement( | ||
| 'button', | ||
| { onClick: props.onClose }, | ||
| 'Close notification' | ||
| ) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
I really don't understand why you need to reimplement the Snackbar to test mui v6. Can you explain?
There was a problem hiding this comment.
You're right; reimplementing Snackbar was unnecessary. Removed the module mock in 3eaa674. The regression now exercises the real MUI Snackbar and triggers its click-away exit path.
| mockMuiMajor = 5; | ||
| }); | ||
|
|
||
| it.each([6, 7, 9])( |
There was a problem hiding this comment.
I don't think that it makes much sense to test all versions. I'd prefer that you create a test that only runs if the MUI major it >=6, with no mock. And then explain in the PR description that, in order to test you change, te tests must pass with devdeps using mui V5 and mui V6.
There was a problem hiding this comment.
Updated in 3eaa674. The regression now runs only when the installed MUI major is 6 or newer, with no MUI mock. I verified the file against MUI 5.16.14 (3 passed, 1 expected skip) and MUI 6.4.10 (4 passed), and updated the PR description with the two-version test requirement.
|
Thanks! |
fzaninotto
changed the title
<Notification> causes undoable notifications to be commited twice in MUI v6
2 participants
Problem
With MUI 6 and later,
Notificationpasses the sameonExitedcallback through both the legacyTransitionPropsAPI andslotProps.transition. MUI invokes both callbacks, causing an undoable mutation to be committed twice.Fixes #11334
Solution
Use
TransitionPropsandContentPropsonly with MUI 5. For MUI 6 and later, use the slot props API exclusively while preserving the existing user prop override order.How To Test
The notification regression suite must pass with the development dependencies installed against both MUI 5 and MUI 6. With MUI 5, the MUI 6+ regression is expected to be skipped; with MUI 6, it runs against the real Snackbar implementation.
yarn jest packages/ra-ui-materialui/src/layout/Notification.spec.tsx --runInBandyarn jest packages/ra-core/src/notification/useNotify.spec.tsx --runInBandyarn workspace ra-core buildyarn workspace ra-ui-materialui buildyarn tsc --noEmit -p packages/ra-ui-materialui/tsconfig.jsonAdditional Checks
masterfor a bug fix