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

Migrate UNSAFE_componentWillReceiveProps #1957

Merged
merged 13 commits into from
Jun 19, 2019
10 changes: 5 additions & 5 deletions src/apps/expenses/components/CreateExpenseForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ class CreateExpenseForm extends React.Component {
expense: {
category: Object.keys(this.categoriesOptions[0])[0],
payoutMethod: 'paypal',
paypalEmail: (props.LoggedInUser && props.LoggedInUser.paypalEmail) || null,
paypalEmail: (props.LoggedInUser && props.LoggedInUser.paypalEmail) || undefined,
},
isExpenseValid: false,
loading: false,
};
}

UNSAFE_componentWillReceiveProps(newProps) {
if (!this.props.LoggedInUser && newProps.LoggedInUser && !this.state.expense.paypalEmail) {
this.handleChange('paypalEmail', newProps.LoggedInUser.paypalEmail);
componentDidUpdate(prevProps) {
if (!prevProps.LoggedInUser && this.props.LoggedInUser && !this.state.expense.paypalEmail) {
this.handleChange('paypalEmail', this.props.LoggedInUser.paypalEmail);
}
}

Expand Down Expand Up @@ -463,7 +463,7 @@ class CreateExpenseForm extends React.Component {
type="email"
name="paypalEmail"
key={`paypalEmail-${get(LoggedInUser, 'id')}`}
defaultValue={this.state.expense.paypalEmail}
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially, paypalEmail is null before the prop updates. defaultValue binds to the initial value during the first render so it doesn't change if the user is actually logged in and has a default email.
This change fixes that.

value={this.state.expense.paypalEmail}
onChange={paypalEmail => this.handleChange('paypalEmail', paypalEmail)}
/>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/apps/expenses/components/TransactionsWithData.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class TransactionsWithData extends React.Component {
super(props);
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (!this.props.LoggedInUser && nextProps.LoggedInUser) {
componentDidUpdate(prevProps) {
if (!prevProps.LoggedInUser && this.props.LoggedInUser) {
return this.props.data.refetch();
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/components/CreateCollectiveForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ class CreateCollectiveForm extends React.Component {
}
}

UNSAFE_componentWillReceiveProps(nextProps) {
componentDidUpdate(prevProps) {
if (
nextProps.collective &&
(!this.props.collective || get(nextProps, 'collective.name') != get(this.props, 'collective.name'))
this.props.collective &&
(!prevProps.collective || get(this.props, 'collective.name') !== get(prevProps, 'collective.name'))
) {
this.setState({
collective: nextProps.collective,
tiers: nextProps.collective.tiers,
collective: this.props.collective,
tiers: this.props.collective.tiers,
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/EditEventForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ class EditEventForm extends React.Component {
});
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (nextProps.event && (!this.props.event || nextProps.event.name != this.props.event.name)) {
this.setState({ event: nextProps.event, tiers: nextProps.event.tiers });
componentDidUpdate(prevProps) {
if (this.props.event && (!prevProps.event || this.props.event.name !== prevProps.event.name)) {
this.setState({ event: this.props.event, tiers: this.props.event.tiers });
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/InputField.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ class InputField extends React.Component {
this.handleChange = this.handleChange.bind(this);
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (nextProps.value != this.props.value) {
this.setState({ value: nextProps.value });
componentDidUpdate(prevProps) {
if (this.props.value !== prevProps.value) {
this.setState({ value: this.props.value });
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/InputTypeCreditCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ class InputTypeCreditCard extends React.Component {
}
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (nextProps.options && nextProps.options.length > 0) {
componentDidUpdate() {
if (this.props.options && this.props.options.length > 0) {
if (typeof this.state.uuid !== 'string') {
this.handleChange('uuid', nextProps.options[0].uuid);
this.handleChange('uuid', this.props.options[0].uuid);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/InputTypeLocation.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ class InputTypeLocation extends React.Component {
this.state = { value: props.value || {} };
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (nextProps.value != this.props.value) {
this.setState({ value: nextProps.value });
componentDidUpdate(prevProps) {
if (this.props.value !== prevProps.value) {
this.setState({ value: this.props.value });
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/components/MatchingFundWithData.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ class MatchingFundWithData extends React.Component {
this.props.onChange(this.uuid);
}

// Whenever this.props.order.totalAmount changes, we update the status
async UNSAFE_componentWillReceiveProps(newProps) {
async componentDidUpdate(prevProps) {
const {
data: { loading, MatchingFund },
collective,
order,
uuid,
} = newProps;
} = this.props;
if (loading) return;

// if the matching fund id doesn't return any matching fund, we notify the OrderForm
Expand All @@ -42,7 +41,7 @@ class MatchingFundWithData extends React.Component {
return;
}

if (order.totalAmount === this.props.order.totalAmount) return;
if (order.totalAmount === prevProps.order.totalAmount) return;
const currency = get(MatchingFund, 'currency');
if (currency && currency !== collective.currency) {
this.fxrate = await getFxRate(collective.currency, currency);
Expand Down
34 changes: 18 additions & 16 deletions src/components/PaymentMethodChooser.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,31 @@ class PaymentMethodChooser extends React.Component {
});
}

UNSAFE_componentWillReceiveProps(newProps) {
componentDidUpdate(prevProps) {
// TODO: Remove these hacky fixes
// Most likely means need to rework the logic split between this and SubscriptionCard component

const { paymentMethodInUse, paymentMethodsList, editMode } = newProps;
const { paymentMethodInUse, paymentMethodsList, editMode } = this.props;

if (!paymentMethodInUse.name) {
// set state to modified
this.setState({ modified: true });
// If there was an existing card, select that
if (paymentMethodsList.length > 0) {
this.handleChange({ uuid: paymentMethodsList[0].uuid });
if (!prevProps.paymentMethodsList && paymentMethodsList) {
Copy link
Member

Choose a reason for hiding this comment

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

Why introducing this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid infinite loops, since we're setting state in componentDidUpdate. This ensures that the state will only be updated for this condition.

if (!paymentMethodInUse.name) {
// set state to modified
this.setState({ modified: true });
// If there was an existing card, select that
if (paymentMethodsList.length > 0) {
this.handleChange({ uuid: paymentMethodsList[0].uuid });
}
}
}

// hack to revert back to cc selector
if (!this.props.editMode && editMode) {
this.setState({ showNewCreditCardForm: false });
}
// hack to revert back to cc selector
if (!prevProps.editMode && editMode) {
this.setState({ showNewCreditCardForm: false });
}

// handles the case where there are no existing credit cards
if (paymentMethodsList.length === 0 && editMode) {
this.setState({ showNewCreditCardForm: true });
// handles the case where there are no existing credit cards
if (paymentMethodsList.length === 0 && editMode) {
this.setState({ showNewCreditCardForm: true });
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/components/Subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class Subscriptions extends React.Component {
});
}

UNSAFE_componentWillReceiveProps(nextProps) {
if (!this.props.LoggedInUser && nextProps.LoggedInUser) {
componentDidUpdate(prevProps) {
if (!prevProps.LoggedInUser && this.props.LoggedInUser) {
return this.props.refetch();
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/UpdatesWithData.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ class UpdatesWithData extends React.Component {
};
}

UNSAFE_componentWillReceiveProps(newProps) {
componentDidUpdate(prevProps) {
const { data, collective } = this.props;
const { LoggedInUser } = newProps;
if (LoggedInUser && LoggedInUser.canEditCollective(collective)) {
const { LoggedInUser } = this.props;
if (!prevProps.LoggedInUser && LoggedInUser && LoggedInUser.canEditCollective(collective)) {
// We refetch the data to get the updates that are not published yet
data.refetch({ options: { fetchPolicy: 'network-only' } });
}
Expand Down
4 changes: 0 additions & 4 deletions src/pages/banner-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ class BannerIframe extends React.Component {
this.onChange();
}

UNSAFE_componentWillReceiveProps() {
this.onChange();
}

componentDidUpdate() {
this.onChange();
}
Expand Down