Skip to content
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

Add prop type for Refs #240

Open
mindtraveller opened this issue Nov 27, 2018 · 8 comments
Open

Add prop type for Refs #240

mindtraveller opened this issue Nov 27, 2018 · 8 comments
Labels
new validator request Requests for a new kind of validator.

Comments

@mindtraveller
Copy link

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 mindtraveller changed the title Add ref prop types Add ref prop type Nov 27, 2018
@mindtraveller mindtraveller changed the title Add ref prop type Add prop type for Refs Nov 27, 2018
@joshalling
Copy link
Contributor

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

@dzucconi
Copy link

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.)

@ahuth
Copy link

ahuth commented Jul 15, 2019

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 🤔

@ljharb
Copy link
Collaborator

ljharb commented Jul 16, 2019

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

@mindtraveller
Copy link
Author

@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.

@oliviertassinari
Copy link

oliviertassinari commented Aug 26, 2019

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.

@ljharb
Copy link
Collaborator

ljharb commented Aug 27, 2019

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

@Izhaki
Copy link

Izhaki commented Sep 24, 2019

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new validator request Requests for a new kind of validator.
Projects
None yet
Development

No branches or pull requests

7 participants