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

Update react-native types to comply to RN 0.72 #890

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

gvarandas
Copy link
Contributor

@gvarandas gvarandas commented Aug 1, 2023

Description

  • Update types to complain with the updated react-native types shipped as part of 0.72.
  • Add an npm script to verify types are sound without having to build the package.

Reviewers’ hat-rack 🎩

No actual changes are being shipped as part of it, except for type changes.

  • CI should be ✅

Screenshots or videos (if needed)

N/A

Checklist

@gvarandas
Copy link
Contributor Author

gvarandas commented Aug 1, 2023

@fortmarek @naqvitalha FYI this PR might affect which RN versions the library supports, given that users might run into TS issues in case they haven't upgraded. We should carefully consider this before releasing it into the wild.

@gvarandas gvarandas self-assigned this Aug 1, 2023
fortmarek

This comment was marked as outdated.

version "0.67.6"
resolved "https://registry.yarnpkg.com/@types/react-native/-/react-native-0.67.6.tgz#9a7de5feba6065aec9f44f9a1e8f6e55ee5d015c"
integrity sha512-NM6atxrefIXMLE/PyQ1bIQjQ/lWLdls3uVxItzKvNUUVZlGqgn/uGN4MarM9quSf90uSqJYPIAeAgTtBTUjhgg==
"@types/react-native@0.72.2":
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can remove this dependency completely now that react-native ships with types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to do that, we'd need to declare react-native as a devDependency instead. It's a more future-proof alternative, I believe, as I'm not really sure how long the @types/react-native lib will be actively maintained now that we have official support for TS in the original repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this PR, it should be fine as-is, I believe.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a more future-proof alternative, I believe, as I'm not really sure how long the @types/react-native lib will be actively maintained now that we have official support for TS in the original repo.

IIRC, that dependency will no longer be updated from 0.73 onwards. I agree we can keep as-is for this PR, though.

@fortmarek fortmarek self-requested a review August 2, 2023 09:37
@fortmarek
Copy link
Contributor

FYI this PR might affect which RN versions the library supports, given that users might run into TS issues in case they haven't upgraded.

To avoid breakage, instead of reusing the RN types, we can change the type to correspond to the new one. Or is the type change itself a breaking change?

@gvarandas
Copy link
Contributor Author

@fortmarek it really depends on how far we want to go with backwards compatibility. The updated types are more permissive in this case (they add the 'auto' type for dimension values), so it shouldn't be an issue overall. It is, however, really hard to guarantee that everything will work with super old RN versions.

@fortmarek
Copy link
Contributor

The updated types are more permissive in this case (they add the 'auto' type for dimension values), so it shouldn't be an issue overall. It is, however, really hard to guarantee that everything will work with super old RN versions.

If that's the case, I'd ship this. While we don't have an official support window, I don't think we should try to support really old RN versions.

@gvarandas gvarandas merged commit e42cc62 into main Aug 3, 2023
13 checks passed
@gvarandas gvarandas deleted the chore/update-rn-types branch August 3, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants