Skip to content

[docs] Document ref forwarding (requirements)#15298

Merged
eps1lon merged 11 commits into
mui:nextfrom
eps1lon:docs/v4/refs
Apr 14, 2019
Merged

[docs] Document ref forwarding (requirements)#15298
eps1lon merged 11 commits into
mui:nextfrom
eps1lon:docs/v4/refs

Conversation

@eps1lon

@eps1lon eps1lon commented Apr 10, 2019

Copy link
Copy Markdown
Member

Close #14415


TODO:

eps1lon added 2 commits April 10, 2019 14:27
- include children (used in Slide and future Tooltip)
- be more specific in strict mode caveat
@eps1lon eps1lon added the docs Improvements or additions to the documentation. label Apr 10, 2019
@eps1lon eps1lon added this to the v4 milestone Apr 10, 2019
@mui-pr-bot

mui-pr-bot commented Apr 10, 2019

Copy link
Copy Markdown

No bundle size changes comparing 5236588...64e2488

Generated by 🚫 dangerJS against 64e2488

@oliviertassinari

oliviertassinari commented Apr 10, 2019

Copy link
Copy Markdown
Member

Should we add a soft deprecation message for RootRef in the documentation? https://next.material-ui.com/api/root-ref/#rootref-api

@eps1lon
eps1lon marked this pull request as ready for review April 11, 2019 15:20
@eps1lon
eps1lon requested a review from mbrookes April 11, 2019 15:20

@mbrookes mbrookes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"?" in a review comment means I wasn't sure...

Comment thread docs/src/pages/getting-started/faq/faq.md Outdated
Comment thread docs/src/pages/guides/api/api.md Outdated
Comment thread docs/src/pages/guides/api/api.md Outdated
Comment thread docs/src/pages/guides/api/api.md Outdated
Comment thread docs/src/pages/guides/composition/composition.md Outdated
Comment thread docs/src/pages/guides/composition/composition.md Outdated
If you use class components for the cases described above you will still see
warnings in `React.StrictMode` and `React.unstable_ConcurrentMode`. We use
`ReactDOM.findDOMNode` internally for backwards compatibility. You can use `React.forwardRef`
and a designated prop in your class component to forward the `ref` to a built-in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and a designated prop in your class component to forward the `ref` to a built-in
and a designated prop in your class component to forward the `ref` to a DOM element.

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest I don't know how to properly explain this. Maybe I'm using the term ref wrong maybe it isn't well defined.

If I refer to the term ref then I mean the return value from e.g. React.createRef(). So the ref is the full object { current: instance }. What we end up is attaching the ref object { current: ... } to the host component i.e. <div ref={{ current: ... }} />. Or in other words the ref is the thing you pass to the ref prop1 of a component. You'll probably still catch me doing <Component ref={ref => ...} /> where naming the parameter ref is probably a misnomer. <Component ref={current => ...} /> should be clearer.

The DOM element will then be ref.current. Or more generally ref.current will point to the component instance. For host components rendered with react-dom it will be an instance of a DOM Element.

Please tell me if that doesn't make sense. In the end the most important thing is that we don't confuse the reader. It's not that important to be technically accurate but that one understands how to fix the issue at hand.

1 ref gets a special treatment so calling it a ref might not be 100% accurate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and a designated prop in your class component to forward the `ref` to a built-in
and a designated prop in your class component to forward the `ref` to a DOM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I found confusing was the terminology "built-in component". What exactly does that refer to?

(I also overlooked the fact that a ref is a pointer to the full object, not just a discrete DOM element, so my suggested replacement wasn't any more helpful.)

Comment thread docs/src/pages/guides/composition/composition.md
Comment thread docs/src/pages/guides/composition/composition.md
Comment thread docs/src/pages/guides/composition/composition.md
Comment thread docs/src/pages/guides/composition/composition.md Outdated
buit-in, host -> DOM

Since we only support react-dom renderer the host is equivalent to the DOM.
@eps1lon
eps1lon requested a review from mbrookes April 13, 2019 15:18
Comment thread docs/src/pages/getting-started/faq/faq.md Outdated
Comment thread docs/src/pages/guides/composition/composition.md Outdated
Comment thread packages/material-ui/src/ClickAwayListener/ClickAwayListener.js Outdated
Comment thread packages/material-ui/src/Slide/Slide.js Outdated
Comment thread pages/api/click-away-listener.md Outdated
Comment thread pages/api/slide.md Outdated
Comment thread docs/src/modules/utils/generateMarkdown.js Outdated
Comment thread pages/api/button-base.md Outdated
@eps1lon
eps1lon merged commit 8ae684d into mui:next Apr 14, 2019
@eps1lon
eps1lon deleted the docs/v4/refs branch April 14, 2019 10:08
@eps1lon eps1lon mentioned this pull request Apr 14, 2019
99 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC] Refs in v4

4 participants