React: Bug: Blocking mode and events are broken (probably) for Material UI Rating component (example)

[ovr]

React version: 0.0.0-experimental-94c0244ba

Steps To Reproduce

import React from "react";
import Rating from "@material-ui/lab/Rating";
import "./styles.css";

export default function App() {
  const [rating, setRating] = React.useState(null);

  return (
    <div>
      Example
      <Rating
        name="rating"
        size="large"
        value={rating}
        onChange={(event, newValue) => {
          console.log(`change rating: ${newValue}`);

          setRating(newValue);
        }}
      />
    </div>
  );
}

ReactDOM
    .unstable_createBlockingRoot(document.querySelector('#app'), {
        hydrate: false,
    })
    .render(
        <App />
    )
;

Live Example

https://codesandbox.io/s/immutable-cloud-h3k2m?file=/src/App.js

Video (describe where is a bug)

https://www.youtube.com/watch?v=-jWTXdaIA6Y&t=1s

The current behavior

It's needed to click 2 times before onChange will be fired

The expected behavior

Should works as expected as it's working in legacy mode

ReactDOM.hydrate(
    <App />,
    document.querySelector('#app') as HTMLDivElement
);

Thanks

[bvaughn]

There is a problem with the sandbox you've shared. It's using legacy ReactDOM.render and even so, the first click shows an error in the console:

Warning: Failed prop type: Invalid prop value of type object supplied to ForwardRef(Rating), expected number.
at Rating (https://h3k2m.csb.app/node_modules/@material-ui/lab/esm/Rating/Rating.js:131:23)
at WithStyles(ForwardRef(Rating)) (https://h3k2m.csb.app/node_modules/@material-ui/styles/esm/withStyles/withStyles.js:47:31)
at div
at App (https://h3k2m.csb.app/src/App.js:24:40)

That's because you're wrapping the rating value in an object, but not extracting it when setting the value attribute. To fix this:

setRating({ rating: newValue }); // No
setRating(newValue); // Yes

Should works as expected as it's working in legacy mode

Not every React component is concurrent mode compatible. This is why we created the StrictMode API, although it can't identify all problems.

Can you repro the error you're reporting in a smaller sandbox? Something that does not require the Material UI library?

[ovr]

Warning: Failed prop type: Invalid prop value of type object supplied to ForwardRef(Rating), expected number.
at Rating (https://h3k2m.csb.app/node_modules/@material-ui/lab/esm/Rating/Rating.js:131:23)
at WithStyles(ForwardRef(Rating)) (https://h3k2m.csb.app/node_modules/@material-ui/styles/esm/withStyles/withStyles.js:47:31)
at div
at App (https://h3k2m.csb.app/src/App.js:24:40)

@bvaughn Oops, sorry, you are too fast, I didn't fix snippet before you click on it.

I added StrictMode API as you described, but didn't see any errors.

Current problem

1. Blocking mode - not working
2. Legacy mode - working

Can you repro the error you're reporting in a smaller sandbox? Something that does not require the Material UI library?

I don't know where is a bug, but it's caused only when I enable blocking mode, in legacy it's working great! It's why I opened a bug in react project instead of material-ui, because blocking mode is experimental and probably bug is here.

[eps1lon]

Reduced repro without Material-UI: https://codesandbox.io/s/onchange-onfocus-radio-change-interrupt-ddrd2

1. load page
2. click label
3. associated input is (sometimes) checked
4. click another label
5. nothing happens
6. click same label again
7. associated input is checked

So clicking a label is only guaranteed to call onChange on subsequent attempts. The first click seems to only sometimes work after initial pageload. This is only reproducible when clicking on the labels not when clicking on the <input type="radio" /> directly.

Setting state in onFocus seems to interrupt the onChange handler (or the timing is off I don't know yet).

[hamlim]

Setting state in onFocus seems to interrupt the onChange handler (or the timing is off I don't know yet).

This seems semi-related to the issue I had opened here: https://github.com/facebook/react/issues/18591

[bvaughn]

Nice repro, @eps1lon. Thanks!

[eps1lon]

Nice repro, @eps1lon. Thanks!

After digging more into this I'm fairly certain that this is indeed a duplicate of #18591 which has a more deterministic repro.

[bvaughn]

Excellent. I'll roll this one into that one then. Thanks!