-
Notifications
You must be signed in to change notification settings - Fork 46.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Share ref with multiple ref handlers #13029
Comments
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 |
If someone can't find a workaround for their |
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?
|
@luddzo in some scenarios, when I was using |
@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:
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 |
@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 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 |
I misread. |
Note that this is not Type definitions in typescript for So you can technically call useCombinedRefs as const dummyRef: Ref<any> = null;
const ref = useCombinedRefs(null, dummyRef, null); and is complying within the type Edit - And now I realise: You were doing the exact same check in } 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. |
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):
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 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 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 |
@jaydenseric Yep, I think you're right. |
Refs should have been split into getter and setters to avoid this invariance hell.
|
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:
For the sake of clarity, let's say
Due to contravariance and because If
Due to double contravariance, Why the hell is this contravariance important? Typechecker is actually pointing out a legitimate problem: 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.
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. |
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: What's wrong with other solutions? Update all refs when one ref changesThis is the most common issue and 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 However with ref1 <div>
ref2 <div>
switching
ref1 null
ref2 null
ref1 <div>
ref3 <div> Essentially Other code that falls into the same trap: #1, #2 Update all refs on every renderCan be fixed by memoisation but then will have the same problem as above. Forget to update ref when it changesOP's code struggles from this. The output will look like this: ref1 <div>
ref2 <div>
switching
Forget to reset old refBasically all the solutions that use ref1 <div>
ref2 <div>
switching
ref3 <div>
|
@Torvin Thank you for pointing this out! While an interesting use case, it wasn't something that That being said, I recognize that some developers might find it, for whatever reason, useful to do that. In such cases, I have considered adding support for this to @Torvin One more thought: using Edit: If anyone is interested in having support for this use case in |
@jurca thanks! My first impulse was to fix
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 |
Very well, should you change your mind in the future, and need a helping hand, just let me know 🙂 . |
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:
After React 16.3 this is more complicated as the ref prop can be a function or an object:
First, is this the right approach?
And if so, Typescript types say:
Which prevents us from assigning to
current
. Shouldn't it not be readonly?The text was updated successfully, but these errors were encountered: