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

Use UNSAFE_ lifecycle event names where supported. #1383

Merged
merged 1 commit into from
Aug 28, 2019
Merged

Use UNSAFE_ lifecycle event names where supported. #1383

merged 1 commit into from
Aug 28, 2019

Conversation

elyobo
Copy link

@elyobo elyobo commented Aug 21, 2019

This provides forward compatibility with React 17.x for those still
using the react-redux 5.x series by detecting support for the UNSAFE_
prefixed lifecycle event names (using React.createContext as a feature
check as it avoids semver checking but was introduced in React 16.3 at
the same time as the UNSAFE_ prefixes).

See discussion on #1374 for context.

@timdorr
Copy link
Member

timdorr commented Aug 22, 2019

Any thoughts on this approach vs remix-run/react-router#6883?

This provides forward compatibility with React 17.x for those still
using the react-redux 5.x series by detecting support for the UNSAFE_
prefixed lifecycle event names (using `React.createContext` as a feature
check as it avoids semver checking but was introduced in React 16.3 at
the same time as the UNSAFE_ prefixes).
@elyobo
Copy link
Author

elyobo commented Aug 23, 2019

@timdorr this has been updated to be consistent with the implementation in remix-run/react-router#6883 (using parseFloat on React.version instead of checking for React.createContext.

@elyobo
Copy link
Author

elyobo commented Aug 23, 2019

The various failures don't appear to be related to the changes in this PR.

@elyobo
Copy link
Author

elyobo commented Aug 27, 2019

@timdorr any interest in merging this? Essentially the same fix as the merged version in remix-run/react-router#6883

@timdorr
Copy link
Member

timdorr commented Aug 28, 2019

Yep, sorry, I've been distracted.

@timdorr timdorr merged commit 9137f7f into reduxjs:5.x Aug 28, 2019
@elyobo
Copy link
Author

elyobo commented Aug 28, 2019 via email

@elyobo elyobo deleted the UNSAFE-lifecycle-hook-support branch August 28, 2019 21:50
@elyobo
Copy link
Author

elyobo commented Aug 28, 2019

When you get time, a patch release for 5.x would be handy :)

@gregnb
Copy link

gregnb commented Sep 5, 2019

Same would love a release for this as well

@elyobo
Copy link
Author

elyobo commented Sep 23, 2019

@timdorr 5.x release for this still on the radar?

import PropTypes from 'prop-types'
import { storeShape, subscriptionShape } from '../utils/PropTypes'
import warning from '../utils/warning'

const prefixUnsafeLifecycleMethods = parseFloat(React.version) >= 16.3

Choose a reason for hiding this comment

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

@elyobo @timdorr Thanks for fixing this, but there's still one more issue here. parseFloat("16.10.1") parses as 16.1 which is of course less than 16.3, so this fails on the latest release of React.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's not good... Can you work up a fix PR?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I got it: #1407

Copy link
Author

Choose a reason for hiding this comment

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

Ha, we've gone back to a feature check like this PR originally shipped with before #1383 (comment) :D Thanks for the pick up Sam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants