react: Make value returned by useState Readonly

[SpLouk]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change.
  • Not sure what to add as a test for this; this change is intended to make mutation of state impossible
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm test <package to test>.

Select one of these and delete the others:

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: <>
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

State is immutable, so the return value of useState should be wrapped in the DeepReadonly utility type which can prevent accidental mutation.

https://github.com/piotrwitek/utility-types/blob/master/src/mapped-types.ts#L396

This will prevent accidental mutation of state. E.g.

const [arr, setArr] = useState(['a', 'b']);
arr.push('c')

[typescript-bot]

@SpLouk Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

• react — on npm, on unpkg

Code Reviews

This PR can be merged once it's reviewed.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• 🕐 Most recent commit is approved by type definition owners or DT maintainers

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 64099,
  "author": "SpLouk",
  "headCommitOid": "4665ec5e7744a6fc5d57bc0839b37a29811813cd",
  "mergeBaseOid": "7a09beca6be782271dd738e86e33c0a460539916",
  "lastPushDate": "2023-01-26T19:40:24.000Z",
  "lastActivityDate": "2023-01-30T21:08:25.000Z",
  "maintainerBlessed": "Waiting for Code Reviews",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react",
      "kind": "edit",
      "files": [
        {
          "path": "types/react/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/react/v17/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "johnnyreilly",
        "bbenezech",
        "pzavolinsky",
        "ericanderson",
        "DovydasNavickas",
        "theruther4d",
        "guilhermehubner",
        "ferdaber",
        "jrakotoharisoa",
        "pascaloliv",
        "hotell",
        "franklixuefei",
        "Jessidhia",
        "saranshkataria",
        "lukyth",
        "eps1lon",
        "zieka",
        "dancerphil",
        "dimitropoulos",
        "disjukr",
        "vhfmag",
        "hellatan",
        "priyanshurav",
        "Semigradsky"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [],
  "mainBotCommentID": 1405524743,
  "ciResult": "pass"
}

[typescript-bot]

🔔 @johnnyreilly @bbenezech @pzavolinsky @ericanderson @DovydasNavickas @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @Jessidhia @saranshkataria @lukyth @eps1lon @zieka @dancerphil @dimitropoulos @disjukr @vhfmag @hellatan @priyanshurav @Semigradsky — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[Semigradsky]

I like this suggestion, but it will break a lot of existing code.

[eps1lon]

I like this suggestion, but it will break a lot of existing code.

Yeah I can see that. Just had a case today were a component library was expecting a mutable array (which is the default in TypeScript). In those cases, TypeScript would now throw.

Could you test this on some small/medium+ sized repositories to check how many new issues we create with this change?

[eps1lon]

Would a Readonly here suffice for the majority of use cases? I'm mainly concerned about perf here.

[SpLouk]

I think DeepReadonly would be preferred to prevent mutating nested members of an object.

[psalv]

I like this suggestion, but it will break a lot of existing code.

It's arguable this code has a high chance of being broken already, they just don't know about it

[DangerBotOSS]

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.

react (unpkg)

was missing the following properties:

1. unstable_act

Generated by 🚫 dangerJS against 4665ec5

[eps1lon]

It's arguable this code has a high chance of being broken already, they just don't know about it

Did you run this on some example codebases? What were patterns that it did reject, that we want to reject and what were patterns we don't necessarily want to reject (e.g. due to the ecosystem not having props as readonly in most cases)?

[SpLouk]

Yeah, so this will definitely be a breaking change. I didn't anticipate this but

const [str,...] = useState(['a', 'b'])

return <Child str={ str } /> // pass the state down to a child

Will cause a type error because Child is expecting a string [] not a readonly string[].

I ran our type checker in Faire's codebase locally and got about 20 type errors of this form in a ~100,000 line React/TS codebase. We don't use hooks everywhere yet so that might be a low number for a codebase of that size.

Now, in a perfect world, all arrays and objects passed as props should be readonly too, but that isn't the case in our codebase and I'm sure many others.

[eps1lon]

Now, in a perfect world, all arrays and objects passed as props should be readonly too, but that isn't the case in our codebase and I'm sure many others.

Same for our design system at Klarna.

Did you catch any actual errors similar to the one explained in the PR description?

[SpLouk]

Did you catch any actual errors similar to the one explained in the PR description?

No, no instances of an object actually being mutated.

[SpLouk]

Going to go with an internal solution here.