React-dates: React 16.9 warning

[maxmoeschinger]

I am getting this warning when running react-dates with react 16.9:

I am using react-dates version 20.2.5.

[ljharb]

Since we support versions of react older than 16.9, and older than the unsafe variants, that warning is unavoidable.

[NMinhNguyen]

that warning is unavoidable.

I don’t think that’s necessarily true. We should be able to use https://github.com/reactjs/react-lifecycles-compat which even has tests for React 0.14.9 and has been mentioned in an older issue https://github.com/airbnb/react-dates/issues/1179

[ljharb]

That’s fair; we’d have to make a breaking change to require the polyfill.

[NMinhNguyen]

I don’t think we need to put the requirement on users and can just apply the polyfill to all react-dates components ourselves:

Typically, this type of change would require third party libraries to release a new major version in order to adhere to semver. However, the react-lifecycles-compat polyfill offers a way to use the new lifecycles with older versions of React as well (0.14.9+) so no breaking release is required. This enables shared libraries to support both older and newer versions of React simultaneously.

package.json does seem to mention 0.14 as the minimum peer dependency though which is slightly outside the version range of react-lifecycles-compat

[ljharb]

hmm, I do wonder why 0.14.9 would be required instead of 0.14.0 - but a breaking change to hoist the minimum to 0.14.9 is something i'd happily do, since i doubt any 0.14 users are on "not the latest 0.14" at this point.

[AmberWilliams]

Is this going to be implemented?

[ranneyd]

Upvote

[ranneyd]

Part 1 of this fix is fixing it in one of this lib's dependencies.

https://github.com/airbnb/react-outside-click-handler/pull/36

[jeremy-clearlabs]

@ranneyd Looks like that was fixed in v1.3.0 from your PR. (airbnb/react-outside-click-handler#36) I tried to update package.json, but my tests weren't passing. Hoping someone else could raise a PR.

[Kivylius]

Is there any progress on this? Can we just release v22 which doesn't support the older versions? or implement the required fix thats above? If not this library is good as dead to me and I can no longer use it moving forward.

[ljharb]

Why would a warning that’s safe to ignore make a library “dead to you”?

[ranneyd]

@ljharb because the fix has been around since react 16.3 (a year ago) so highly used component libraries (this) could fix it before this warning happens (one year later).

If this lib hasn't fixed this by now, will it be fixed before React 17 comes out?

Also a warning like this not only pollutes the console but would be totally unacceptable at my company if it was from in-house code. What kind of company is AirBnB? They don't have standards?

It's also annoying as all hell.

warning that’s safe to ignore

What a stupid comment.

[ljharb]

Obviously it’ll be fixed before/when React 17 comes out - but that is a very long time from now.

The source of the warning is react itself; direct your commentary there. “highly used” also means “supports older React versions for a very long time”.

[sibelius]

are you willing to move to a hooks based implementation?

[ljharb]

@sibelius that would be a breaking change that would force React v16.9 or later, for minimal benefit.

[sibelius]

this would be v16.8 https://reactjs.org/blog/2019/02/06/react-v16.8.0.html

this is need to be compatible with Strict Mode and also enable concurrent mode of React

React v16.8 was released on February 2019

[ljharb]

@sibelius hooks isn't needed for that, avoiding the unsafe lifecycles is, which does not require hooks.

February 2019 is very very recent; there's people still on React 15 in production, even.

[MarkEhr]

Hi, anyone working on this? I could do a PR with the react-lifecycles-compat solution if there's no one.

[chaoming]

any solution on this?

[vedees]

any solution on this?

see here https://github.com/JedWatson/react-select/issues/3720#issuecomment-568778681

[ljharb]

That's not a solution; see https://github.com/airbnb/react-dates/issues/1748#issuecomment-519946312

[ranneyd]

@ljharb so is the plan to not have this lib support React 17? If you plan on supporting React 17, when do you plan on fixing this? You either have to drop <16.9 support, forego React 17+ support, or make a legacy version. How did this lib handle the React 15 -> 16 switch?

[ljharb]

React 17 is still a long ways off; eventually we'll do a major that requires React 16.3+, but there's no value in doing that until 17 is out.

React 16 didn't drop anything that was in React 15, so there wasn't really any ecosystem pain from it.

[bashirpour]

Use static getDerivedStateFromProps instead of that

For example:

static getDerivedStateFromProps(props, state) {
    if (props.currentRow !== state.lastRow) {
      return {
        isScrollingDown: props.currentRow > state.lastRow,
        lastRow: props.currentRow,
      };
    }

    // Return null to indicate no change to state.
    return null;
  }
}

