Share ref with multiple ref handlers
joaovieira opened this issue · 37 comments
Before React 16.3 we were able to proxy the same element ref to multiple listeners, for example, to grab hold of the element for internal purposes and also expose it publicly, like so:
class MyComponent extends Component {
attachRef = el => {
this.buttonEl = el;
this.props.buttonRef(el);
}
// Do something with `this.buttonEl`
render () {
return <button ref={this.attachRef}>Button</button>;
}
}After React 16.3 this is more complicated as the ref prop can be a function or an object:
class MyComponent extends Component {
buttonRef = React.createRef();
attachRef = el => {
this.buttonRef.current = el;
if (typeof this.props.inputRef === 'function') {
this.props.inputRef(el);
} else {
this.props.inputRef.current = el;
}
}
// Do something with `this.buttonRef.current`
render () {
return <button ref={this.attachRef}>Button</button>;
}
}First, is this the right approach?
And if so, Typescript types say:
interface RefObject<T> {
readonly current: T | null;
}Which prevents us from assigning to current. Shouldn't it not be readonly?
The approach you're suggesting should work. Yeah, it's more complicated. But we also don't encourage often doing this as it breaks encapsulation.
The property is usually read-only in the way it's currently used, but in your case it's fair to want to write to it. It's possible that we'll ask to change that type to be writeable in the future. (We don't maintain the typings ourselves.)
@gaearon
Is there any update on writeability of the current?
The problem of composing the refs is still relevant.
If the property should be writeable - I need you to confirm that - I can open a PR in typings repo.
But it looks for me as it should not be.
Maybe it should be allowed to pass an array of RefObjects to ref property of the element?
If someone can't find a workaround for their findDOMNode use case please file a new issue and succinctly describe your use case. Preferably with an example. Thanks!
Just wrote this general ref-"merger" or "resolver" accepting as many refs as you'd like. Added typings as well.
@Gearon According to your previous comment, doing it this way would have no negative effects, right?
export const mergeRefs = <T>(...refs: Array<Ref<T>>) => (ref: T) => {
refs.forEach((resolvableRef) => {
if (typeof resolvableRef === "function") {
resolvableRef(ref);
} else {
(resolvableRef as any).current = ref;
}
});
};
@luddzo in some scenarios, when mergeRefs is called as part of the render, your implementation can make a DOM element repeatedly unload and load because the returned function is a new object on every call.
I was using react-measure, there may be some weird ordering going on, but to fix it, I had to memorize the returned function so it wasn't always new.
@ngbrown the same principle applies for everything in React, changing props triggers an update.
This is particularly useful for hooks now, because you might have two pieces of encapsulated logic that they should work toghether.
For instance, let's say we have two hooks:
useHeight(): [height: number, ref: Ref<HTMLElement>]=> whenever ref mounts, it gets the height of that element, and listens to resize events that updates height.useScrollY(): [scrollY: number, ref: Ref<HTMLElement>]=> whenever ref mounts, it gets the current scroll position of the element, and listens to scroll events that updates the scrollY.
In this use case, we need to pass those refs to the same element, so we need a way of combining them.
For anyone trying to allow a forwarded ref while also needing to access the ref internally regardless of whether a ref was passed in or not - here is how I did this.
UPDATED:
Just use this guys npm package it works really well, is built with typescript and I've switched to it and like it a lot.
https://www.npmjs.com/package/@seznam/compose-react-refs
thanks for letting us know about this ☝️ @jurca
@gaearon Do you have any thoughts on this use-case and the pattern described above ?
@StJohn3D Your example doesn't work if the ref passed in is a function. (That's why Typescript presumably was giving you a type error that you had to override with that as expression.) (Function refs have use-cases that mutable object refs don't address, so I think it's wrong to assume any new code can always use mutable object refs.)
@Macil I assume you're referring to the callback pattern? Thanks for pointing that out! I'll have to do some more experiments later to see what happens.
Two iterations I went over in forwardRef components that also need their own ref:
helpers
// simplified commitAttachRef from https://github.com/facebook/react/blob/1b752f1914b677c0bb588d10a40c12f332dfd031/packages/react-reconciler/src/ReactFiberCommitWork.js#L693
function setRef(ref, value) {
if (typeof ref === 'function') {
ref(value);
} else if (ref !== null) {
ref.current = value;
}
}First version:
export function useForkRef(refA, refB) {
/**
* This will create a new function if the ref props change.
* This means react will call the old forkRef with `null` and the new forkRef
* with the ref. Cleanup naturally emerges from this behavior
*/
return React.useCallback(
instance => {
setRef(refA, instance);
setRef(refB, instance);
},
[refA, refB],
);
}Second version
export function useForkRef(refA, refB) {
/**
* This will create a new function if the ref props change and are defined.
* This means react will call the old forkRef with `null` and the new forkRef
* with the ref. Cleanup naturally emerges from this behavior
*/
return React.useMemo(() => {
if (refA == null && refB == null) {
return null;
}
return instance => {
setRef(refA, instance);
setRef(refB, instance);
};
}, [refA, refB]);
}Forking refs in a non-hooks world requires a lot more code to properly cleanup old refs i.e. setting old current to null on change. With hooks this behavior naturally emerges.
It's not perfect since it re-attaches a ref if any fork changes. This can trigger unnecessary calls for callback refs. If you require minimal calls of your passed callback refs you need to diff each one individually. So far I haven't had this issue which is why I prefer this simplistic version.
The second version supports a special case where forwarded ref and child ref are null and the component in question is a function component. The first version would trigger warnings (function components cannot be given refs). I recommend using the first version until you have a use case for version 2.
Both can be adjusted for variadic arguments if you need to.
Just if anyone is interested, here it's my implementation (in typescript):
/**
* Combines many refs into one. Useful for combining many ref hooks
*/
export const useCombinedRefs = <T extends any>(...refs: Array<Ref<T>>): Ref<T> =>
useCallback(
(element: T) =>
refs.forEach(ref => {
if (!ref) {
return;
}
// Ref can have two types - a function or an object. We treat each case.
if (typeof ref === 'function') {
return ref(element);
}
// As per https://github.com/facebook/react/issues/13029
// it should be fine to set current this way.
(ref as any).current = element;
}),
refs
);This is a hook that accepts any number of any kind of ref (Ref object or ref callback) and returns another ref as a callback.
Of course the amount of refs passed to this hook can't change, otherwise useCallback will fail.
if (!ref) {
This means you're not cleaning up properly. The refs will still hold a reference to component instances even after they're unmounted. What do you need this behavior for?
I misread.
Note that this is not !ref.current, so if ref is null/undefined it's really not holding a reference to anything. it's an empty variable.
Type definitions in typescript for Ref<T> are defined as:
type Ref<T> = (instance: T | null) => void | RefObject<T> | null
So you can technically call useCombinedRefs as
const dummyRef: Ref<any> = null;
const ref = useCombinedRefs(null, dummyRef, null);and is complying within the type Ref<T>. To not fall into the exception when doing ref.current = element (can't set property current of null), we need to run that check.
Edit - And now I realise: You were doing the exact same check in setRef:
} else if (ref !== null) {
ref.current = value;
}Yeah nevermind you're guarding against an empty input not an empty instance passed in the created ref callback. All good 👍
Thanks @Macil, @eps1lon, & @voliva for your feedback. I added a test case that uses useCallback and worked backwards from there to see what solutions would work. I ultimately arrived at something very simple that just uses @eps1lon 's setRef helper function. I've updated my original post to reflect the changes I've made in my project. It could probably be simplified even more into a custom hook or function but for now I'm just happy knowing that it works for both use-cases.
Thanks again everyone!
In case anyone is interested in implementation that does not require react's hooks and therefore can be used in any component, you may want to check out this little package: https://www.npmjs.com/package/@seznam/compose-react-refs
React's documentation gives us a big hint (emphasis mine):
React also supports another way to set refs called “callback refs”, which gives more fine-grain control over when refs are set and unset.
This tells me that there are things that simply cannot be done with object refs. Accept their limitations instead of trying to shoehorn them into use cases they don't fit.
My personal opinion is that object refs were a bad idea that created problems rather than solving them, but please correct me if I'm wrong. We have two non-deprecated, non-legacy APIs for the same thing and when we start building components and HOCs that take refs, we have to account for both types of ref and so our code gets super-complicated and/or there are things we just cannot do that we could previously do because we had only callback refs.
@steve-taylor callback refs have always been there, while object refs were made to replace the old ref API which worked with strings (see changelog for 16.3)
Object refs are now handy - and in a way essential when you use hooks, as it's the only way to have a mutable value that won't be tracked in React's lifecycle. Of course the use cases to deal with those should be limited, but sometimes it's needed.
We could argue whether <Component ref={...}> should only accept callbacks and not objects, given that if you need to store that in an object, you could just do:
class MyComponent {
_ref = null;
_refCb = ref => this._ref = ref;
render() {
return <SubComponent ref={this._refCb} />;
}
}or
const MyComponent = () => {
const ref = useRef();
const refCb = useCallback(r => ref.current = r);
return <SubComponent ref={refCb} />;
}But I personally see more convenient (and less boilerplate-y) to allow passing in just the object, which probably covers 99% of the cases. The places where we need to deal with the object vs callback stuff it's very limited compared to just using ref objects (specially with hooks)
You can write a custom hook that composes multiple refs:
function useCombinedRefs(...refs) {
const targetRef = useRef();
useEffect(() => {
refs.forEach(ref => {
if (!ref) return;
if (typeof ref === 'function') {
ref(targetRef.current);
} else {
ref.current = targetRef.current;
}
});
}, [refs]);
return targetRef;
}Usage:
<div ref={useCombinedRefs(ref1, ref2)} />This supports both object and function refs.
EDIT: This implementation also supports useLayoutEffect:
export default function useCombinedRefs(...refs) {
return target => {
refs.forEach(ref => {
if (!ref) return;
if (typeof ref === 'function') {
ref(target);
} else {
ref.current = target;
}
});
};
}@amannn doesn't the useCombinedRefs argument ...refs cause a new array to be created every render, and because it's a dependency of the useEffect, invalidate the effect every render?
@jaydenseric Yep, I think you're right.
Refs should have been split into getter and setters to avoid this invariance hell.
const Foo = () => {
const ref = useRef();
useEffect(() => {
console.log(ref.get().height);
}, []);
return <div ref={ref.set} />;
}
Refs should have been split into getter and setters to avoid this invariance hell.
const Foo = () => { const ref = useRef(); useEffect(() => { console.log(ref.get().height); }, []); return <div ref={ref.set} />; }
I'm afraid I fail to see how that would help compared to the current state:
const Foo = () => {
const ref = useRef();
useEffect(() => {
console.log(ref.current.height);
}, []);
return <div ref={ref} />;
}Could you please elaborate?
@jurca https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science)
Your example:
const Foo = () => {
// Let's assume I have a ref of HTMLElement
// Because I'll use this ref with a hook that works with any HTMLElement
const ref = useRef<HTMLElement>();
useEffect(() => {
console.log(ref.current.height);
}, []);
return <div ref={ref} />;
}
For the sake of clarity, let's say div is a function as below:
function div(ref: Ref<HTMLDivElement>): JSXElement;
Due to contravariance and because ref combines a getter and a setter, you can only pass a Ref<HTMLDivElement> as argument.
If div was accepting a setter callback instead:
function div(ref: (el: HTMLDivElement) => void): JSXElement;
Due to double contravariance, div can be called by any callback that takes an HTMLDivElement or any of its subtypes such as an HTMLElement.
Why the hell is this contravariance important?
Typechecker is actually pointing out a legitimate problem: <div>'s ref property says that it may try to read the ref which it expects to be at least an HTMLDivElement, but we are passing a ref of HTMLElement. HTMLElement doesn't have all the properties/methods of an HTMLDivElement. In reality, we know that a <div ref={ref}> will just set the ref, will not try to read it. So in short, getter+setter combination is not the minimal API required for ref property to work, we shot ourselves in the foot.
What I'm wondering is how React team made this mistake. I thought they were still heavily using Flow.
Ah, now I see what you mean - you are using Flowtype. I got confused for a moment there, because there is no covariance issue with TypeScript (which I mistakenly assumed you were using).
You see, there is no such issue with TypeScript, the following code works correctly and using the ref on an element of a different type would result in a compile error:
const ref = useRef<HTMLDivElement>()
return <div ref={ref}/>That being said, I do acknowledge that "just switch to TypeScript" is hardly an advice you want to hear. If I were you, I would look into callback refs and whether they suffer from the same issue. If they don't suffer from this issue, use that to your advantage and build your own callback ref factory function that exposes the API to your liking.
If, however, the callback refs suffer from the same issue, I would recommend reporting that to the flow project. It is somewhat unfortunate that flowtype makes it harder (from my outdated perspective, and this is subjective) to specify the type declarations you want to use for a library and/or patch them if needed, especially when it comes to react.
Hey, thanks for trying to help. I'll try to clarify things in bullet points so as to make it easier to read. I created a sandbox instead.
- I'm using Typescript.
- Every language that properly implements subtyping has this problem (Flow, Typescript (in strict mode), FFI for Reason, Purescript, Kotlin etc.). Usually, FFI languages come up with a different API because the existing RefObject API has design shortcomings, as showcased in the sandbox.
For detailed explanation, I created a sandbox with thorough comments: https://codesandbox.io/s/react-ref-covariance-i2ln4
I see... Unfortunately, I have faced the same issue with various type systems, it seems that covariance and contravariance do not seem to work that well with generics.
I would recommend going with that custom ref factory function for your use case:
type RefCallback<T> = Exclude<NonNullable<React.Ref<T>>, React.RefObject<T>>
type RefCallbackWithValue<T> = RefCallback<T> & {
readonly value: null | T
}
function makeRef<T>(defaultValue: null | T): RefCallbackWithValue<T> {
let current = defaultValue
const ref: RefCallback<T> = (value: null | T) => {
current = value
}
Object.defineProperty(ref, 'value', {
enumerable: true,
get() {
return current
},
})
return ref as RefCallbackWithValue<T>
}
const magicRef = makeRef<HTMLElement>(null);
<div ref={magicRef} />@jurca Hmm I guess I miscommunicated what I think is wrong here. Generics, co/contravariance and subtyping work just fine. They work as intended. The problem is, React has introduced an API that won't work for sound type systems and some unsound ones too (e.g. partial soundness for subtyping in Typescript).
I can use my own utility functions to keep using ref callbacks, I'm just afraid that React team may decide to deprecate it.
In the end, I'd really appreciate if this problem is acknowledged as early as possible, because 3rd party libraries and hooks will start to make more use of ObjectRefs for which there'll be no workaround except for type-casting.
@gaearon You are probably interested in this discussion.
I can use my own utility functions to keep using ref callbacks, I'm just afraid that React team may decide to deprecate it.
Well, I guess we may never know what the future holds. But I think it's better to use the utility, because when the API changes, all you will need to change is the utility itself (unless the API changes too much, of course).
Also, you may use the utility itself to showcase a suggestion how the React's API should change. Who knows, maybe they'll listen to you a design an API that will be better suited for this. :)
Here is my approach
import { MutableRefObject, useRef } from 'react';
type InputRef<T> = ((instance: T | null) => void) | MutableRefObject<T | null> | null;
/**
* Works like normal useRef, but accepts second argument which is array
* of additional refs of the same type. Ref value will be shared with
* all of those provided refs as well
*/
export function useSharedRef<T>(initialValue: T, refsToShare: Array<InputRef<T>>) {
// actual ref that will hold the value
const innerRef = useRef<T>(initialValue);
// ref function that will update innerRef value as well as will publish value to all provided refs
function sharingRef(value: T) {
// update inner value
innerRef.current = value;
// for each provided ref - update it as well
refsToShare.forEach((resolvableRef) => {
// react supports various types of refs
if (typeof resolvableRef === 'function') {
// if it's functional ref - call it with new value
resolvableRef(value);
} else {
// it should be ref with .current prop
// make sure it exists - if so - assign new value
if (resolvableRef) {
(resolvableRef as any).current = value;
}
}
});
}
/**
* We want ref we return to work using .current, but it has to be function in order
* to share value with other refs.
*
* Let's add custom get property called 'current' that will return
* fresh value of innerRef.current
*/
if (!(sharingRef as any).current) {
Object.defineProperty(sharingRef, 'current', {
get() {
return innerRef.current;
},
});
}
return sharingRef as typeof sharingRef & { current: T };
}It can be used like:
const Foo = forwardRef<HTMLDivElement, {bar: number}>((props, refToForward) => {
const ref = useSharedRef<HTMLDivElement | null>(null, [refToForward]);
return <div ref={ref}>baz</div>
})Unfortunately all the solutions posted in this thread have issues and do not handle all the possible cases correctly. So I had to create my own: react-ref-composer
What's wrong with other solutions?
Update all refs when one ref changes
This is the most common issue and compose-react-refs also struggles from it. Don't get me wrong, I like the elegant approach of compose-react-refs with WeakMap, but unfortunately it breaks assumptions around React refs. Consider:
function MyComponent(){
const ref1 = useCallback(div => console.log('ref1', div), [])
const ref2 = useCallback(div => console.log('ref2', div), [])
const ref3 = useCallback(div => console.log('ref3', div), [])
const [flag, setFlag] = useState(true)
function onSwitch() {
console.log('switching')
setFlag(f => !f)
}
return <div ref={composeRefs(ref1, flag ? ref2 : ref3)}>
<button onClick={onSwitch}>Switch refs</button>
</div>
}This is what the expected output looks like when the user clicks the button:
ref1 <div>
ref2 <div>
switching
ref2 null
ref3 <div>So the old ref resets to null and the new ref is set to the DOM node as expected.
However with compose-react-refs and other similar libraries this happens:
ref1 <div>
ref2 <div>
switching
ref1 null
ref2 null
ref1 <div>
ref3 <div>Essentially ref1 goes through reset/set cycle. This will trick the consumer of that ref into thinking that the component got unmounted and remounted again, which can be harmful in some cases (e.g. during drag-and-drop operation) and cause undesired behaviour.
Other code that falls into the same trap: #1, #2
Update all refs on every render
Can be fixed by memoisation but then will have the same problem as above.
Forget to update ref when it changes
OP's code struggles from this. The output will look like this:
ref1 <div>
ref2 <div>
switchingref3 will never get its value, ref2 will never reset to null. (In OP's case this.props.inputRef changes are ignored.)
Forget to reset old ref
Basically all the solutions that use useEffect. The output (at best) will look like this:
ref1 <div>
ref2 <div>
switching
ref3 <div>ref2 will never reset to null: #4 and others
@Torvin Thank you for pointing this out!
While an interesting use case, it wasn't something that compose-react-refs was/is meant to handle - you are expected to always provide all the same sub-refs in the same order for use on the same react element (I'm not sure if there could be a case where it would not be possible to do).
That being said, I recognize that some developers might find it, for whatever reason, useful to do that. In such cases, react-ref-composer seems to be a good fit, at the cost of slightly worse (for understandable reasons!) ergonomics.
I have considered adding support for this to compose-react-refs, however, I do not see a way to do it without changing the API (and without accessing React's internals). If anyone has a suggestion on how to do it, please let me know.
@Torvin One more thought: using Set or using Array.prototype.includes should be enough to enable you to remove the current limitation of always having to provide the same number of arguments to refComposer. I can provide a pull request, if you are interested 😃 .
Edit: If anyone is interested in having support for this use case in compose-react-refs, even at the cost of changing the API, please let me know. I will, however, have to release it as a new major version 😉 . I'll do my best to do as little change in the API as possible.
@jurca thanks! My first impulse was to fix compose-react-refs but like you said I don't think it's possible without breaking the API. At least I couldn't find a way, so I made react-ref-composer instead.
One more thought: using Set or using Array.prototype.includes should be enough to enable you to remove the current limitation of always having to provide the same number of arguments to refComposer
Thanks, I was considering that, but decided it was an overkill since it's almost never useful (that's not how you typically call react hooks and I've never come across a situation where I want to pass variable number of refs). So I decided in the end that slightly worse performance of includes/Set wasn't worth it 🙂
Thanks, I was considering that, but decided it was an overkill since it's almost never useful (that's not how you typically call react hooks and I've never come across a situation where I want to pass variable number of refs). So I decided in the end that slightly worse performance of
includes/Setwasn't worth it
Very well, should you change your mind in the future, and need a helping hand, just let me know 🙂 .