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.

Sign up for GitHub

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

Jump to bottom

changing name of deprecated react lifecycle methods #2059

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

changing name of deprecated react lifecycle methods #2059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6f1d82
changing name of deprecated react lifecycle methods
ranneyd Oct 30, 2020
e171106
fixing travis and removing react 17
ranneyd Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@

"react/jsx-props-no-spreading": 0, // TODO: re-evaluate

"react/no-deprecated": 1, // TODO: update to UNSAFE_componentWillReceiveProps
"camelcase": ["error", {
"properties": "never",
"ignoreDestructuring": false,
"allow": [
"UNSAFE_componentWillReceiveProps",
"UNSAFE_componentWillUpdate",
],
}],
},

"overrides": [
Expand Down
2 changes: 0 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ env:
global:
- TEST=true
matrix:
- REACT=0.14
- REACT=15
- REACT=16
sudo: false
matrix:
Expand Down
8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@
"node-sass": "^4.12.0",
"nyc": "^14.1.1",
"raw-loader": "^0.5.1",
"react": "^0.14 || ^15.5.4 || ^16.1.1",
"react-dom": "^0.14 || ^15.5.4 || ^16.1.1",
"react": "^16.3.0",
"react-dom": "^16.3.0",
"react-with-styles-interface-aphrodite": "^6.0.0",
"react-with-styles-interface-css-compiler": "^2.2.0",
"rimraf": "^2.6.3",
Expand Down Expand Up @@ -135,8 +135,8 @@
"peerDependencies": {
"@babel/runtime": "^7.0.0",
"moment": "^2.18.1",
"react": "^0.14 || ^15.5.4 || ^16.1.1",
"react-dom": "^0.14 || ^15.5.4 || ^16.1.1",
"react": "^16.3.0",
"react-dom": "^16.3.0",
"react-with-direction": "^1.3.1"
},
"greenkeeper": {
Expand Down
2 changes: 1 addition & 1 deletion src/components/CalendarMonth.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class CalendarMonth extends React.PureComponent {
this.setMonthTitleHeightTimeout = setTimeout(this.setMonthTitleHeight, 0);
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
Copy link
Member

Choose a reason for hiding this comment

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

if we're doing the breaking change, we might as well migrate to getDerivedStateFromProps where we can? It's ok to keep the unsafe versions where that's not easy tho.

Copy link
Author

Choose a reason for hiding this comment

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

I have a few thoughts on this:

  • I want to rock the boat with this PR as little as possible. If people are upgrading from an older react to 16.3+, then upgrading this, if they have a bug it would be nice to say fairly confidently that it wasn't this and probably part of some other part of their migration.
  • Related, a ton of the logic in these components is in these lifecycle methods, so I see any substantive change to them as a larger-scope change.
  • From my understanding, these methods aren't going anywhere anytime soon. My understanding is the react team has great interest in discouraging their use rather than removing them.
  • If you think this is the time/PR to do this lmk and I can see what I can do. Might be more appropriate to do it in two parts.
  • Depending on what the future holds, this lib might go the way of many of the other libs out there and get a functional, hooks-based refactor. Maybe instead of migrating all this huge lifecycle method logic twice we just wait until that day comes (though I believe the migration to getDerivedStateFromProps is not nearly on the same scale as a migration to hooks, which would be...non-trivial.)

I will say in general I'm all about the "while we're at it" mentality. If it were my own repo or wasn't 427,576 weekly downloads then I'd feel more comfortable just doing it 😂 .

My vote is to do it this dumb, rename-only way first to get the warnings out and get the compatibility there, then either have a follow-up PR or include this in future refactoring conversations/planning.

Copy link
Member

Choose a reason for hiding this comment

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

If we're going to migrate to hooks any time soon, we'd need to require React v16.9 now, and not just v16.3, even if we do this PR as-is first, and change other things later.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah definitely not advocating hooks migration in the near-term.

const { month, enableOutsideDays, firstDayOfWeek } = nextProps;
const {
month: prevMonth,
Expand Down
2 changes: 1 addition & 1 deletion src/components/CalendarMonthGrid.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class CalendarMonthGrid extends React.PureComponent {
);
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
const { initialMonth, numberOfMonths, orientation } = nextProps;
const { months } = this.state;

Expand Down
2 changes: 1 addition & 1 deletion src/components/DateInput.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class DateInput extends React.PureComponent {
this.setState({ isTouchDevice: isTouchDevice() });
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
const { dateString } = this.state;
if (dateString && nextProps.displayValue) {
this.setState({
Expand Down
4 changes: 2 additions & 2 deletions src/components/DayPicker.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ class DayPicker extends React.PureComponent {
this.setCalendarMonthWeeks(currentMonth);
}

componentWillReceiveProps(nextProps, nextState) {
UNSAFE_componentWillReceiveProps(nextProps, nextState) {
const {
hidden,
isFocused,
Expand Down Expand Up @@ -361,7 +361,7 @@ class DayPicker extends React.PureComponent {
}
}

componentWillUpdate() {
UNSAFE_componentWillUpdate() {
const { transitionDuration } = this.props;

// Calculating the dimensions trigger a DOM repaint which
Expand Down
2 changes: 1 addition & 1 deletion src/components/DayPickerKeyboardShortcuts.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class DayPickerKeyboardShortcuts extends React.PureComponent {
this.onKeyDown = this.onKeyDown.bind(this);
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
const { phrases } = this.props;
if (nextProps.phrases !== phrases) {
this.keyboardShortcuts = getKeyboardShortcuts(nextProps.phrases);
Expand Down
2 changes: 1 addition & 1 deletion src/components/DayPickerRangeController.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ export default class DayPickerRangeController extends React.PureComponent {
this.getFirstFocusableDay = this.getFirstFocusableDay.bind(this);
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
const {
startDate,
endDate,
Expand Down
4 changes: 2 additions & 2 deletions src/components/DayPickerSingleDateController.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export default class DayPickerSingleDateController extends React.PureComponent {
this.isTouchDevice = isTouchDevice();
}

componentWillReceiveProps(nextProps) {
UNSAFE_componentWillReceiveProps(nextProps) {
const {
date,
focused,
Expand Down Expand Up @@ -344,7 +344,7 @@ export default class DayPickerSingleDateController extends React.PureComponent {
}
}

componentWillUpdate() {
UNSAFE_componentWillUpdate() {
this.today = moment();
}

Expand Down
4 changes: 2 additions & 2 deletions test/components/CalendarMonthGrid_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('CalendarMonthGrid', () => {
<CalendarMonthGrid numberOfMonths={12} initialMonth={initialMonth} />
)).dive();

wrapper.instance().componentWillReceiveProps({
wrapper.instance().UNSAFE_componentWillReceiveProps({
initialMonth,
numberOfMonths: 24,
});
Expand All @@ -51,7 +51,7 @@ describe('CalendarMonthGrid', () => {
<CalendarMonthGrid numberOfMonths={12} initialMonth={initialMonth} />
)).dive();

wrapper.instance().componentWillReceiveProps({
wrapper.instance().UNSAFE_componentWillReceiveProps({
initialMonth,
numberOfMonths: 12,
firstVisibleMonthIndex: 0,
Expand Down
6 changes: 3 additions & 3 deletions test/components/DateInput_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@ describe('DateInput', () => {
});
});

describe('#componentWillReceiveProps', () => {
describe('#UNSAFE_componentWillReceiveProps', () => {
describe('nextProps.displayValue exists', () => {
it('sets state.dateString to \'\'', () => {
const dateString = 'foo123';
const wrapper = shallow(<DateInput id="date" />).dive();
wrapper.setState({ dateString });
wrapper.instance().componentWillReceiveProps({ displayValue: '1991-07-13' });
wrapper.instance().UNSAFE_componentWillReceiveProps({ displayValue: '1991-07-13' });
expect(wrapper.state()).to.have.property('dateString', '');
});
});
Expand All @@ -137,7 +137,7 @@ describe('DateInput', () => {
const dateString = 'foo123';
const wrapper = shallow(<DateInput id="date" />).dive();
wrapper.setState({ dateString });
wrapper.instance().componentWillReceiveProps({ displayValue: null });
wrapper.instance().UNSAFE_componentWillReceiveProps({ displayValue: null });
expect(wrapper.state()).to.have.property('dateString', dateString);
});
});
Expand Down
6 changes: 3 additions & 3 deletions test/components/DayPickerKeyboardShortcuts_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import DayPickerKeyboardShortcuts from '../../src/components/DayPickerKeyboardSh
const event = { preventDefault: sinon.stub(), stopPropagation: sinon.stub() };

describe('DayPickerKeyboardShortcuts', () => {
describe('#componentWillReceiveProps', () => {
describe('#UNSAFE_componentWillReceiveProps', () => {
describe('when the phrases have been updated', () => {
const prevProps = { phrases: { enterKey: 'foo', escape: 'bar', questionMark: 'baz' } };
const newProps = { phrases: { enterKey: 'bleep', escape: 'blah', questionMark: 'boop' } };

it('updates the keyboardShortcuts', () => {
const wrapper = shallow(<DayPickerKeyboardShortcuts {...prevProps} />).dive();
const prevKeyboardShortcuts = wrapper.instance().keyboardShortcuts;
wrapper.instance().componentWillReceiveProps(newProps);
wrapper.instance().UNSAFE_componentWillReceiveProps(newProps);
const updatedKeyboardShortcuts = wrapper.instance().keyboardShortcuts;
expect(prevKeyboardShortcuts).to.not.equal(updatedKeyboardShortcuts);
});
Expand All @@ -33,7 +33,7 @@ describe('DayPickerKeyboardShortcuts', () => {
it('does NOT update the keyboardShortcuts', () => {
const wrapper = shallow(<DayPickerKeyboardShortcuts {...prevProps} />).dive();
const prevKeyboardShortcuts = wrapper.instance().keyboardShortcuts;
wrapper.instance().componentWillReceiveProps(newProps);
wrapper.instance().UNSAFE_componentWillReceiveProps(newProps);
const updatedKeyboardShortcuts = wrapper.instance().keyboardShortcuts;
expect(prevKeyboardShortcuts).to.deep.equal(updatedKeyboardShortcuts);
});
Expand Down