https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html

[ljharb]

@bashirpour that requires React 16.3, which would be a breaking change for this package.

[pedroapfilho]

@ljharb do you think that it's a good idea for @MarkEhr to implement react-lifecycles-compat?

[sibelius]

https://twitter.com/dan_abramov/status/1227242568116604939

[ThiefMaster]

My guess would be that @airbnb is using ancient react versions internally and they can't / don't want to drop support for those versions yet because of this..

[PlayMa256]

Drop official support on master for that, move 15 support for another one and support them side by side

[gaearon]

To clarify, this is just my opinion :-). I'm not saying any library should necessarily follow this advice. This is what I would do if I were a maintainer, but I don't have the same constraints.

[sunstorymvp]

I hope this issue will be addressed as soon as possible.

Actually it hurts ecosystem. react-dates is very popular and widely used library. From when StrictMode component was added to react many of us want to use it to catch errors earlier. But this issue might stop many of us because of annoyance of those warnings.

[oleksandr-dziuban]

@sunstorymvp 👍

[ljharb]

@ThiefMaster that guess is incorrect.

@sunstorymvp What hurts the ecosystem is dropping support for older react versions, and in this case, React adding warnings in semver-minor versions.

@pedroapfilho I've thumbed-up https://github.com/airbnb/react-dates/issues/1748#issuecomment-559991882, so at the least it's waiting for a PR.

[MarkEhr]

I was trying a different approach moving the logic to componentDidUpdate but with some drawbacks, like potential infinite render loops and and extra render for each prop change.
But having @ljharb thumb I'll work on the react-lifecycles-compat PR, I hope it's ready before the end of the week (there are several components to fix).
I just hope it's not to much of a hassle to fit everything inside getDerivedStateFromProps
It's funny tho, that the examples in the dev enviroment don't support React ^0.14

[deivid11]

Viva Mark! @MarkEhr

[MarkEhr]

Hi guys, I have good news and bad news.

Good news - I've done the refactoring, it was not easy and a lot of code changed (which means many possible bugs). It works for me with no warnings in the latest react. I'll be testing it on a couple of production sites to quickly find and fix any bug. You can take a look, or npm install it from here

Bad news - It doesn't work with react ^0.14 which means it will not be merged to master soon. I believe the react-lifecycles-compat will be of no use for us. As stated in their readme it's no quite the same as the original methods and it doesn't work in some cases which it looks like they apply to this library.

[ljharb]

@MarkEhr does it work with 15+ with the compat package, or does it require 16.3+?

[MarkEhr]

@ljharb It does not work with 15+ although it seems easier to fix than 0.14+.
0.14+ is very broken.
15+ only breaks a little on the initial render, after changing a page it works well.
16.3+ works fine.

if you would be willing to accept a breaking change to require react 15+ I can work on fixing it. If we still require 0.14+ to be supported I don't think I may work on it anytime soon as it seems to be a lengthy work to do.

[burovmarley]

@MarkEhr do you know when it will be fixed? Maybe we should create react-dates-15 and leave it as it is. And for new react version create fixed version? Without that we will not be able migrate to react 17 when it will be ready.

[MarkEhr]

