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 v18 fork for TS 5.0 and below #65126

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Apr 14, 2023

If microsoft/TypeScript#51328 ships we want to take advantage of it immediately. We could just add JSX.ElementType now but this would leave us in an awkward spot where you couldn't use React.FC to return all the valid things from render. So I'd rather make a clear cut where we consider TS 5.1. + React 18 types fixed. Supporting all other combinations would add a lot of work and this way we have another carrot for people to update TypeScript and React (especially because the biggest motivator is only in React 18 anyway: Server Components).

We can leverage the fork for other things as well (e.g. adding data-${string} props to HTML attributes).

@DangerBotOSS
Copy link

DangerBotOSS commented Apr 14, 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.

react (unpkg)

was missing the following properties:

  1. unstable_act

Generated by 🚫 dangerJS against ec5d954

@eps1lon eps1lon force-pushed the feat/react/ts-50-fork branch from 931de2e to 75c7c38 Compare April 14, 2023 14:35
@@ -3,6 +3,7 @@
"dependencies": {
"csstype": "^3.0.2"
},
"types": "index",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

required by dtslint:

$ npm run test react

> definitely-typed@0.0.3 test
> node --require source-map-support/register node_modules/@definitelytyped/dtslint/ types react

dtslint@0.0.159
AssertionError [ERR_ASSERTION]: "types" in '/home/eps1lon/Development/forks/DefinitelyTyped/types/react/package.json' should be "index".
    at checkPackageJson (/home/eps1lon/Development/forks/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/checks.ts:27:12)
    at runTests (/home/eps1lon/Development/forks/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:167:5)
    at main (/home/eps1lon/Development/forks/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:83:5)

@eps1lon eps1lon marked this pull request as ready for review April 15, 2023 11:17
@typescript-bot
Copy link
Contributor

typescript-bot commented Apr 15, 2023

@eps1lon Thank you for submitting this PR!

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
  • 🕐 A DT maintainer needs to approve changes which affect module config files

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": 65126,
  "author": "eps1lon",
  "headCommitOid": "ec5d954dcb5a940d57e1e3e148808e6eab6e2e50",
  "mergeBaseOid": "2d2e244fd68b8b1284ebdbf2846fce269a0c8c65",
  "lastPushDate": "2023-04-17T16:29:25.000Z",
  "lastActivityDate": "2023-04-17T16:56:39.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": true,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react",
      "kind": "edit",
      "files": [
        {
          "path": "types/react/package.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `exports`)"
        },
        {
          "path": "types/react/ts5.0/.eslintrc.json",
          "kind": "package-meta",
          "suspect": "edited"
        },
        {
          "path": "types/react/ts5.0/OTHER_FILES.txt",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-other_filestxt)"
        },
        {
          "path": "types/react/ts5.0/experimental.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/global.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/jsx-dev-runtime.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/jsx-runtime.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/next.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react/ts5.0/test/cssProperties.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/elementAttributes.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/experimental.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/hooks.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/index.ts",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/managedAttributes.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/next.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/test/tsx.tsx",
          "kind": "test"
        },
        {
          "path": "types/react/ts5.0/tsconfig.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-tsconfigjson) (check: `compilerOptions.exactOptionalPropertyTypes`)"
        },
        {
          "path": "types/react/ts5.0/tslint.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-linter-tslintjson) (check: `rules`)"
        }
      ],
      "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": 1509733487,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Critical package Author is Owner The author of this PR is a listed owner of the package. Check Config Changes a module config files Huge Change labels Apr 15, 2023
@typescript-bot
Copy link
Contributor

@eps1lon eps1lon force-pushed the feat/react/ts-50-fork branch from afb835d to afa81ee Compare April 17, 2023 16:24
@eps1lon eps1lon force-pushed the feat/react/ts-50-fork branch from afa81ee to ec5d954 Compare April 17, 2023 16:29
@eps1lon
Copy link
Collaborator Author

eps1lon commented Apr 17, 2023

Landing now to test infra. As long as we don't rely on newer TS features we can always bump the max version of the fork later.

@eps1lon eps1lon merged commit 77ecf34 into DefinitelyTyped:master Apr 17, 2023
@eps1lon eps1lon deleted the feat/react/ts-50-fork branch April 17, 2023 16:58
@johnnyreilly
Copy link
Member

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author is Owner The author of this PR is a listed owner of the package. Check Config Changes a module config files Critical package Huge Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants