Skip to content

Update useRef types to reflect normal usage #64855

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.

Sign up for GitHub

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

Closed
ericanderson wants to merge 1 commit into from
Closed

Update useRef types to reflect normal usage #64855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions types/react/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,15 @@ declare namespace React {
| ((props: P) => ReactElement<any, any> | null)
| (new (props: P) => Component<any, any>);

interface RefObject<T> {
readonly current: T | null;
}
/**
* @deprecated All refs are mutable. This type has always been wrong. Expect it to change to
Copy link
Collaborator

@eps1lon eps1lon Mar 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording doesn't really add value. There was a rationale why we did this. It's like saying ReadonlyArray<SomeArray> "was always wrong". Just because something works in JavaScript, doesn't mean it's correct.

I don't think we should deprecate this. Doesn't it make more sense to drop MutableRefObject in the future instead and let RefObject be the mutable one?

Copy link
Collaborator

@eps1lon eps1lon Mar 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, MutableRefs meant user-managed while readonly-refs meant react-managed. See https://github.com/DefinitelyTyped/DefinitelyTyped/pull/64772/files#r1148507326

Copy link
Contributor Author

@ericanderson ericanderson Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the history prior to making the recommendation. However there isn't really a way to know if the intended use is user managed vs react-managed, which is unfortunate.

Changing RefObject to be representable with MutableRefObject allows for deprecation. The reverse isn't possible.

I'm saying its "wrong" because really you can change it as you see fit and applying magic typing to try to signify if something is readonly or not is just confusing. I'm saying its "wrong" because we created a weird mechanism in the types that are inconsistent with react itself and have been since day one.

Also, saying the ref is 'readonly' when you pass it to something that clearly modifies the value, also breaks most of the ways that readonly is used, which is that it lets you know that the thing you are giving it to doesn't mutate it. To your example of a ReadonlyArray, if an API had a doFoo() that returned {foo: ReadonlyArray<T>} then I would assume that the inner array would never change out from under me. But in the analogy, you're allowing it to change by react.

* `type RefObject<T> = { current: T };` in React 19.
**/
type RefObject<T> = Readonly<MutableRefObject<T|null>>;

// Bivariance hack for consistent unsoundness with RefObject
type RefCallback<T> = { bivarianceHack(instance: T | null): void }["bivarianceHack"];
type Ref<T> = RefCallback<T> | RefObject<T> | null;
type Ref<T> = RefCallback<T> | RefObject<T> | MutableRefObject<T | null | undefined> | null;
type LegacyRef<T> = string | Ref<T>;
/**
* Gets the instance type for a React element. The instance will be different for various component types:
Expand Down Expand Up @@ -769,7 +772,7 @@ declare namespace React {
[propertyName: string]: any;
}

function createRef<T>(): RefObject<T>;
function createRef<T>(): MutableRefObject<T|null>;

// will show `ForwardRef(${Component.displayName || Component.name})` in devtools by default,
// but can be given its own specific name
Expand Down Expand Up @@ -1014,13 +1017,11 @@ declare namespace React {
* Note that `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.
*
* Usage note: if you need the result of useRef to be directly mutable, include `| null` in the type
* of the generic argument.
*
* @version 16.8.0
* @see https://reactjs.org/docs/hooks-reference.html#useref
*/
function useRef<T>(initialValue: T|null): RefObject<T>;
function useRef<T>(initialValue: T|null): MutableRefObject<T | null>;
// convenience overload for potentially undefined initialValue / call with 0 arguments
// has a default to stop it from defaulting to {} instead
/**
Expand All @@ -1029,6 +1030,12 @@ declare namespace React {
*
* Note that `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.
*
* Note about this overload: this definition is technically incorrect as the object is of type `{ current: undefined }`
* at creation, but upon unmount of a component react will assign the `current` to `null` (and always has).
* However updating the type would be a breaking change. Expect this return type to be
* `MutableRefObject<T | undefined | null>` in React 19.
*
*
* @version 16.8.0
* @see https://reactjs.org/docs/hooks-reference.html#useref
Expand Down
6 changes: 6 additions & 0 deletions types/react/test/hooks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ function useEveryHook(ref: React.Ref<{ id: number }>|undefined): () => boolean {
const c: React.MutableRefObject<number | null> = React.useRef(null);
const d: React.RefObject<number> = React.useRef(null);

// checks for https://twitter.com/mattpocockuk/status/1636098722982404096
const pocockukA = React.useRef<string>(null);
pocockukA.current = "Hello";
const pocockukB = React.useRef<HTMLDivElement>();
const el = <div ref={pocockukB}/>

const id = React.useMemo(() => Math.random(), []);
React.useImperativeHandle(ref, () => ({ id }), [id]);
// was named like this in the first alpha, renamed before release
Expand Down