@burovmarley I don't know as I don't have any relation with the maintainers. What I know for the comments up here is that airbnb requires support for react 0.14.9, so I think it's improbable that a good fix will arive anytime soon.
To fix this for react 17 while maintaining support for 0.14.9 would require someone with a lot of time to spend as it needs a big refactor, not only moving code around but changing the internal component logic in clever ways.

I don't see any obvious answer to this problem.

In the meantime I have created a branch that removes the warning and added the build folders to the repo so it can be installed directly from github. I moved most of the logic to getDerivedStateFromProps and made the methods it calls static. It works well with react >= 16.3
You can install it and use it with:
npm i MarkEhr/react-dates#fix-react-deprecated-methods-warning --save

[ranneyd]

@burovmarley @MarkEhr I too have my own fork that pretty much does the same thing. I think most people in this thread do.

https://www.npmjs.com/package/react-dates-temp

[ljharb]

@burovmarley react 17 is not imminent, and when it comes out, obviously react-dates will release a version that works with it. The only consideration right now is the warning, which is a mild inconvenience but does not otherwise obstruct anything.

[Journerist]

I can't follow the dogmatic decision on all the related issues. Consider that, if the warning would just be an error then react would not work with this deprecated method. Instead, people argue why keeping backward compatibility is important. Still, the issue will become a major issue sooner or later and there is no point in keeping backward compatibility. The only thing you get is that a lot of people will be annoyed and distracted by this warning. Seriously, what about just fixing it and motivate people to stay up to date, implement little migrations instead of big ones, that will anyhow be required as soon as this method will not exist anymore.

[ljharb]

there is no point in keeping backward compatibility

There's always a point - users stuck on old React versions shouldn't be hung out to dry, and back compat is what makes upgrading easier.

[Journerist]

there is no point in keeping backward compatibility

There's always a point - users stuck on old React versions shouldn't be hung out to dry, and back compat is what makes upgrading _easier_.

So what do you gain by postponing the breaking change. Older react version apps will be forced to update, it will happen, you can't work around on that. I only see that the amount of work that people need to implement is getting more and more over time. The react guys give packages the possibility to use the new methods earlier to avoid heavy migrations. Instead of taking this opportunity, you postpone it like a lot of other packages and then the upcoming migrations will be a heavy piece.

[ljharb]

Why would it be a "heavy piece"? When we do finally make the breaking change, for users of React 16.3+, it will require nothing on their end but bumping the major version; only users of React < 16.3 will be stuck.

[Journerist]

Afaik the whole discussion is about breaking backward compatibility for React < 16.3. Hence, tech debt just gets more and more for react <16.3 apps.

[ljharb]

@Journerist and the best way to make it harder for them to upgrade, is if their dependencies drop support.

[Journerist]

🙈 Alright, I just realized that we have totally different approaches to handle and communicate the state of a project and tech debt. Delay the removal of deprecated methods, that actually are only deprecated and not removed to allow you to migrate, is not their intent ... but I will stop here, it's fine, we are running circles.

Anyhow, thanks for this library and I hope you will find a good solution in the future. ✌️

[ranneyd]

A year later, still no fix!

[ljharb]

@ranneyd there's nothing to "fix" because nothing is broken. The warning is that it will break in React 17, but this package doesn't support React 17 (which isn't actually released yet). Just ignore the warnings.

[ranneyd]

Release candidate is out. Gonna wait until it's released, then take a month to fix this issue that could have been fixed two years ago?

You're going to have to leave the React <16.3 people behind some day. You release this fix and what? They just _don't upgrade to the latest version of this lib_. It's not like the current version vanishes from the face of the earth. They're already not upgrading React and probably every other library they use that actually addressed this warning already. So they'll miss out on the latest and greatest date picker features? Are peoples' decision to upgrade to a newer version of React really hinging on getting the latest features of react-dates? People are like "I'm 3 versions behind in every other library I use but thank god I can click on dates that are outside the allowable date range so I don't have to upgrade to the latest version of React".

Just rip the band aid.

[ljharb]

@ranneyd absolutely we'll wait til it's out, because actually as that blog post implies, it won't be until React 18 that things are broken.

Again, this isn't a "fix" - it's a breaking change. You are correct that folks can choose not to upgrade, but that still leaves those users stranded on a likely unsupported version that won't get bugfixes, security updates, or new features.

At some point in the future, we'll do a breaking change, drop support for React < 16.9, and rename the appropriate lifecycle methods. Until then, it is, quite simply, a non problem that these warnings appear, since you can just ignore them.

[johncalvinyoung]

Well react v17 is out, and I just found out that my team pulled this library in and it's constantly emitting warnings that I cannot silence. I cannot even monkey-patch DateInput to rename the methods. When are we going to address this incompatibility?

https://reactjs.org/blog/2020/10/20/react-v17.html

[ljharb]

Indeed, react 17 came out yesterday.

Again, React 18 is when it's broken; React 16.9+ (including 17) just has the warnings.

I'd be happy to review a PR that was a breaking change that required React 16.3, but I don't think it'll be landed particularly soon (and if it were, i'd publish it as a prerelease, but that's added maintenance burden).

[johncalvinyoung]

actually, best I could tell, v17 won't call componentWillReceiveProps, just the UNSAFE_ rename. So I'm pretty sure it won't work at all on v17

[ljharb]

@johncalvinyoung ah, in that case we do need to make a change ASAP. I think we may be able to provide both componentWillReceiveProps and UNSAFE_ componentWillReceiveProps, which will probably produce a different warning, as a non-breaking change, to at least ensure react-dates works on react 17. A PR that does that would be most appreciated.

[ranneyd]

Good thing we're running around rushing to do this AFTER the release instead of years ago when we first got the warning 😂😂😂😂😂

[ljharb]

@ranneyd That's not helpful or productive. There is no rush for you or anyone else to upgrade to react 17, so there's also no rush here - it'd just be _nice_ to take care of it sooner than later.

[gaearon]

actually, best I could tell, v17 won't call componentWillReceiveProps, just the UNSAFE_ rename. So I'm pretty sure it won't work at all on v17

No, we did not make a breaking change regarding this in 17. We originally planned to, but we pushed all bigger deprecations to 18, so 17 is mostly about events-related changes. All breaking changes are enumerated here and as you can see it's not there.

[ljharb]

@gaearon awesome, thanks for clarifying!

In that case, we're back to: I'd be happy to review a PR that constitutes the breaking change of requiring React 16.3+ that avoids the warnings.

[vcarel]

@ljharb And lets' add support of React 17 in the peer dependencies too, no ? 😃

[ranneyd]

@ljharb

https://github.com/airbnb/react-dates/pull/2059

[OfirD1]

I'm not sure if that's expected behavior, but installing react-dates in a React 17 app downgrades it to 16.1.1. I thought it worth sharing to save time for anyone stumbling this like me.

To reproduce, do the following:

npx create-react-app test                 // this results with package.json: "react": "^17.0.1"
cd test
npx install-peerdeps react-dates          // this changes package.json to: "react": "^16.1.1"

[ljharb]

@OfirD1 it is quite expected that it downgrades it to react 16; ^16.1.1 includes the latest 16.x so it's not actually downgrading it that far.

[littlebigbot]

Again, React 18 is when it's broken; React 16.9+ (including 17) just has the warnings.

@ljharb I'm on React 17 and I can't even install it? Some other dependencies made me upgrade to 17, but I'll find some other date picker

[ranneyd]

Again, React 18 is when it's broken; React 16.9+ (including 17) just has the warnings.

@ljharb I'm on React 17 and I can't even install it? Some other dependencies made me upgrade to 17, but I'll find some other date picker

You can do what I did which is fork the repo, fix the issue, and publish on npm. I think mine is called react-dates-temp.

[ljharb]

Indeed; we don’t explicitly support react 17 yet, but it should work just fine.