Update react-is to 16.6

[frehner]

A followup to #6420 and #6417 , the version of React-is that was installed didn't allow lazy to actually pass validation -- it doesn't recognize it yet, since lazy was added in 16.6.

I was going to add a test for it, but that also requires updating the package's react and react-dom to 16.6 so I can import React, {lazy, Suspense} from 'react'. However, when I tried that update, other tests in other areas failed that I'm not that familiar with.

If you want me to look into that, I can. I'm just not familiar with those areas. :)

[timdorr]

This only affects our local testing, which doesn't use any of the newer APIs from 16.6.

Note that npm doesn't ship package-lock.json files. The version string provided in the package.json's dependency list is used to resolve the version obtained when installing the package. ^16.5.2 is loose enough to support anything up to version 17, including the 16.6.0 you're after.

This should be enforced by react/react-dom being at version 16.6.0, but the npm module resolution algorithm can be weird with conflicting dep versions. Is this something you're running into with beta.6?

[frehner]

Sorry, I'm confused -- react-is is in dependencies and not devDependencies, so whatever is in the package-lock.json is ultimately what's bundled with react-router, right?

The reason for this PR is because I'm using a lazy component, and with beta.6 it's still giving me a prop-type warning:

However, I could be misunderstanding what you're saying?

[pshrmn]

The package lock is only used for installing the actual react-router repo (git clone https://github.com/ReactTraining/react-router.git && npm install).

If you npm install react-router, you are installing a release and the dependencies from the package.json are installed.

If you remove your node_modules and re-install, the latest react-is should be installed.

[frehner]

Ah, maybe yarn handles this differently then. This is what happens with a clean start with yarn in a project that is using beta.6:

(screenshot is of yarn.lock file)

Note that react-is is still only at 16.5.2

[edit] hm, actually, I did a completely clean start with only adding react-router and react-router-dom in an empty directory/project, and it did install 16.6. So I'm missing something then :)

[frehner]

Turns out that the project was using enzyme-adapter-react-16@^1.6.0 which, at the time, had installed react-is@16.5.2

Because beta.6's version also matched that, it didn't install the newer 16.6 version of react-is, and thus used the older one that doesn't include lazy components.

So this PR would solve the issue in cases where projects already have a version of react-is@16.5.2. But if you feel that that's unnecessary, then I'll just leave it closed.

[frehner]

I'm still unsure that this should be closed -- if your project has react-is@16.5.2 already installed through another dependency, you're not going to get react-is@16.6 and your prop types are going to fail for lazy components.

[mjackson]

We can't update our React dep to version 16 w/out bumping our major version. I will note that I am currently planning on making version 5 dependent on React 16.7+, but that is still in alpha.

In the meantime, maybe it makes sense to ditch react-is for now and relax the propType to PropTypes.any with a note that says "let's upgrade this in v5". What do you think @frehner?

[frehner]

I believe we can upgrade ‘react-is’ independently of ‘react’, though I’m not 100% sure.

The only reason I mentioned upgrading react was because I wanted to add a test but can’t until it is upgraded :(

If react-is can’t be updated independently, then yeah I can make the change to something more relaxed.

(Sorry for formatting, on mobile)

[mjackson]

Just checked react-is 16.6 and there is no dependency on react, so I think that should work.

[frehner]

Is there anything preventing this from being merged that I can help with?

As seen in both #6420 and #6471 people are getting confused because the beta isn't updating react-is to the latest version. I'm just trying to help reduce the amount of confusion and comments :)

[timdorr]

I'm cool with it if @mjackson is cool with it.

[mjackson]

Thanks, @frehner!