Skip to content

Commit

Permalink
Fix an issue with unsafe picker access to setNativeProps.
Browse files Browse the repository at this point in the history
Summary:
Fixing an issue where PickerIOS and DatePicker are being accessed unsafely, As a side effect we are also using ref callbacks as oppose to strings.

Fixed after spotting an issue in our app where the picker is closed and the callback attempts to update native props for an item that no longer exists.
Closes facebook#3920

Reviewed By: svcscm

Differential Revision: D2663634

Pulled By: nicklockwood

fb-gh-sync-id: 813b32a038f59864401d5d3985c7ea32f5e13301
  • Loading branch information
admmasters authored and facebook-github-bot-9 committed Dec 4, 2015
1 parent 4661e59 commit c1849e7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 10 deletions.
8 changes: 3 additions & 5 deletions Libraries/Components/DatePicker/DatePickerIOS.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ var View = require('View');

var requireNativeComponent = require('requireNativeComponent');

var DATEPICKER = 'datepicker';

type DefaultProps = {
mode: 'date' | 'time' | 'datetime';
};
Expand Down Expand Up @@ -108,8 +106,8 @@ var DatePickerIOS = React.createClass({
// certain values. In other words, the embedder of this component should
// be the source of truth, not the native component.
var propsTimeStamp = this.props.date.getTime();
if (nativeTimeStamp !== propsTimeStamp) {
this.refs[DATEPICKER].setNativeProps({
if (this._picker && nativeTimeStamp !== propsTimeStamp) {
this._picker.setNativeProps({
date: propsTimeStamp,
});
}
Expand All @@ -120,7 +118,7 @@ var DatePickerIOS = React.createClass({
return (
<View style={props.style}>
<RCTDatePickerIOS
ref={DATEPICKER}
ref={ picker => this._picker = picker }
style={styles.datePickerIOS}
date={props.date.getTime()}
maximumDate={
Expand Down
8 changes: 3 additions & 5 deletions Libraries/Picker/PickerIOS.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ var View = require('View');

var requireNativeComponent = require('requireNativeComponent');

var PICKER = 'picker';

var PickerIOS = React.createClass({
mixins: [NativeMethodsMixin],

Expand Down Expand Up @@ -57,7 +55,7 @@ var PickerIOS = React.createClass({
return (
<View style={this.props.style}>
<RCTPickerIOS
ref={PICKER}
ref={ picker => this._picker = picker }
style={styles.pickerIOS}
items={this.state.items}
selectedIndex={this.state.selectedIndex}
Expand All @@ -81,8 +79,8 @@ var PickerIOS = React.createClass({
// disallow/undo/mutate the selection of certain values. In other
// words, the embedder of this component should be the source of
// truth, not the native component.
if (this.state.selectedIndex !== event.nativeEvent.newIndex) {
this.refs[PICKER].setNativeProps({
if (this._picker && this.state.selectedIndex !== event.nativeEvent.newIndex) {
this._picker.setNativeProps({
selectedIndex: this.state.selectedIndex
});
}
Expand Down

0 comments on commit c1849e7

Please sign in to comment.