From 86c8c8db7938319027132eeea81c9b7e28938f9d Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Mon, 13 Feb 2023 21:45:59 +0100 Subject: [PATCH 01/11] test: Don't retry flushActWork if flushUntilNextPaint threw (#26121) ## Summary Fixes "ReferenceError: You are trying to access a property or method of the Jest environment after it has been torn down." in `ReactIncrementalErrorHandling-test.internal.js` Alternatives: 1. Additional `await act(cb)` call where `cb` makes sure we can flush until next paint without throwing ```js // Ensure test isn't exited with pending work await act(async () => { root.render(); }); ``` 1. Use `toFlushAndThrow` ```diff - let error; - try { - await act(async () => { - root.render(); - }); - } catch (e) { - error = e; - } + root.render(); - expect(error.message).toBe('Oops!'); + expect(Scheduler).toFlushAndThrow('Oops!'); expect(numberOfThrows < 100).toBe(true); ``` But then it still wouldn't make sense to pass `resolve` and `reject` to the next `flushActWork`. Even if the next `flushActWork` would flush until next paint without throwing, we couldn't resolve or reject because we already did reject. ## How did you test this change? - `yarn test --watch packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js` produces no more errors after the test finishes. --- packages/jest-react/src/internalAct.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-react/src/internalAct.js b/packages/jest-react/src/internalAct.js index 5d87891e7109e..da7e9f6108490 100644 --- a/packages/jest-react/src/internalAct.js +++ b/packages/jest-react/src/internalAct.js @@ -128,6 +128,7 @@ function flushActWork(resolve: () => void, reject: (error: any) => void) { Scheduler.unstable_flushUntilNextPaint(); } catch (error) { reject(error); + return; } // If Scheduler yields while there's still work, it's so that we can From fccf3a9fba5fd778c678657c556344b333111cfb Mon Sep 17 00:00:00 2001 From: Mateus Toledo <98076988+mateusmtoledo@users.noreply.github.com> Date: Mon, 13 Feb 2023 17:47:42 -0300 Subject: [PATCH 02/11] Remove redundant test steps (#26161) ## Summary This TODO mentions an issue with JSDOM that [seems to have been resolved](https://github.com/jsdom/jsdom/pull/2996). ## How did you test this change? - Ensured that the `document.activeElement` is no longer `node` after `node.blur` is called. - Verified that the tests still pass. - Looked for [a merged PR that fixes the issue](https://github.com/jsdom/jsdom/pull/2996). --- .../src/__tests__/ReactDOMInput-test.js | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 7fe9bb1728de1..64ee09f09abe1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1880,13 +1880,7 @@ describe('ReactDOMInput', () => { node.focus(); setUntrackedValue.call(node, '2'); dispatchEventOnNode(node, 'input'); - // TODO: it is unclear why blur must be triggered twice, - // manual testing in the fixtures shows that the active element - // is no longer the input, however blur() + a blur event seem to - // be the only way to remove focus in JSDOM node.blur(); - dispatchEventOnNode(node, 'blur'); - dispatchEventOnNode(node, 'focusout'); if (disableInputAttributeSyncing) { expect(node.value).toBe('2'); @@ -1906,13 +1900,7 @@ describe('ReactDOMInput', () => { node.focus(); setUntrackedValue.call(node, 4); dispatchEventOnNode(node, 'input'); - // TODO: it is unclear why blur must be triggered twice, - // manual testing in the fixtures shows that the active element - // is no longer the input, however blur() + a blur event seem to - // be the only way to remove focus in JSDOM node.blur(); - dispatchEventOnNode(node, 'blur'); - dispatchEventOnNode(node, 'focusout'); expect(node.getAttribute('value')).toBe('1'); }); @@ -1926,13 +1914,7 @@ describe('ReactDOMInput', () => { node.focus(); setUntrackedValue.call(node, 4); dispatchEventOnNode(node, 'input'); - // TODO: it is unclear why blur must be triggered twice, - // manual testing in the fixtures shows that the active element - // is no longer the input, however blur() + a blur event seem to - // be the only way to remove focus in JSDOM node.blur(); - dispatchEventOnNode(node, 'blur'); - dispatchEventOnNode(node, 'focusout'); expect(node.getAttribute('value')).toBe('1'); }); From 2f401701921d13ab65e2a2ee1c96caa94f426442 Mon Sep 17 00:00:00 2001 From: BIKI DAS Date: Thu, 16 Feb 2023 15:41:12 +0530 Subject: [PATCH 03/11] Don't recommend deprecated debugger script (#26171) yarn debug-test is now deprecated in React package.json. It has been replaced by yarn test --debug. https://user-images.githubusercontent.com/72331432/219268188-8ff5dd42-da2b-434c-83be-72a9d258ee98.mp4 --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 21ff46dbab299..23fbc65ab5769 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -9,7 +9,7 @@ 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. - 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". + 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). From fbf3bc31585f7142b66eb22770fac90aa9a7e2c1 Mon Sep 17 00:00:00 2001 From: Jonny Burger Date: Thu, 16 Feb 2023 11:12:32 +0100 Subject: [PATCH 04/11] Add `scale` as a unitless property (#25601) ## Summary CSS has a new property called `scale` (`scale: 2` is a shorthand for `transform: scale(2)`). In vanilla JavaScript, we can do the following: ```js document.querySelector('div').scale = 2; ``` which will make the `
` twice as big. So in JavaScript, it is possible to pass a plain number. However, in React, the following does not work currently: ```js
``` because `scale` is not in the list of unitless properties. This PR adds `scale` to the list. ## How did you test this change? I built `react` and `react-dom` from source and copied it into the node_modules of my project and verified that now `
` does indeed work whereas before it did not. --- packages/react-dom-bindings/src/shared/CSSProperty.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dom-bindings/src/shared/CSSProperty.js b/packages/react-dom-bindings/src/shared/CSSProperty.js index 158c533cbbe62..5952aca573fa1 100644 --- a/packages/react-dom-bindings/src/shared/CSSProperty.js +++ b/packages/react-dom-bindings/src/shared/CSSProperty.js @@ -40,6 +40,7 @@ export const isUnitlessNumber = { opacity: true, order: true, orphans: true, + scale: true, tabSize: true, widows: true, zIndex: true, From 189f70e17b8f5e1e81d6a3ae024b086895bbc0f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 16 Feb 2023 11:01:52 -0500 Subject: [PATCH 05/11] Create a bunch of custom webpack vs unbundled node bundles (#26172) We currently have an awkward set up because the server can be used in two ways. Either you can have the server code prebundled using Webpack (what Next.js does in practice) or you can use an unbundled Node.js server (what the reference implementation does). The `/client` part of RSC is actually also available on the server when it's used as a consumer for SSR. This should also be specialized depending on if that server is Node or Edge and if it's bundled or unbundled. Currently we still assume Edge will always be bundled since we don't have an interceptor for modules there. I don't think we'll want to support this many combinations of setups for every bundler but this might be ok for the reference implementation. This PR doesn't actually change anything yet. It just updates the plumbing and the entry points that are built and exposed. In follow ups I'll fork the implementation and add more features. --------- Co-authored-by: dan --- fixtures/flight-browser/index.html | 2 +- .../ReactFlightClientHostConfig.dom-bun.js | 9 ++- ...lightClientHostConfig.dom-edge-webpack.js} | 0 ...FlightClientHostConfig.dom-node-webpack.js | 12 ++++ ... ReactFiberHostConfig.dom-edge-webpack.js} | 0 .../ReactFiberHostConfig.dom-node-webpack.js | 10 ++++ .../client.browser.js | 10 ++++ .../react-server-dom-webpack/client.edge.js | 10 ++++ packages/react-server-dom-webpack/client.js | 2 +- .../react-server-dom-webpack/client.node.js | 10 ++++ .../client.node.unbundled.js | 10 ++++ .../npm/client.browser.js | 7 +++ .../npm/client.edge.js | 7 +++ .../react-server-dom-webpack/npm/client.js | 6 +- .../npm/client.node.js | 7 +++ .../npm/client.node.unbundled.js | 7 +++ .../npm/server.node.unbundled.js | 7 +++ .../react-server-dom-webpack/package.json | 30 +++++++++- .../server.node.unbundled.js | 10 ++++ .../src/ReactFlightWebpackPlugin.js | 2 +- .../src/__tests__/ReactFlightDOM-test.js | 2 +- .../forks/ReactFlightServerConfig.dom-bun.js | 2 +- ...actFlightServerConfig.dom-edge-webpack.js} | 0 ...eactFlightServerConfig.dom-node-webpack.js | 11 ++++ ...actServerFormatConfig.dom-edge-webpack.js} | 0 ...eactServerFormatConfig.dom-node-webpack.js | 10 ++++ ...actServerStreamConfig.dom-edge-webpack.js} | 0 ...eactServerStreamConfig.dom-node-webpack.js | 10 ++++ scripts/rollup/bundles.js | 38 +++++++++++- scripts/shared/inlinedHostConfigs.js | 59 +++++++++++++------ 30 files changed, 257 insertions(+), 33 deletions(-) rename packages/react-client/src/forks/{ReactFlightClientHostConfig.dom-edge.js => ReactFlightClientHostConfig.dom-edge-webpack.js} (100%) create mode 100644 packages/react-client/src/forks/ReactFlightClientHostConfig.dom-node-webpack.js rename packages/react-reconciler/src/forks/{ReactFiberHostConfig.dom-edge.js => ReactFiberHostConfig.dom-edge-webpack.js} (100%) create mode 100644 packages/react-reconciler/src/forks/ReactFiberHostConfig.dom-node-webpack.js create mode 100644 packages/react-server-dom-webpack/client.browser.js create mode 100644 packages/react-server-dom-webpack/client.edge.js create mode 100644 packages/react-server-dom-webpack/client.node.js create mode 100644 packages/react-server-dom-webpack/client.node.unbundled.js create mode 100644 packages/react-server-dom-webpack/npm/client.browser.js create mode 100644 packages/react-server-dom-webpack/npm/client.edge.js create mode 100644 packages/react-server-dom-webpack/npm/client.node.js create mode 100644 packages/react-server-dom-webpack/npm/client.node.unbundled.js create mode 100644 packages/react-server-dom-webpack/npm/server.node.unbundled.js create mode 100644 packages/react-server-dom-webpack/server.node.unbundled.js rename packages/react-server/src/forks/{ReactFlightServerConfig.dom-edge.js => ReactFlightServerConfig.dom-edge-webpack.js} (100%) create mode 100644 packages/react-server/src/forks/ReactFlightServerConfig.dom-node-webpack.js rename packages/react-server/src/forks/{ReactServerFormatConfig.dom-edge.js => ReactServerFormatConfig.dom-edge-webpack.js} (100%) create mode 100644 packages/react-server/src/forks/ReactServerFormatConfig.dom-node-webpack.js rename packages/react-server/src/forks/{ReactServerStreamConfig.dom-edge.js => ReactServerStreamConfig.dom-edge-webpack.js} (100%) create mode 100644 packages/react-server/src/forks/ReactServerStreamConfig.dom-node-webpack.js diff --git a/fixtures/flight-browser/index.html b/fixtures/flight-browser/index.html index 5b16a2809c7a1..76035601d315a 100644 --- a/fixtures/flight-browser/index.html +++ b/fixtures/flight-browser/index.html @@ -20,7 +20,7 @@

Flight Example

- + - + + - - + + + - - - - + + + + + - + +
- + +
- + +
- + + - + + diff --git a/package.json b/package.json index f1b7f922c09f4..961d78345ddbe 100644 --- a/package.json +++ b/package.json @@ -108,8 +108,8 @@ "testRegex": "/scripts/jest/dont-run-jest-directly\\.js$" }, "scripts": { - "build": "node ./scripts/rollup/build.js", - "build-combined": "node ./scripts/rollup/build-all-release-channels.js", + "build": "node ./scripts/rollup/build-all-release-channels.js", + "build-combined": "echo 'build-combined is deprecated. yarn build instead.'", "build-for-devtools": "cross-env RELEASE_CHANNEL=experimental yarn build react/index,react/jsx,react-dom/index,react-dom/unstable_testing,react-dom/test-utils,react-is,react-debug-tools,scheduler,react-test-renderer,react-refresh,react-art --type=NODE && cp -r ./build/node_modules build/oss-experimental/", "build-for-devtools-dev": "yarn build-for-devtools --type=NODE_DEV", "build-for-devtools-prod": "yarn build-for-devtools --type=NODE_PROD", @@ -144,7 +144,7 @@ "publish-prereleases": "node ./scripts/release/publish-using-ci-workflow.js", "download-build": "node ./scripts/release/download-experimental-build.js", "download-build-for-head": "node ./scripts/release/download-experimental-build.js --commit=$(git rev-parse HEAD)", - "download-build-in-codesandbox-ci": "cd scripts/release && yarn install && cd ../../ && yarn download-build-for-head || yarn build-combined --type=node react/index react-dom/index react-dom/src/server react-dom/test-utils scheduler/index react/jsx-runtime react/jsx-dev-runtime", + "download-build-in-codesandbox-ci": "cd scripts/release && yarn install && cd ../../ && yarn download-build-for-head || yarn build --type=node react/index react-dom/index react-dom/src/server react-dom/test-utils scheduler/index react/jsx-runtime react/jsx-dev-runtime", "check-release-dependencies": "node ./scripts/release/check-release-dependencies", "generate-inline-fizz-runtime": "node ./scripts/rollup/generate-inline-fizz-runtime.js" }, diff --git a/scripts/jest/jest-cli.js b/scripts/jest/jest-cli.js index 4aaf59ba728f8..fbd55ab2a5d3f 100644 --- a/scripts/jest/jest-cli.js +++ b/scripts/jest/jest-cli.js @@ -255,7 +255,7 @@ function validateOptions() { const buildDir = path.resolve('./build'); if (!fs.existsSync(buildDir)) { logError( - 'Build directory does not exist, please run `yarn build-combined` or remove the --build option.' + 'Build directory does not exist, please run `yarn build` or remove the --build option.' ); success = false; } else if (Date.now() - fs.statSync(buildDir).mtimeMs > 1000 * 60 * 15) {