[react]: Infer state type for useReducer #64412

mfp22 · 2023-02-18T11:35:43Z

[ x] Use a meaningful title for the pull request. Include the name of the package modified.
[ x] Test the change in your own code. (Compile and run.)
[ x] Add or edit tests to reflect the change.
[ x] Follow the advice from the readme.
[ x] Avoid common mistakes.
[ x] Run npm test <package to test>.

Select one of these and delete the others:

If changing an existing definition:

[ x] Provide a URL to documentation or source code which provides context for the suggested changes: <>

typescript-bot · 2023-02-18T11:36:22Z

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

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

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

Status

✅ No merge conflicts
❌ Continuous integration tests have failed
🕐 Most recent commit is approved by a DT maintainer

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

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 64412,
  "author": "mfp22",
  "headCommitOid": "abf1a80ae12b0dba53c3216926288d673c685313",
  "mergeBaseOid": "be21e88d0d7bd8d3714ae3af1baf9f9f925e456e",
  "lastPushDate": "2023-04-23T03:09:48.000Z",
  "lastActivityDate": "2023-04-23T03:47:18.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react",
      "kind": "edit",
      "files": [
        {
          "path": "types/react/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/test/hooks.tsx",
          "kind": "test"
        }
      ],
      "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": [
    {
      "type": "stale",
      "reviewer": "eps1lon",
      "date": "2023-02-22T21:23:18.000Z",
      "abbrOid": "5eac079"
    }
  ],
  "mainBotCommentID": 1435649006,
  "ciResult": "fail",
  "ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/abf1a80ae12b0dba53c3216926288d673c685313/checks?check_suite_id=12418770497"
}

typescript-bot · 2023-02-18T11:36:25Z

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

eps1lon · 2023-02-18T12:31:54Z

types/react/index.d.ts

+    // NOTE 2: The past concern around autocompletion with the returned state object
+    // seems to no longer be an issue. See https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/63607


We can remove NOTE and NOTE 2 now.

I can't even get autocomplete to work with the previous typings (down to TS 3.3): Playground with previous types

Proposed typings don't have autocomplete either: Playground with proposed types

Strange. It works in the StackBlitz I made. If you go to line 40 and erase the count property and start typing again it autocompletes it. https://stackblitz.com/edit/react-ts-gy5gv6?file=App.tsx

Anyway, pipeline failed so I'm exploring that. And I'll erase those notes

My overload is being used but the other specific overloads are causing my overload to not be able to infer the type. I will explore this more.

@eps1lon I came up with something that might work. 2 overloads instead of 6. I just updated this PR.

Tests pass locally except for the test I added in this PR. I don't know why, because in VSCode running 5.0.0-dev20221218 It's correctly inferring how I want so the test should pass, and in the TS environment like what you shared, it's inferring correctly with v4.9.5. If it's failing for me locally, I bet it will fail in the pipeline. If it does fail, should I just remove my added test? It will pass existing tests and apparently sometimes be better than what's there now.

In VSCode it's autocompleting, and I just confirmed it is in the TS environment too.

Less overloads is always a win. Especially if we can have 3 or less overloads since that has poor debuggability: microsoft/TypeScript#52745

Yes it failed in the pipeline for some reason. I'll try to figure it out a bit longer, but I could just remove my test and make this a "simplify useReducer overloads" PR (maybe it fixes autocomplete too) and in every environment except whatever this specific one is, it will infer the reducer state like I wanted.

I got it passing everything locally! $ExpectType is very picky and not helpful at telling you how you're using it wrong. But now I know.

typescript-bot · 2023-02-18T12:43:35Z

@mfp22 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2023-02-18T15:52:36Z

@mfp22 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

DangerBotOSS · 2023-02-18T17:08:01Z

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:

unstable_act

Generated by 🚫 dangerJS against cebd684

typescript-bot · 2023-02-18T17:08:40Z

@eps1lon Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

eps1lon

Super excited to see this land while also reducing the number of overloads.

I need to double check, but I think this might cause some breakages if people specify the type parameters since some signatures are now gone. But I have to double check. Want to test this on some larger repos anyway before landing.

Leaving the requested changes mostly, so this doesn't get merged before we do some integration testing.

types/react/index.d.ts

typescript-bot · 2023-02-18T18:01:42Z

@mfp22 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

typescript-bot · 2023-02-19T02:06:11Z

@eps1lon Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

eps1lon

I love this change and it integrates pretty well (basically just removing explicit type parameters which is nice). However, the changes to the type parameter interface will require work.

