remove defaultProps from picker of components

[kerm1it]

Summary

Remove defaultProps from Picker of components, replace it with destructuring assignment.

Changelog

[JavaScript] [Changed] - Remove defaultProps from picker

close #31603

Test Plan

all test suite and CI passes.

[analysis-bot]

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

Base commit: 819b926

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	9,240,048	+1,159
android	hermes	armeabi-v7a	8,749,749	+506
android	hermes	x86	9,702,653	+1,356
android	hermes	x86_64	9,667,530	+1,133
android	jsc	arm64-v8a	10,885,363	-1,239
android	jsc	armeabi-v7a	9,786,326	-1,913
android	jsc	x86	10,943,370	-1,042
android	jsc	x86_64	11,549,773	-1,272

Base commit: 58444c7

[lunaleaps]

Oh interesting, I wouldn't have thought these snapshots would change. Shouldn't we still be setting the prop?

[kerm1it]

Because snap only print props of passing to Picker.
After removing defaultProps，it doesn't exist in props.

[lunaleaps]

It should still exist in the props we pass because we're setting a default value via the destructured assignment
{mode = MODE_DIALOG...

[kerm1it]

No, it is a single var rather than a part of props by destructured assignment.

[elicwhite]

I think this change is expected. This test is when the component is mocked, so what shows up in the snapshot is exactly what props were passed to Picker, ignoring anything internal to picker.

The change in the test below this kinda surprises me because the title of the test says that isn't mocked. I bet something is wrong in the test there.

[kerm1it]

I found that the results of mock and non-mock are the same in original snapshot, both showing the props passed to Picker.

[lunaleaps]

Ah I see, missed the fact that it was mocked. @kerm1it did you want to look into why the second snapshot isn't passing mode? If not, I can follow up

[kerm1it]

I have explained the reason in #31644 (comment) from my opinion.

If you can follow up, I will be happy.

[lunaleaps]

Thanks for working on this @kerm1it! Appreciate your help~

[kerm1it]

I removed this.props.children，because rest has included children prop.

[lunaleaps]

Somewhat of a nit and open for discussion but I think it'd be clearer if we kept the children as the "inner child" of this component but I don't feel strongly. We can extract it from this.props on line 170 so you can still spread rest

[kerm1it]

<PickerIOS mode={mode} {...rest}>
  {children}
</PickerIOS>

this way will appear flow errors:

[lunaleaps]

Ah I see, missed the fact that it was mocked. @kerm1it did you want to look into why the second snapshot isn't passing mode? If not, I can follow up

[facebook-github-bot]

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

[facebook-github-bot]

@lunaleaps merged this pull request in b4cde15.

[kerm1it]

b4cde15

There was an obvious problem:

return <PickerIOS {...rest}>{children}</PickerIOS>;

miss mode prop

<PickerAndroid mode={mode}>{children} </PickerAndroid>

miss ...rest

[lunaleaps]

b4cde15

There was an obvious problem:

return <PickerIOS {...rest}>{children}</PickerIOS>;

miss mode prop

<PickerAndroid mode={mode}>{children} </PickerAndroid>

miss ...rest

Ah yup good point. I mistakenly left out the spread of rest. For mode I realized that it's only relevant for Android. Will re-land this