React: Bug: (17.0.0-rc.0) Event propagation through portals is inconsistent

[nstepien]

React version: 17.0.0-rc.0

Steps To Reproduce

1. Open the codesandbox demo link below.
2. Click on the root and portal divs, check the logs.
3. Uncomment the portal div's onClickCapture noop handler, check the logs again.

Link to code example: https://codesandbox.io/s/determined-montalcini-vjrgc?file=/src/App.js

The current behavior

Clicking on the portal div logs "portal click" only.
Adding an onClickCapture noop handler on the portal div "fixes" the root's onClickCapture handler.
You might have to refresh the page between edits, otherwise the root's onClickCapture handler might keep working even after removing the portal's onClickCapture handler.

The expected behavior

Clicking on the portal div should trigger the root's onClickCapture handler, whether the portal div has an onClickCapture handler or not.

[gaearon]

Thanks for reporting!

[koba04]

I've investigated the issue and figured out why this happens.
React registers event handlers to a root container, with Portal, the root container is the container of Portal. React listens to events, which are only registered event handlers on components within Portal, at the container of Portal. It means that events that are not registered in Portal container don't propagate outside the Portal.
To fix this, React should listen to events, which are registered event handlers on all parent root containers, at the Portal container to propagate them.
But I'm not sure that is the right way to fix this issue.

The following is a minimum case for this issue.

container = document.createElement('div');
const onClick= jest.fn();

const ref = React.createRef();
ReactDOM.render(
  <div onClick={onClick}>
    {ReactDOM.createPortal(
      <button ref={ref}>click</button>,
      document.body
    )}
  </div>,
  container
);
const event = new MouseEvent('click', {
  bubbles: true,
});
ref.current.dispatchEvent(event);

expect(onClick).toHaveBeenCalledTimes(1); // 0

[diegohaz]

The funny thing is that this bug actually fixes #11387

And I would consider this the desired behavior 99% of the time.

[gaearon]

I wouldn't say it "fixes" it, it's just an inconsistent behavior. I understand the desire for a different behavior there but let's not spill it off in this thread. (My latest reply is https://github.com/facebook/react/issues/16721#issuecomment-674807919.)

[nstepien]

@gaearon Thanks! Can we expect a rc.1 with the fix?

[gaearon]

Yes, but it will take a bit of time for us to test it internally. It's a pretty significant internal change.

[gaearon]

Fixed in 17.0.0-rc.1.
https://codesandbox.io/s/agitated-wiles-t27jm?file=/src/App.js

[targos]

Is there a way to know what commits have been added between 17.0.0-rc.0 and 17.0.0-rc.1. There seems to be no tags on GitHub for those versions.

[gaearon]

https://github.com/reactjs/reactjs.org/pull/3231