That's why I think we should schedule this for the next React major versions so that we don't break any code relying on the current type parameters. I added it to #64451 so that we don't forget.

One thing I noticed when integrating is the following issue:

const reducer = (state: boolean, next?: unknown) => {
  if (typeof next === 'boolean') {
    return next
  }

  return !state
}

const useToggle = (initialState: boolean) => useReducer(reducer, initialState)

const [isTooltipVisible, toggleTooltipVisibility] = useToggle(false)
toggleTooltipVisibility(false)
//                      ^^^^^ Expected 0 arguments, but got 1

Is this something we can fix?

typescript-bot · 2023-02-22T21:24:03Z

@mfp22 One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

mfp22 · 2023-02-23T06:28:45Z

That's why I think we should schedule this for the next React major versions

Oh that could be years away... too bad. Sure we can't just YOLO this into React 18? I'll volunteer to fix everyone's problems :)

const reducer = (state: boolean, next?: unknown) => {
  if (typeof next === 'boolean') {
    return next
  }

  return !state
}

const useToggle = (initialState: boolean) => useReducer(reducer, initialState)

const [isTooltipVisible, toggleTooltipVisibility] = useToggle(false)
toggleTooltipVisibility(false)

The only way I've found to solve this case is to spread the action arguments onto the dispatch function directly. That preserves optional parameters, but would allow potentially multiple arguments to be passed into Dispatch.

So, I changed useReducer to limit the number of reducer parameters. This matches the Flow types better as well as the prior behavior here.

I added tests to reject reducers with extra parameters, and a test to make sure optional action parameters are preserved.

If someday they change useReducer to take in a 3rd argument for some reason (in StateAdapt I pass in initialState to the reducer in the 3rd argument, which is really useful, so it could happen) then we can change the name AnyActionArg back to ReducerRest or something and allow it to be length 2 or whatever as well. There are ways to handle it. We can always check against ReducerRest['length'] to narrow down to specific types.

typescript-bot · 2023-02-23T07:07:52Z

@eps1lon Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

typescript-bot · 2023-03-06T06:45:31Z

Re-ping @johnnyreilly, @bbenezech, @pzavolinsky, @ericanderson, @DovydasNavickas, @theruther4d, @guilhermehubner, @ferdaber, @jrakotoharisoa, @pascaloliv, @Hotell, @franklixuefei, @Jessidhia, @saranshkataria, @lukyth, @eps1lon, @zieka, @dancerphil, @dimitropoulos, @disjukr, @vhfmag, @hellatan, @priyanshurav, @Semigradsky:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

typescript-bot · 2023-03-13T06:54:09Z

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @mfp22.

(Ping @johnnyreilly, @bbenezech, @pzavolinsky, @ericanderson, @DovydasNavickas, @theruther4d, @guilhermehubner, @ferdaber, @jrakotoharisoa, @pascaloliv, @Hotell, @franklixuefei, @Jessidhia, @saranshkataria, @lukyth, @eps1lon, @zieka, @dancerphil, @dimitropoulos, @disjukr, @vhfmag, @hellatan, @priyanshurav, @Semigradsky.)

sandersn · 2023-03-20T23:16:59Z

When is react 19 going to release? I'm not happy about having this PR open for a long time, but if it's reasonably soon, then it's not too bad.

eps1lon · 2023-03-27T14:28:20Z

I'm not happy about having this PR open for a long time, but if it's reasonably soon, then it's not too bad.

Should we convert to a draft in the meantime instead? Or we can also close in the meantime. Though it's not clear to me what's so special of having one more open.

weswigham · 2023-03-27T17:27:51Z

Draft should be good - that aughta remove it from the maintainer queue.

typescript-bot · 2023-03-29T01:03:04Z

@mfp22 Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

typescript-bot · 2023-04-21T01:10:14Z

@mfp22 I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Apr 26th (in a week) if the issues aren't addressed.

typescript-bot · 2023-04-23T02:35:42Z

@mfp22 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2023-04-23T03:31:53Z

@mfp22 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

mfp22 · 2023-04-23T03:47:18Z

@eps1lon I don't understand how this build could have failed with just the changes in this PR. It looks like it doesn't have anything to do with useReducer. https://github.com/DefinitelyTyped/DefinitelyTyped/actions/runs/4776139211/jobs/8490968425?pr=64412

I also noticed a ts5 folder. Should I add my changes to that folder too?

typescript-bot · 2023-05-16T06:47:16Z

@mfp22 I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on May 23rd (in a week) if the issues aren't addressed.

