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

removed defaultprops from DatePickerIOS #31619

Closed
wants to merge 2 commits into from
Closed

removed defaultprops from DatePickerIOS #31619

wants to merge 2 commits into from

Conversation

turker0
Copy link

@turker0 turker0 commented May 27, 2021

Summary

Removed defaultProps from DatePickeriOS #31605 .

Changelog

[iOS] [Removed] - Removed defaultProps from DatePickerIOS.ios.js

Test Plan

Build & render DatePickeriOS on RNTester app successfully.
image

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 27, 2021
@analysis-bot
Copy link

analysis-bot commented May 27, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: cb610dd
Branch: main

@lunaleaps lunaleaps self-assigned this May 28, 2021
@@ -117,10 +117,6 @@ type Props = $ReadOnly<{|
* source of truth.
*/
class DatePickerIOS extends React.Component<Props> {
static DefaultProps: {|mode: $TEMPORARY$string<'datetime'>|} = {
mode: 'datetime',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need to set the default value of this prop. So for every access of props.mode, if no value is set it still needs to be datetime -- the thing we want to remove is depending on defaultProps static type as a construct used in React.

You can take a look at destructuring with a default value here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

Copy link
Contributor

Choose a reason for hiding this comment

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

In addition, can you add a snapshot test that ensures the default value is used? You can take a look at an example component snapshot test from VirtualizedList-test.js: https://github.com/facebook/react-native/blob/dc80b2dcb52fadec6a573a9dd1824393f8c29fdc/Libraries/Lists/__tests__/VirtualizedList-test.js

@lunaleaps lunaleaps linked an issue May 31, 2021 that may be closed by this pull request
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,527,058 +0
android hermes armeabi-v7a 7,830,789 +0
android hermes x86 8,996,981 +0
android hermes x86_64 8,942,879 +0
android jsc arm64-v8a 9,841,448 +0
android jsc armeabi-v7a 8,806,393 +0
android jsc x86 9,791,324 +0
android jsc x86_64 10,393,964 +0

Base commit: cb610dd
Branch: main

This pull request was closed.
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. Needs: Author Feedback Platform: iOS iOS applications. Type: Removal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove defaultProps in DatePickeriOS
5 participants