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

Fix compatibility with React 16.9 #472

Closed
wants to merge 2 commits into from
Closed

Fix compatibility with React 16.9 #472

wants to merge 2 commits into from

Conversation

rifaidev
Copy link
Contributor

Fix for #465, #426 and #413 by updating react-side-effect to v2.0.0 since it's now using UNSAFE_componentWillMount instead of componentWillMount.

@CLAassistant
Copy link

CLAassistant commented Sep 15, 2019

CLA assistant check
All committers have signed the CLA.

@wheeler
Copy link

wheeler commented Sep 25, 2019

@rifaidev you need to change react and react-dom dependencies in package.json to >=16.3 or they don't match the requirements of react-side-effect. This'll be more than a patch update then too, probably a major.

@abumalick
Copy link

It would be nice to merge this PR please @tmbtech . It is really confusing to hack yarn.lock to make a warning disappear.

@Remzi1993
Copy link

@cwelch5 Could you look at this?

@verheyenkoen
Copy link

verheyenkoen commented Dec 6, 2019

@rifaidev #473 is now merged and Travis CI is building again. Maybe add an empty commit to trigger CI build again for this PR (or trigger manually if possible)?

@rifaidev
Copy link
Contributor Author

@rifaidev #473 is now merged and Travis CI is building again. Maybe add an empty commit to trigger CI build again for this PR (or trigger manually if possible)?

I think maintainers can manually trigger it.

@hitendramalviya
Copy link

Please merge this PR it become a need of hour to get our console free from such warning messages.

@verheyenkoen
Copy link

I think maintainers can manually trigger it.

@rifaidev Would you mind try pushing an empty commit? Maybe the maintainers won't pick this up until all checks are green and this could speed up merging.

@rifaidev
Copy link
Contributor Author

Would you mind try pushing an empty commit? Maybe the maintainers won't pick this up until all checks are green and this could speed up merging.

I just did, and still failing.

Actually I highly recommend switching to https://github.com/staylor/react-helmet-async instead.

@hitendramalviya
Copy link

This is such a popular repository, and I am amazed with the fact that an issue since last year still have no resolution even after a potential fix given in current PR. Seems authors are no more interested in this community work. I am disappointed to drop such a fantastic pacakge from my project I will add a plan to drop it or will use some alternative of this 😞

@@ -1,7 +1,7 @@
{
"name": "react-helmet",
"description": "A document head manager for React",
"version": "5.2.1",
"version": "5.2.2",

Choose a reason for hiding this comment

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

It can't be a patch upgrade, a new version of react-side-effect uses react >= 16.9

#501 (comment)

Copy link

@hitendramalviya hitendramalviya Jan 23, 2020

Choose a reason for hiding this comment

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

Okay understood, so need to create different pull react for future minor version or you already added into development plan? Asking because I am not seeing any label on this.

Choose a reason for hiding this comment

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

I've just noticed that upgrade of react-side-effect breaks backward compatibility (I’m not a maintainer of this repo)

Personally I would like to help to resolve react depreciations, but it looks like react-helmet-async appeared for a reason with the same intentions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, and @wheeler mentioned so already few months ago, however since this package is no longer maintained, I switched to react-helmet-async and looks like everyone is doing the same.

Screen Shot 2020-01-23 at 08 59 00

Choose a reason for hiding this comment

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

I already added in my development plan, soon I will be switched to react-helmet-async. Thanks @maxmalov & @rifaidev for quick response.

Choose a reason for hiding this comment

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

I switched today - it's fairly simple for a client-side rendered React project, you just have to add the provider and then replace all imports and that's pretty much it.

@cwelch5
Copy link
Contributor

cwelch5 commented Jan 27, 2020

Full 6.0.0 release coming soon. In the meantime, 6.0.0-beta.2 will work with the latest React version.

@rifaidev rifaidev deleted the release/5.2.2 branch January 28, 2020 08:24
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.

10 participants