typescript-bot · 2023-05-24T06:48:48Z

@mfp22 To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!

mfp22 · 2024-02-11T07:39:35Z

@eps1lon Does this mean this PR can be resurrected soon? https://www.reddit.com/r/reactjs/comments/1ajur2f/is_there_any_news_on_react_19/

eps1lon · 2024-02-12T12:32:08Z

It will be part of the React 19 release. I already have a branch that includes the commits from this PR.

mfp22 added 2 commits February 18, 2023 04:20

[react]: Infer state type for useReducer

e98ee70

[react]: Test useReducer state type inference

ef466a8

mfp22 requested review from Jessidhia, eps1lon and johnnyreilly as code owners February 18, 2023 11:35

typescript-bot added the Critical package label Feb 18, 2023

[react]: Fix useReducer parameter name

c060b8f

eps1lon reviewed Feb 18, 2023

View reviewed changes

typescript-bot added the The CI failed When GH Actions fails label Feb 18, 2023

typescript-bot added The CI failed When GH Actions fails and removed The CI failed When GH Actions fails labels Feb 18, 2023

typescript-bot removed the The CI failed When GH Actions fails label Feb 18, 2023

eps1lon requested changes Feb 18, 2023

View reviewed changes

types/react/index.d.ts Show resolved Hide resolved

typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Feb 18, 2023

[react]: Rewrite useReducer overloads

5eac079

mfp22 force-pushed the patch-1 branch from 0de4eea to 5eac079 Compare February 19, 2023 01:05

typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Feb 19, 2023

eps1lon requested changes Feb 22, 2023

View reviewed changes

typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Feb 22, 2023

[react]: Fix optional reducer action & limit args

cebd684

typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Mar 6, 2023

typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. and removed Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. labels Mar 29, 2023

typescript-bot added the Abandoned This PR had no activity for a long time, and is considered abandoned label Apr 21, 2023

Merge branch 'master' into patch-1

e8eae0f

typescript-bot added The CI failed When GH Actions fails and removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. Abandoned This PR had no activity for a long time, and is considered abandoned labels Apr 23, 2023

Disable eslint warning

abf1a80

typescript-bot added The CI failed When GH Actions fails and removed The CI failed When GH Actions fails labels Apr 23, 2023

typescript-bot added the Abandoned This PR had no activity for a long time, and is considered abandoned label May 16, 2023

typescript-bot closed this May 24, 2023

This was referenced Mar 26, 2024

Write changelog for React 19 types eps1lon/DefinitelyTyped#32

Closed

[react] Types for React 19 #69022

Merged

		// NOTE 2: The past concern around autocompletion with the returned state object
		// seems to no longer be an issue. See https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/63607

[react]: Infer state type for useReducer #64412

[react]: Infer state type for useReducer #64412

Uh oh!

Conversation

mfp22 commented Feb 18, 2023

Uh oh!

typescript-bot commented Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1 package in this PR

Code Reviews

Status

Inactive

Uh oh!

typescript-bot commented Feb 18, 2023

Uh oh!

eps1lon Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

mfp22 Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

mfp22 Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

mfp22 Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

eps1lon Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

mfp22 Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

mfp22 Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

typescript-bot commented Feb 18, 2023

Uh oh!

typescript-bot commented Feb 18, 2023

Uh oh!

DangerBotOSS commented Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

react (unpkg)

Uh oh!

typescript-bot commented Feb 18, 2023

Uh oh!

eps1lon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

typescript-bot commented Feb 18, 2023

Uh oh!

typescript-bot commented Feb 19, 2023

Uh oh!

eps1lon left a comment

Choose a reason for hiding this comment

Uh oh!

typescript-bot commented Feb 22, 2023

Uh oh!

mfp22 commented Feb 23, 2023

Uh oh!

typescript-bot commented Feb 23, 2023

Uh oh!

typescript-bot commented Mar 6, 2023

Uh oh!

typescript-bot commented Mar 13, 2023

Uh oh!

sandersn commented Mar 20, 2023

Uh oh!

eps1lon commented Mar 27, 2023

Uh oh!

weswigham commented Mar 27, 2023

Uh oh!

typescript-bot commented Mar 29, 2023

Uh oh!

typescript-bot commented Apr 21, 2023

Uh oh!

typescript-bot commented Apr 23, 2023

Uh oh!

typescript-bot commented Apr 23, 2023

typescript-bot commented Feb 18, 2023 •

edited

Loading

DangerBotOSS commented Feb 18, 2023 •

edited

Loading

react (`unpkg`)