Skip to content
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.

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

[react] Add typing for useTransition config in react/next #58421

Conversation

droidenator
Copy link

@droidenator droidenator commented Jan 24, 2022

Please fill in this template.

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: React useTransition docs
  • 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.

@typescript-bot typescript-bot added this to Needs Author Action in New Pull Request Status Board Jan 24, 2022
@droidenator droidenator marked this pull request as ready for review January 24, 2022 23:33
@typescript-bot
Copy link
Contributor

typescript-bot commented Jan 24, 2022

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

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 passed
  • 🕐 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.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 58421,
  "author": "droidenator",
  "headCommitOid": "79206fb721bd03f987e0d7c36a8cb2856ed18432",
  "mergeBaseOid": "3c54df48ebc539e1bf97e328d83a96601b833b97",
  "lastPushDate": "2022-01-24T23:29:12.000Z",
  "lastActivityDate": "2022-01-25T15:14:17.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react",
      "kind": "edit",
      "files": [
        {
          "path": "types/react/next.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/test/next.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"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "changereq",
      "reviewer": "eps1lon",
      "date": "2022-01-25T09:06:07.000Z"
    }
  ],
  "mainBotCommentID": 1020658633,
  "ciResult": "pass"
}

@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Jan 24, 2022
@typescript-bot
Copy link
Contributor

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Jan 24, 2022
Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

react/next points to the current next release. In this release timeoutMS is no longer available.

That's why we removed the config in #52464

@typescript-bot typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Jan 25, 2022
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Needs Author Action in New Pull Request Status Board Jan 25, 2022
@typescript-bot
Copy link
Contributor

@droidenator 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!

@droidenator
Copy link
Author

droidenator commented Jan 25, 2022

@eps1lon I'm not sure that that's correct. The PR you linked removed the following config:

    export interface SuspenseConfig {
        busyDelayMs?: number;
        busyMinDurationMs?: number;
    }

However all the current React 18 documentation shows a config option with timeoutMS. Here's a link to the config in documentation regarding useTransition: https://reactjs.org/docs/concurrent-mode-reference.html#usetransition-config

Is the React documentation out of date regarding this config? It wouldn't surprise me but as far as I can tell, timeoutMS should be supported.

EDIT: Did some digging and you're absolutely right, @eps1lon. Here's the issue where they dropped support for this: facebook/react#19703

That's really frustrating that the React documentation is out of date. Thanks for the clarification; I'll close my PR.

@typescript-bot typescript-bot removed this from Needs Author Action in New Pull Request Status Board Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical package Revision needed This PR needs code changes before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants