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

Warning should appear when versions of react and react-dom do not match #14443

Open
mjhoffm2 opened this issue Dec 14, 2018 · 9 comments
Open

Comments

@mjhoffm2
Copy link

Do you want to request a feature or report a bug?

Request a feature

What is the current behavior?

If the version of react and react-dom do not match, some features fail silently. See this issue for example: reduxjs/react-redux#1125

In this issue, the new Context API wasn't working as intended, but no errors or warnings were visible. Components simply did not update. It turns out that this issue was because I updated react to version 16.6.3, but still had react-dom at version 16.5.

What is the expected behavior?

I would like to see some sort of warning message in the console in development mode when the versions of react and react-dom do not match.

@nhunzaker
Copy link
Contributor

I think this is a neat idea and would be easy to add. Filing this as a good first issue.

@aweary
Copy link
Contributor

aweary commented Jan 18, 2019

We were just discussing this in #14572

@jesperhodge
Copy link

I'd like to do it!

@jesperhodge
Copy link

As far as I could tell - I have been trying around a bit on a branch on my fork - the simplest way is just to compare the versions in a warning that relies, for example, on the version string passed as attribute of the React object in the React.js file.

ReactDOM does not have something like that, at least at first glance, so I'd like to add a version attribute there, too.

And right now the React version is imported from a file in the shared folder that just has the version hardcoded. In both of these cases, I prefer to read the version from the respective package.json before react is built for distribution. I'm just wondering if there is any reason not to do that (since it was not done that way before)? Is there an edge case where the value can end up wrong?

Forgive my very basic questions, I am yet a humble beginner in the react source code.

@teleginzhenya
Copy link

teleginzhenya commented Jan 21, 2019

Unfortunately didn't see that Aliin works on this issue and took a look too (:

@aliin probably in order to make code cleaner, since React version used in different packages.
@aweary in which cases warning should be thrown? Diff even in path, or minor version?

@aweary
Copy link
Contributor

aweary commented Jan 21, 2019

Based on the conversation in #14572 it's not clear that this is actually something we want to do. We'll need some input from @gaearon and/or the other core team members.

@nhunzaker
Copy link
Contributor

Ah, sorry I'm just getting to this. I've re-labelled this and let's keep the conversation going.

@mjhoffm2
Copy link
Author

Based on what @gaearon said in that issue

We kind of wanted react-dom to keep working with react of the same major without breaking. I don't think a strict match is necessarily desired unless you're trying to use new features — in which case you should have at least that version of both.

The ideal scenario is that React works fine with react-dom as long as the major version matches. In that scenario, we wouldn't need warnings when there is a mismatch in the minor/patch numbers. However, in my original issue, this is unfortunately not the case and there can be silent breaking changes between minor versions. I don't think it is realistic to have extensive compatibility testing between each combination of versions to ensure these issues don't appear. To me it is clear that some kind of warning should be presented, at a minimum.

@jesperhodge
Copy link

I thought about it a bit, a warning would be really nice, but it does not seem like an optimal solution in any way. If there is a minor version mismatch that won't be fixed for some reason and does not cause problems, the warning can be annoying.
Since the warning cannot fix the past version conflicts anyway, for the future this might just be a motivation instead to work even more on stability. It might be wise to test the upcoming release well against the last 2 minor versions of react-dom to prevent problems, either that or just add the warning temporarily for some of the minor releases if there is uncertainty. In the future the stability can be further improved, or another solution for mismatches can be explored. Just my thoughts on the issue.

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

No branches or pull requests

5 participants