KEMBAR78
[react] Make all refs mutable by default by eps1lon · Pull Request #64896 · DefinitelyTyped/DefinitelyTyped · GitHub
Skip to content

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Mar 26, 2023

Ready for review but won't be merged here and rather in a PR for 19 stacking all changes from #64451

Closes #64855
Closes #64772
Fixes first gotcha in https://twitter.com/mattpocockuk/status/1636098722982404096 (second one will be fixed in #64920).

 interface RefObject<T> {
-   readonly current: T | null
+   current: T
 }

RefObject is no longer considered a ref managed by React (i.e. it no longer makes current nullable). Instead, current will only have the type you declare and be mutable.

Historically RefObject was the type returned from createRef so it made sense to always include null since that what React does on initialization. The mistake we made was reusing it for useRef which has a different audience (useRef for function components where it can act as "instance variables", createRef for class components where we only care about React managed refs).

But now that we moved along it makes sense to fix this incorrect abstraction, take on a breaking change and reduce papercuts going forward.

Migration

  1. Update all @types/* packages to their latest versions
  2. Apply npx types-react-codemod refobject-defaults ./src (see types-react-codemod for full usage)

The remaining fixes should only include libraries shipping their own types. Either these libraries are already compatible with React 19 types and you just need to upgrade them or you should raise an issue on their repositories notifying them that their types are not compatible with React 19.

@eps1lon eps1lon force-pushed the feat/react/new-refs branch from dc13350 to 07075da Compare March 26, 2023 09:35
@eps1lon eps1lon force-pushed the feat/react/new-refs branch 2 times, most recently from ffff287 to 95c7b2e Compare March 26, 2023 20:59
@DangerBotOSS
Copy link

DangerBotOSS commented Mar 26, 2023

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

babel-plugin-react-html-attrs (unpkg)

was missing the following properties:

  1. The declaration doesn't match the JavaScript module 'babel-plugin-react-html-attrs'. Reason:
    The JavaScript module can be called or constructed, but the declaration module cannot.

gatsbyjs__reach-router (unpkg)

was missing the following properties:

  1. BaseContext
  2. LocationContext
  3. insertParams
  4. match
  5. navigate
as well as these 34 other properties...

pick, resolve, shallowCompare, startsWith, useBaseContext, useLocationContext, useNavigate, validateRedirect, BaseContext, LocationContext, insertParams, match, navigate, pick, resolve, shallowCompare, startsWith, useBaseContext, useLocationContext, useNavigate, validateRedirect, BaseContext, LocationContext, insertParams, match, navigate, pick, resolve, shallowCompare, startsWith, useBaseContext, useLocationContext, useNavigate, validateRedirect

reach__router (unpkg)

was missing the following properties:

  1. matchPath
  2. matchPath

react-bootstrap-typeahead (unpkg)

was missing the following properties:

  1. useHint
  2. Input
  3. asyncContainer
  4. useAsync
  5. withAsync
as well as these 5 other properties...

menuItemContainer, useItem, withItem, tokenContainer, withToken

react-map-gl (unpkg)

was missing the following properties:

  1. AttributionControl
  2. setRTLTextPlugin
  3. MapContext

react-onclickoutside (unpkg)

was missing the following properties:

  1. IGNORE_CLASS_NAME
  2. IGNORE_CLASS_NAME

react (unpkg)

was missing the following properties:

  1. unstable_act

tuya-panel-kit (unpkg)

was missing the following properties:

  1. Drawer
  2. Wave
  3. Diffusion
  4. OfflineView
  5. FullView

wordpress__components (unpkg)

was missing the following properties:

  1. CustomGradientPicker
  2. DuotonePicker
  3. DuotoneSwatch
  4. GradientPicker
  5. GuidePage
as well as these 17 other properties...

Line, SearchControl, TextHighlight, ToolbarDropdownMenu, ToolbarItem, useBaseControlProps, CustomGradientPicker, DuotonePicker, DuotoneSwatch, GradientPicker, GuidePage, Line, SearchControl, TextHighlight, ToolbarDropdownMenu, ToolbarItem, useBaseControlProps

wordpress__rich-text (unpkg)

was missing the following properties:

  1. store

Generated by 🚫 dangerJS against 95c7b2e

@ericanderson
Copy link
Contributor

I kept MutableRefObject around so that anyone using RefObject in their application code would not have the type change from under them and represented RefObject with MutableRefObject. It seemed like a cleaner deprecation path.

I also think if we are making breaking changes for React 19, we should not allow the zero argument version of useRef() since there are no documented uses in react. The behavior of useRef<T>() implying { current: T | undefined } is really confusing to most people when we inherit null in the other cases. I think either useRef<T>() needs to be { current: T | undefined | null } or needs to not exist. Both cover issue 2 in the referenced tweet.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Mar 28, 2023

I also think if we are making breaking changes for React 19, we should not allow the zero argument version of useRef() since there are no documented uses in react.

We track this separately in #64920. This PR is not stacking all React 19 changes.

The behavior of useRef() implying { current: T | undefined } is really confusing to most people when we inherit null in the other cases.

Which other cases?

I think either useRef() needs to be { current: T | undefined | null } or needs to not exist.

{ current: T | undefined | null } will not be assignable to React managed refs since object properties are covariant in TypeScript. We cannot change that at the types declaration level. We need contravariant object properties for that: microsoft/TypeScript#21759 (comment)

#64920 will propose its non-existent i.e. only useRef<T>(undefined): { current: T < undefined } will exist (i.e. convenience overload for undefined just like null.

@eps1lon eps1lon force-pushed the feat/react/new-refs branch 3 times, most recently from 13f70c4 to c018004 Compare December 5, 2023 19:55
@eps1lon eps1lon force-pushed the feat/react/new-refs branch from c018004 to 610918c Compare December 5, 2023 20:21
@DiegoAndai
Copy link

Hi @eps1lon. What would be the recommendation for the refs that are handled by React, i.e.:

const divRef = React.useRef<HTMLDivElement>(null)

// ...

<div ref={divRef} />

If I understand correctly, divRef will be mutable in React 19. Should we keep it as such even though it's not supposed to be mutated? Is this restriction outside of @types/react's scope?

I don't have an opinion on it, just wondering what is the recommended usage.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Aug 8, 2024

If I understand correctly, divRef will be mutable in React 19. Should we keep it as such even though it's not supposed to be mutated?

There was never such a restriction that you shouldn't mutate it. React will not read this value so you can write whatever you want into that value. You just need to be aware that React may overwrite what you wrote into the ref. But that would equally apply to all instances where you pass a ref to multiple places that will write to it.

@DiegoAndai
Copy link

Thanks for the reply!

There was never such a restriction that you shouldn't mutate it.

This confuses me a bit, wasn't the readonly in

interface RefObject<T> {
    readonly current: T | null;
}

A restriction about not mutating it?

@eps1lon
Copy link
Collaborator Author

eps1lon commented Aug 12, 2024

A restriction about not mutating it?

At the type level, yes. But types are in service to how we want React code to be authored. And there this restriction never existed.

@DiegoAndai
Copy link

Thanks for the clarification

@eps1lon eps1lon deleted the feat/react/new-refs branch May 20, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants