Flow: React.useRef should not have nullable current

[TrySound]

Since useRef has default value null can be inferred from it. So it's not necessary to add another default which requires to add unnecessary checks in some cases.

declare export function useRef<T>(initialValue: T): {current: T};

const count = useRef<number>(0);

count.current // number

const reference = useRef<?Element>(null)

reference.current // ?Element

/cc @bvaughn @jbrown215

[TrySound]

I would make a PR but after running make I get this error

ocamlbuild -use-ocamlfind -no-links -I scripts scripts/ppx_gen_flowlibs.native
Failure: ocamlfind not found on path, but -no-ocamlfind not used.
Compilation unsuccessful after building 0 targets (0 cached) in 00:00:00.
make: *** [_build/scripts/ppx_gen_flowlibs.native] Error 2

[bvaughn]

Interesting! This implies that when a default value is provided– the ref will never be set to null, which might not be true in all cases. But I guess it would be nice for cases where it is true, and people could always add an explicit <Element | null> in that case... (I don't think <?Element> would be as accurate since that would also permit undefined)

What do you think, @jbrown215?

[jbrown215]

I have to think about this more, but this would cause incompatibility issues with places that currently expect {current: null | T}, since current is a read/write prop. try-flow.

In spirit, I'm in favor of more precise types when defaults are provided, but I'll have to evaluate the downstream effects a bit more.

[bdrobinson]

people could always add an explicit <Element | null> in that case...

@bvaughn that seems like a good approach – and just to make the case further, the flow typings for React.createContext already work in the way you've suggested right? ie

const MyContext = createContext<Element | null>(element)

if they want the value to be nullable, so it feels natural to also do the same for useRef?

[TrySound]

Yeah, currently I write a lot of noisy conditions for refs which are initialised and updated from state to get actual values in async operation. So those values always exist.

I think it's ok to make this argument mandatory before hooks became stable at least with type systems.

[jbrown215]

Can you show an example of how you use these refs? I agree it would be better for this function to always require an argument, but I don't want to make it difficult to use refs with existing code that expects {current: null | T}.

[TrySound]

Nothing difficult I think. Passing null in existing code is a simple change. I personally always pass null to useRef

const [value, setValue] = React.useState('')
const valueRef = React.useRef<string>(value)
valueRef.current = value;

React.useEffect(() => {
  mutate(result => {
    if (valueRef.current.length !== 0) {
      // do something
    }
  })
}, [value])

const elementRef = React.useRef<Element | null>(null)

<div ref={elementRef} /

[jbrown215]

I personally always pass null to useRef

Is this what's expected or is this the exception? That's what's making me nervous to change things here. I'm more convinced, now though.

[TrySound]

Hooks are unstable. Users expect changes I guess. Not sure about facebook teams though. But yeah, this makes things much simpler so good for users.

[TrySound]

Hey @jbrown215 @bvaughn Would be good to consider this before hooks release. Currently refs are too verbose. This is more critical since we cannot rely on dependency array in useEffect.

[jbrown215]

I’m out of the office on PTO until Monday. I will look into this more when
I get back.

If you can still pass the refs to react components then I’m ok with it, but
I don’t think that type checks.

Any functions that have write access to these refs will also break with
this change. I’m ok with that because you can fix that with a type

annotation at the callsite.

Sent from Gmail Mobile

[jbrown215]

My primary concern is that you won’t have a default value if you want to
pass this as a ref to a component. If that’s the case, then you’ll always
have to explicitly annotate the call with a type argument of null | T when
you intend to pass the result to a component.

Maybe instead we should overload the function. If null is passed, set the
field to null | T. If a non-null value is passed, just use T.

This is sound, and will probably satisfy your use case.

Sent from Gmail Mobile

[bvaughn]

My primary concern is that you won’t have a default value if you want to
pass this as a ref to a component.

I think as refs start to be used more commonly for things other than React elements, it makes more sense to support just type T. Component refs are nullable, but it's easy to think of use cases for non-nullable refs of other types.

[TrySound]

This is also useful for cases where I cannot pass nullable value and I need to do some hacks to workaround it.

const now = React.useRef(null);
if (now.current == null) {
  now.current = new Date();
}

return <DatePicker value={now.current} /> // error, value should not be nullable