Use workspace version of React for documentation

[jonkoops]

Removes React dependencies from the react-docs package, as this dependency is already present in the root of the workspace. Found this during the React 18 upgrade so this can be considered part of landing #7142.

[patternfly-build]

Preview: https://patternfly-react-pr-7217.surge.sh

A11y report: https://patternfly-react-pr-7217-a11y.surge.sh

[tlabaj]

@jonkoops react-docs is our local workspace app we use for testing our components. In the past we have had downgrade version to test some version specific changes. For those purposes we would like to keep this dependency.

[jonkoops]

@tlabaj I don't think this currently has a different version, as you can see below the root of the workspace has the same version range as react-docs.

patternfly-react/package.json

Lines 64 to 65 in b7823d1

	"react": "^17.0.0",
	"react-dom": "^17.0.0",

Since react-docs is in the workspace of the root with the same version these dependencies are already shared.

[tlabaj]

@tlabaj I don't think this currently has a different version, as you can see below the root of the workspace has the same version range as react-docs.

patternfly-react/package.json

Lines 64 to 65 in b7823d1

	"react": "^17.0.0",
	"react-dom": "^17.0.0",

Since react-docs is in the workspace of the root with the same version these dependencies are already shared.

That is correct @jonkoops, but in the past during testing we have updated versions to test specific version related issues. We would like to maintain flexibility.

[jonkoops]

We would like to maintain flexibility.

Can you explain to me how this removes that flexibility? If the root workspace version is upgraded it does not prevent anyone from downgrading the versions here at a later time by running yarn add react@x react-dom@x.

[tlabaj]

We would like to maintain flexibility.

Can you explain to me how this removes that flexibility? If the root workspace version is upgraded it does not prevent anyone from downgrading the versions here at a later time by running yarn add react@x react-dom@x.

That is true. It's more for documentation at the react-doc level so it is more intuitive for the devs. That is my opinion. I am open to other's thoughts here as well.

[jonkoops]

@jschuler @evwilkin any opinions on this?

[evwilkin]

Thanks for the feedback @jonkoops & we appreciate the help in updating for supporting React 18 🏆 I'll defer to @tlabaj here as the the lead for the PF react repo, but can you help me understand - does this dependency as it exists today block or disrupt your workflow in anyway that we're missing, or is this suggested change more for cleanup?

[jonkoops]

This does not block the React 18 upgrade, I would simply bump the version there. This work is coming from my research in upgrading React (#7146) where I found that this could simply be removed (see #7146 (comment)).

The reason why I'd recommend removing this is simply because it reduces maintenance burden of having multiple places where the same dependency is defined. This decreases the likeliness that someone will forget to update this in the future, which can cause unexpected behavior.

This PR does not remove this dependency as it is already defined in the workspace. Just like all other packages in the repo do not have an explicit dependency on React either. It simply brings this package in line with all the other ones.

[edewit]

Wouldn't it be better to document how to downgrade the version when needed then always have a static version here that can differ from the rest of the project. I agree with @jonkoops it seems to me like an unnecessary maintenance burden to keep this, just in case it might be needed

[jschuler]

LGTM, seems like a good approach, if we want to run react-docs with other package versions we can install those locally for test purposes.

[tlabaj]

@jonkoops do you mind just adding something to the contribution guide about downgrading. Or you can open an issue to that as well. Then we can take this in.

[jonkoops]

@tlabaj I've created #7303 so we can cover this.