-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Add test selectors to experimental build #22760
Conversation
Comparing: 4ff5f57...a4d4e2f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
32622ae
to
50f6211
Compare
@@ -109,7 +109,7 @@ | |||
"scripts": { | |||
"build": "node ./scripts/rollup/build.js", | |||
"build-combined": "node ./scripts/rollup/build-all-release-channels.js", | |||
"build-for-devtools": "cross-env RELEASE_CHANNEL=experimental yarn build react/index,react/jsx,react-dom,react-is,react-debug-tools,scheduler,react-test-renderer,react-refresh --type=NODE && cp -r ./build/node_modules build/oss-experimental/", | |||
"build-for-devtools": "cross-env RELEASE_CHANNEL=experimental yarn build react/index,react/jsx,react-dom/index,react-dom/test,react-is,react-debug-tools,scheduler,react-test-renderer,react-refresh --type=NODE && cp -r ./build/node_modules build/oss-experimental/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes yarn build-for-devtools
build a tiny bit faster, since we don't need e.g. server rendering stuff.
@@ -55,7 +55,7 @@ const config = { | |||
react: resolve(builtModulesDir, 'react'), | |||
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'), | |||
'react-devtools-feature-flags': resolveFeatureFlags('shell'), | |||
'react-dom': resolve(builtModulesDir, 'react-dom'), | |||
'react-dom': resolve(builtModulesDir, 'react-dom/testing'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sure we're building the test shell with the version of react-dom that has the extra test selector APIs.
findBoundingRects, | ||
focusWithin, | ||
observeVisibleRects, | ||
} from 'react-reconciler/src/ReactFiberReconciler'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change adds the new APIs to the experimental NPM release.
@@ -23,7 +23,7 @@ export const enableUpdaterTracking = false; | |||
export const enableSuspenseServerRenderer = false; | |||
export const enableSelectiveHydration = false; | |||
export const enableLazyElements = false; | |||
export const enableCache = false; | |||
export const enableCache = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should have turned this on earlier but didn't matter before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be set to __EXPERIMENTAL__
?
@@ -249,7 +249,7 @@ const bundles = [ | |||
/******* React DOM - www - Testing *******/ | |||
{ | |||
moduleType: RENDERER, | |||
bundleTypes: [FB_WWW_DEV, FB_WWW_PROD], | |||
bundleTypes: [FB_WWW_DEV, FB_WWW_PROD, NODE_DEV, NODE_PROD], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds the react-dom/testing
entry point to the NPM build. I wish we could only do this for the __EXPERIMENTAL__
release channel but I don't think that's possible with our current Rollup configuration?
As it stands, this would add a /testing.js
entrypoint to the react-dom
stable release but its exports would exactly match react-dom/index
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename to unstable_testing
in this case? We shouldn't ship it in a way that seems usable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion.
Little weird that source paths/tests will refer to react-dom/testing
but built stuff will need to refer to react-dom/unstable_testing
but it's still an improvement.
50f6211
to
2b840f2
Compare
2b840f2
to
cd2f004
Compare
cd2f004
to
a4d4e2f
Compare
@@ -23,7 +23,7 @@ export const enableUpdaterTracking = false; | |||
export const enableSuspenseServerRenderer = false; | |||
export const enableSelectiveHydration = false; | |||
export const enableLazyElements = false; | |||
export const enableCache = false; | |||
export const enableCache = __EXPERIMENTAL__; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably should have been flipped on before.
Summary: This sync includes the following changes: - **[c1220ebdd](facebook/react@c1220ebdd )**: treat empty string as null ([#22807](facebook/react#22807)) //<salazarm>// - **[09d9b1775](facebook/react@09d9b1775 )**: Update deprecated features in ESLint configuration files. ([#22767](facebook/react#22767)) //<Esteban>// - **[bddbfb86d](facebook/react@bddbfb86d )**: Revert "Fix Node package.json ./ exports deprecation warning ([#22783](facebook/react#22783))" ([#22792](facebook/react#22792)) //<Sebastian Silbermann>// - **[b831aec48](facebook/react@b831aec48 )**: chore(fast-refresh): double check wasMounted ([#22740](facebook/react#22740)) //<anc95>// - **[8edeb787b](facebook/react@8edeb787b )**: Fix Node package.json ./ exports deprecation warning ([#22783](facebook/react#22783)) //<Rin Arakaki>// - **[fdc1d617a](facebook/react@fdc1d617a )**: Flag for client render fallback behavior on hydration mismatch ([#22787](facebook/react#22787)) //<salazarm>// - **[aa19d569b](facebook/react@aa19d569b )**: Add test selectors to experimental build ([#22760](facebook/react#22760)) //<Brian Vaughn>// - **[520ffc77a](facebook/react@520ffc77a )**: Use globalThis if possible for native fetch in browser build ([#22777](facebook/react#22777)) //<Jiachi Liu>// - **[afbc2d08f](facebook/react@afbc2d08f )**: Remove unused react-internal/invariant-args ESLint rule. ([#22778](facebook/react#22778)) //<Esteban>// - **[ca94e2680](facebook/react@ca94e2680 )**: Remove 'packages/shared/invariant.js' ([#22779](facebook/react#22779)) //<Esteban>// - **[83564712b](facebook/react@83564712b )**: Move SuspenseList to experimental channel ([#22765](facebook/react#22765)) //<Andrew Clark>// - **[d4144e6e5](facebook/react@d4144e6e5 )**: fix : grammatical typo for test description ([#22764](facebook/react#22764)) //<Brijesh Prasad>// - **[0b329511b](facebook/react@0b329511b )**: chore: fix comment typo ([#22657](facebook/react#22657)) //<Han Han>// - **[e6f60d2ad](facebook/react@e6f60d2ad )**: fix typos ([#22715](facebook/react#22715)) //<180909>// Changelog: [General][Changed] - React Native sync for revisions c0c71a8...c1220eb jest_e2e[run_all_tests] Reviewed By: yungsters Differential Revision: D32646433 fbshipit-source-id: c534ee7a17141634700c90fc2c7b34bfbe17887a
Summary: This sync includes the following changes: - **[c1220ebdd](facebook/react@c1220ebdd )**: treat empty string as null ([facebook#22807](facebook/react#22807)) //<salazarm>// - **[09d9b1775](facebook/react@09d9b1775 )**: Update deprecated features in ESLint configuration files. ([facebook#22767](facebook/react#22767)) //<Esteban>// - **[bddbfb86d](facebook/react@bddbfb86d )**: Revert "Fix Node package.json ./ exports deprecation warning ([facebook#22783](facebook/react#22783))" ([facebook#22792](facebook/react#22792)) //<Sebastian Silbermann>// - **[b831aec48](facebook/react@b831aec48 )**: chore(fast-refresh): double check wasMounted ([facebook#22740](facebook/react#22740)) //<anc95>// - **[8edeb787b](facebook/react@8edeb787b )**: Fix Node package.json ./ exports deprecation warning ([facebook#22783](facebook/react#22783)) //<Rin Arakaki>// - **[fdc1d617a](facebook/react@fdc1d617a )**: Flag for client render fallback behavior on hydration mismatch ([facebook#22787](facebook/react#22787)) //<salazarm>// - **[aa19d569b](facebook/react@aa19d569b )**: Add test selectors to experimental build ([facebook#22760](facebook/react#22760)) //<Brian Vaughn>// - **[520ffc77a](facebook/react@520ffc77a )**: Use globalThis if possible for native fetch in browser build ([facebook#22777](facebook/react#22777)) //<Jiachi Liu>// - **[afbc2d08f](facebook/react@afbc2d08f )**: Remove unused react-internal/invariant-args ESLint rule. ([facebook#22778](facebook/react#22778)) //<Esteban>// - **[ca94e2680](facebook/react@ca94e2680 )**: Remove 'packages/shared/invariant.js' ([facebook#22779](facebook/react#22779)) //<Esteban>// - **[83564712b](facebook/react@83564712b )**: Move SuspenseList to experimental channel ([facebook#22765](facebook/react#22765)) //<Andrew Clark>// - **[d4144e6e5](facebook/react@d4144e6e5 )**: fix : grammatical typo for test description ([facebook#22764](facebook/react#22764)) //<Brijesh Prasad>// - **[0b329511b](facebook/react@0b329511b )**: chore: fix comment typo ([facebook#22657](facebook/react#22657)) //<Han Han>// - **[e6f60d2ad](facebook/react@e6f60d2ad )**: fix typos ([facebook#22715](facebook/react#22715)) //<180909>// Changelog: [General][Changed] - React Native sync for revisions c0c71a8...c1220eb jest_e2e[run_all_tests] Reviewed By: yungsters Differential Revision: D32646433 fbshipit-source-id: c534ee7a17141634700c90fc2c7b34bfbe17887a
This change adds a new "react-dom/unstable_testing" entry point but I believe its contents will exactly match "react-dom/index" for the stable build. (The experimental build will have the added new selector APIs.)
I'd like to investigate what it might look like to write DevTools e2e tests using these selectors.
This change adds a new
react-dom/unstable_testing
entry point but I believe its contents will exactly matchreact-dom/index
for the stable build. (The experimental build will have the added new selector APIs.) See below on testing/verification of this assumption.If we think it's important to not even include this file, I can look into that– not sure if our current build script supports this functionality at the moment though.
To test the result of this change on the built bundles, I ran:
Then compared
build/oss-experimental/react-dom
andbuild/oss-stable/react-dom
.The experimental build contains the following exports:
And the stable build has the following:
Note to self, after building this...
I played around with this a bit in the test shell like so: