Skip to content

[TM] add spec for RedBox#24922

Closed
ericlewis wants to merge 1 commit into
facebook:masterfrom
ericlewis:spec-RedBox
Closed

[TM] add spec for RedBox#24922
ericlewis wants to merge 1 commit into
facebook:masterfrom
ericlewis:spec-RedBox

Conversation

@ericlewis
Copy link
Copy Markdown
Contributor

Summary

part of #24875.

Changelog

[General] [Added] - add TM spec for RedBox

Test Plan

Flow passes

@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. Contributor A React Native contributor. labels May 16, 2019
@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label May 16, 2019

export default (Platform.OS === 'ios'
? TurboModuleRegistry.getEnforcing<Spec>('RedBox')
: null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use undefined here instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm good question, I didn't see anything about using it specifically. But it would be nice to have a consensus.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I saw some of the specific platforms PRs using undefined and others using null.
I don't have a strong opinion on that, but my preference is to go with undefined what denotes that this Native Module "was not defined" for this platform.

Anyway, I can update my PRs if we decide to go with null.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should actually be using get not get enforcing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I'll update it before I import/land to FB

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @ericlewis in c44d4f9.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 21, 2019
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
Summary:
part of facebook#24875.

## Changelog

[General] [Added] - add TM spec for RedBox
Pull Request resolved: facebook#24922

Reviewed By: RSNara

Differential Revision: D15423532

Pulled By: fkgozali

fbshipit-source-id: 3c30e5b32a29628caf0bb9286c0628597ac64fb7
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. Contributor A React Native contributor. Flow Merged This PR has been merged. Native Module Type: Enhancement A new feature or enhancement of an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants