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

Add @types/react as optional peerDependency on packages that use it #43509

Closed
wants to merge 3 commits into from

Conversation

acoates-ms
Copy link
Contributor

Now that RN is providing TS type information, many of those .d.ts files depend on types from react. In modern packagemanagers (Ex: pnpm) @types/react will not be available to RN since it does not declare it as a dependency.

I also noticed that the types for react-native-popup-menu-android appear to be pointing to the wrong location.

Summary:

Add @types/react as a peerDependency on the packages that have .d.ts files that import from React.
Add @types/react to peerDependencyMeta with optional:true to prevent users not using TS from requiring @types/react.

Changelog:

[GENERAL] [ADDED] Added @types/react as an optional peerDependency

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Mar 15, 2024
Copy link

github-actions bot commented Mar 15, 2024

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.
⚠️ 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.

Generated by 🚫 dangerJS against 315fc10

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Mar 15, 2024
@analysis-bot
Copy link

analysis-bot commented Mar 15, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,110,053 -13
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,475,090 +23
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 1021448
Branch: main

Comment on lines +10 to +11
export type {default} from './js/PopupMenuAndroid';
export type {PopupMenuAndroidInstance} from './js/PopupMenuAndroid';
Copy link
Contributor

@NickGerleman NickGerleman Mar 22, 2024

Choose a reason for hiding this comment

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

Thanks! FYI @RSNara

@facebook-github-bot
Copy link
Contributor

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@NickGerleman merged this pull request in dbf8e3f.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 25, 2024
@acoates-ms acoates-ms deleted the peerreacttypes branch March 27, 2024 16:20
huntie pushed a commit that referenced this pull request Apr 2, 2024
…43509)

Summary:
Now that RN is providing TS type information, many of those .d.ts files depend on types from react.  In modern packagemanagers (Ex: pnpm) types/react will not be available to RN since it does not declare it as a dependency.

I also noticed that the types for react-native-popup-menu-android appear to be pointing to the wrong location.

Add types/react as a peerDependency on the packages that have .d.ts files that import from React.
Add types/react to peerDependencyMeta with optional:true to prevent users not using TS from requiring types/react.

[GENERAL] [ADDED] Added types/react as an optional peerDependency

Pull Request resolved: #43509

Reviewed By: cortinico

Differential Revision: D55225940

Pulled By: NickGerleman

fbshipit-source-id: 4cbab071928cb925baec45f55461559acc9a54e6
This was referenced Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants