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-native: Ensure elements narrowed by isValidElement are valid props #64840

Merged

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Mar 21, 2023

Works around a TypeScript bug introduced between 5.0 Beta and RC that doesn't get attention. To unblock the ecosystem while keeping the blast radius minimal, I'll change the RN types.

This probably doesn't fix all problems caused by TS 5.0 but it fixes the ones I encountered.

We should default the props of React.ReactElement to unknown anyway which would at least call out explicit breakage when props accept React.ReactElement<any>. But that has considerable larger blast-radius so for now libraries have to fix these issues on the spot.

@typescript-bot typescript-bot added this to Needs Author Action in New Pull Request Status Board Mar 21, 2023
@eps1lon eps1lon marked this pull request as ready for review March 22, 2023 07:06
@eps1lon eps1lon requested a review from alloy as a code owner March 22, 2023 07:06
@typescript-bot
Copy link
Contributor

typescript-bot commented Mar 22, 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
  • 🕐 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": 64840,
  "author": "eps1lon",
  "headCommitOid": "22c80346eb918a2ad9499f94752f4ac13abc35b0",
  "mergeBaseOid": "f296fa25fe4a69f9ca9217a8cd0a7b1474b9b09b",
  "lastPushDate": "2023-03-21T17:36:12.000Z",
  "lastActivityDate": "2023-03-24T13:45:50.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "react-native",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-native/Libraries/Lists/FlatList.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/Libraries/Lists/SectionList.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/Libraries/Lists/VirtualizedList.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/test/index.tsx",
          "kind": "test"
        },
        {
          "path": "types/react-native/v0.63/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.64/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.65/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.66/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.67/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.68/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.69/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.70/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "alloy",
        "huhuanming",
        "iRoachie",
        "timwangdev",
        "kamal",
        "alexdunne",
        "swissmanu",
        "bm-software",
        "mvdam",
        "esemesek",
        "mrnickel",
        "souvik-ghosh",
        "nossbigg",
        "saranshkataria",
        "tykus160",
        "jakebloom",
        "ceyhun",
        "mcmar",
        "theohdv",
        "romain-faust",
        "bebebebebe",
        "Naturalclar",
        "chinesedfan",
        "vtolochk",
        "SychevSP",
        "sasurau4",
        "256hz",
        "doumart",
        "drmas",
        "jeremybarbet",
        "ds300",
        "natsathorn",
        "connectdotz",
        "alexeymolchan",
        "alexbrazier",
        "kuasha420",
        "phvillegas",
        "eps1lon",
        "ZihanChen-MSFT",
        "kelset",
        "MateWW",
        "lunaleaps",
        "saadnajmi"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [],
  "mainBotCommentID": 1479018389,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Critical package Author is Owner The author of this PR is a listed owner of the package. labels Mar 22, 2023
@typescript-bot typescript-bot moved this from Needs Author Action to Needs Maintainer Review in New Pull Request Status Board Mar 22, 2023
const { ListEmptyComponent } = this.props;
const listEmptyComponent: JSX.Element | null | undefined = React.isValidElement(ListEmptyComponent)
? ListEmptyComponent
: ListEmptyComponent && <ListEmptyComponent />;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ListEmptyComponent is not narrowed to ReactElement<unknown> if a union member is ReactElement<any>. But with the change in this PR the full type is ReactElement<unknown> | ... so isValidElement does narrow it again.

@kelset
Copy link
Contributor

kelset commented Mar 22, 2023

cc @lunaleaps

@eps1lon
Copy link
Collaborator Author

eps1lon commented Mar 24, 2023

Merging since this has the smallest blast radius as far as I can tell.

@eps1lon eps1lon merged commit 0b74bb1 into DefinitelyTyped:master Mar 24, 2023
@typescript-bot typescript-bot removed this from Needs Maintainer Review in New Pull Request Status Board Mar 24, 2023
@eps1lon eps1lon deleted the fix/react-native/unknown-element-props branch March 27, 2023 08:58
@eps1lon eps1lon mentioned this pull request Apr 26, 2023
8 tasks
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. Critical package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants