[DevTools] Add --replaceBuild option to Older React Builds Download Script

[lunaruan]

This PR adds a --replaceBuild option to the script that downloads older React version builds. If this flag is true, we will replace the contents of the build folder with the contents of the build-regression folder and remove the build-regression folder after, which was the original behavior.

However, for e2e tests, we need both the original build (for DevTools) and the new build (for the React Apps), so we need both the build and the build-regression folders. Not adding the --replaceBuild option will do this.

This PR also modifies the circle CI config to reflect this change.

[sizebot]

Comparing: 05c34de...bd1a9f4

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	131.28 kB	131.28 kB	=	42.13 kB	42.13 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	136.54 kB	136.54 kB	=	43.68 kB	43.68 kB
facebook-www/ReactDOM-prod.classic.js	=	439.35 kB	439.35 kB	=	80.29 kB	80.29 kB
facebook-www/ReactDOM-prod.modern.js	=	424.64 kB	424.64 kB	=	78.13 kB	78.13 kB
facebook-www/ReactDOMForked-prod.classic.js	=	439.35 kB	439.35 kB	=	80.29 kB	80.29 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against bd1a9f4

[bvaughn]

I'm not sure I really understand what's going on with this script.

Won't replacing the contents of build with the older versions we downloaded also cause DevTools itself to run with those older versions? Since we're downloading both react-dom and react-test-renderer?

const INSTALL_PACKAGES = ['react-dom', 'react', 'react-test-renderer'];

[lunaruan]

Won't replacing the contents of build with the older versions we downloaded also cause DevTools itself to run with those older versions? Since we're downloading both react-dom and react-test-renderer?

Yeah it does, but we're only using the older versions for Jest tests that render a React App to test the DevTools backend implementation, and the DevTools code that requires React is the front end code. We don't have regression tests for jest tests that test the DevTools front end, and for e2e regression tests, we have a separate build folder for DevTools front end (build) and the React app (build-regression)

This script is in charge of downloading the npm packages and either keeping them in build-regression (for e2e tests)or replacing thebuild` folder (in jest backend DevTools tests).

Do you have a different suggestion?

[bvaughn]

This script is in charge of downloading the npm packages and either keeping them in build-regression (for e2e tests)or replacing thebuild` folder (in jest backend DevTools tests).

Ah, okay. I see. Sorry it's a little hard to context switch between this project and Replay sometimes because the test situation is very different.

What you're saying makes sense. It's a little unfortunate, since it limits the types of things we're able to test, but I guess we can still test the store and the inspect element logic which is for the most part the only things that change between versions anyway. And it's way better coverage than the nothing we had before.

My main concern was that this would build all of DevTools (frontend and backend) using the older version of React that we just downloaded, so the frontend would be broken if we ever did want to use it (and maybe it wouldn't be obvious to someone who wasn't familiar with this part of the code bootstrapping):

- run: ./scripts/circleci/download_devtools_regression_build.js << parameters.version >> --replaceBuild
- run: node ./scripts/jest/jest-cli.js --build --project devtools --release-channel=experimental --reactVersion << parameters.version >> --ci

Do you have a different suggestion?

No, probably not. I had initially imagined that we could leave the build-regression folder as a separate one and run the React code that we were testing from it, but given how the pragmas work per-test-case and the imports are more global for the file, I'm not sure how that would work. I guess it wouldn't because of the JSX transform.

[lunaruan]

What you're saying makes sense. It's a little unfortunate, since it limits the types of things we're able to test, but I guess we can still test the store and the inspect element logic which is for the most part the only things that change between versions anyway. And it's way better coverage than the nothing we had before.

Yeah, and we have e2e tests that we want to expand on to hopefully test the rest.

My main concern was that this would build all of DevTools (frontend and backend) using the older version of React that we just downloaded, so the frontend would be broken if we ever did want to use it (and maybe it wouldn't be obvious to someone who wasn't familiar with this part of the code bootstrapping):

Yeah that makes sense. I was imagining this only being used in the regression tests though (it doesn't actually create a build folder, just replace it in regression tests), so I think it's probably OK. Also open to copying the build folder, running the regression tests, and then replacing them if you think that's a better way to do it?

[bvaughn]

Also open to copying the build folder, running the regression tests, and then replacing them if you think that's a better way to do it?

Might be a nice change to have in a finally block or something (so if you ran this locally, your "build" directory would be left in a reasonable state after). Doesn't seem super pressing though.