prop-types icon indicating copy to clipboard operation
prop-types copied to clipboard

Add prop type for Refs

Open mindtraveller opened this issue 6 years ago • 8 comments

How about adding prop types for Refs as a built in feature? Right now every *ref property looks like this:

buttonRef: PropTypes.oneOfType([
    PropTypes.func, // for legacy refs
    PropTypes.shape({ current: PropTypes.instanceOf(Element) })
])

Why not just provide it out of the box like:

buttonRef: PropTypes.ref

mindtraveller avatar Nov 27 '18 07:11 mindtraveller

I would also like to see this feature added and would be willing to submit a PR for it if approved.

joshalling avatar Dec 04 '18 18:12 joshalling

PropTypes.shape({ current: PropTypes.instanceOf(Element) })

Breaks under server-side rendering because of Element not existing in that context.

ReferenceError: Element is not defined

(I agree there should be a prop-type for refs.)

dzucconi avatar Jan 17 '19 15:01 dzucconi

One thing to note is that refs don't necessarily ref to a DOM element. This became especially clear while learning hooks. It's convenient to use refs to refer to callback functions, timeout ids, the current time, etc, while using useEffect, useMemo, useCallback, etc.

Generally I think of refs as being immutable references to mutable values.

Of course, maybe those kinds of refs aren't as likely to get passed around as props 🤔

ahuth avatar Jul 15 '19 20:07 ahuth

That seems less like a ref then, and more like state?

ljharb avatar Jul 16 '19 05:07 ljharb

@ahuth @ljharb The original idea was to create a prop-type to combine legacy callback refs and new object refs. Of course a separate prop-type for mutable object might also be added e.g. PropTypes.shape({ current: PropTypes.any }), because that structure has been widely used since useRef appeared.

mindtraveller avatar Jul 16 '19 17:07 mindtraveller

I have one doubt with: PropTypes.instanceOf(Element). Does it work cross-window? cc @ryancogswell http://tobyho.com/2011/01/28/checking-types-in-javascript/.

react-popout can be a way to test that out.

oliviertassinari avatar Aug 26 '19 19:08 oliviertassinari

@oliviertassinari no, instanceof itself is broken and unreliable in this way, so PropTypes.instanceOf would have to be as well.

ljharb avatar Aug 27 '19 07:08 ljharb

As @ahuth mentioned, we use refs for all sort of non-element things.

From the React Docs:

However, useRef() is useful for more than the ref attribute. It’s handy for keeping any mutable value around similar to how you’d use instance fields in classes.

In more than one occasion, whilst migrating class to functional component, I've found some reliance on a mutable value in the class component - and only refs can do this in functional components.

So I think instead of:

trimWidth: PropTypes.ref

it should be:

trimWidth: PropTypes.ref(PropTypes.number)

Izhaki avatar Sep 24 '19 20:09 Izhaki