Skip to content

Conversation

@JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Mar 10, 2020

The problem

In mui there's a concept of forking a ref, this means that we combine the passed ref from the top with the one used internally inside a component to manage presentation.

An example: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Grow/Grow.js#L32

This becomes problematic in the following scenario

const Wrapper = forwardRef((props, ref) => <div ref={ref}>I am wrapped</div>);
const App = () => {
  const ref = useRef()
  return (
    <Grow>
       <Wrapper ref={ref} />
    </Grow>
  )
}

At this point we have evaluated Wrapper and moved the vnode.ref to vnode.props.ref due to options.vnode being called (createVNode). This makes the forking of the Grow ref fail since we talk to children.ref which in this case is Wrapper.ref. That's why we move the forwarding of refs to the point where we are going to diff the forwarded component.

Fixes: #2249

@github-actions
Copy link

github-actions bot commented Mar 10, 2020

Size Change: +6 B (0%)

Total Size: 38.5 kB

Filename Size Change
compat/dist/compat.js 3.12 kB +3 B (0%)
compat/dist/compat.module.js 3.14 kB +1 B
compat/dist/compat.umd.js 3.17 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
debug/dist/debug.js 2.98 kB 0 B
debug/dist/debug.module.js 2.96 kB 0 B
debug/dist/debug.umd.js 3.04 kB 0 B
devtools/dist/devtools.js 175 B 0 B
devtools/dist/devtools.module.js 185 B 0 B
devtools/dist/devtools.umd.js 252 B 0 B
dist/preact.js 3.72 kB 0 B
dist/preact.min.js 3.72 kB 0 B
dist/preact.module.js 3.73 kB 0 B
dist/preact.umd.js 3.78 kB 0 B
hooks/dist/hooks.js 1.05 kB 0 B
hooks/dist/hooks.module.js 1.08 kB 0 B
hooks/dist/hooks.umd.js 1.13 kB 0 B
test-utils/dist/testUtils.js 390 B 0 B
test-utils/dist/testUtils.module.js 392 B 0 B
test-utils/dist/testUtils.umd.js 469 B 0 B

compressed-size-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Material-UI 4: popover loose positioning

3 participants