[ClickAwayListener] Remove findDOMNode usage

[eps1lon]

Removes findDOMNode calls.

Breaking change

The child element of ClickAwayListener needs to be able to hold a ref.

[mui-pr-bot]

Details of bundle changes.

Comparing: ae24eea...aee82dc

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	-0.06%	+0.06% 🔺	349,543	349,346	89,881	89,933
@material-ui/core/Paper	0.00%	0.00%	67,853	67,853	19,809	19,809
@material-ui/core/Paper.esm	0.00%	0.00%	60,184	60,184	18,558	18,558
@material-ui/core/Popper	+0.30% 🔺	+0.30% 🔺	34,825	34,930	11,883	11,919
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	17,364	17,364	5,726	5,726
@material-ui/core/useMediaQuery	0.00%	0.00%	2,469	2,469	1,042	1,042
@material-ui/lab	+0.07% 🔺	+0.06% 🔺	147,331	147,436	43,555	43,581
@material-ui/styles	0.00%	0.00%	53,105	53,105	15,424	15,424
@material-ui/system	0.00%	0.00%	17,136	17,136	4,525	4,525
Button	+0.12% 🔺	+0.11% 🔺	87,919	88,024	26,055	26,083
Modal	+0.13% 🔺	+0.12% 🔺	82,035	82,140	24,553	24,582
colorManipulator	0.00%	0.00%	3,232	3,232	1,301	1,301
docs.landing	0.00%	0.00%	49,914	49,914	10,815	10,815
docs.main	-0.03%	+0.03% 🔺	645,251	645,055	200,422	200,482
packages/material-ui/build/umd/material-ui.production.min.js	-0.06%	+0.05% 🔺	297,961	297,772	82,664	82,704

Generated by 🚫 dangerJS against aee82dc

[oliviertassinari]

@eps1lon We have to solve the same problem with the Tooltip and TrapFocus components. How does this approach scale? The approach has a none negligeable downside for the developer experience. What other options to we have?

1. use cloneElement + ref
2. Wait v5 and RFC: createElement changes and surrounding deprecations reactjs/rfcs#107
3. ?

[eps1lon]

How does this approach scale?

What is scaling here?

none negligeable downside

What's the downside? I would rather add some code to avoid subtle bugs with findDOMNode.

use cloneElement + ref

I don't trust this approach to be honest. I'm not sure this properly updates if the ref value of the children changes.

[oliviertassinari]

What is scaling here?

Can we apply the same approach everywhere? Or will we have with an inconsistent mix of solutions for the same problem?

What's the downside?

Our users might see this
as "unnecessary" boilerplate (if we don't need it for 80% of the cases, and can warn for 20% of the time).

I don't trust this approach to be honest. I'm not sure this properly updates if the ref value of the children changes.

Have you tried it?

[eps1lon]

Have you tried it?

That's why I said I don't trust it not that it doesn't work.

Can we apply the same approach everywhere?

Why do we need this everywhere?

[oliviertassinari]

That's why I said I don't trust it not that it doesn't work.

Do you want me to prepare a test?

Why do we need this everywhere?

Because we want to make the usage of findDOMNode() safe in the whole code base.
The Tooltip and the Modal have the same code architecture as the ClickAwayLister. They need to access the host element of their PropTypes.element child.

[eps1lon]

That's why I said I don't trust it not that it doesn't work.

Do you want me to prepare a test?

I wouldn't be satisfied by any tests since it won't include concurrent react. What I was hoping for was some official docs for it.

I thought about it a bit more and it seems obvious that this "just" works: https://codesandbox.io/s/n730ozwrrl. I'm just a little bit uncomfortable with all the ref forking without proper cleanup.

[oliviertassinari]

Yes, we will be able to kill this :)