Convert ReactDOMInput-test to createRoot #28084

kassens · 2024-01-25T17:00:11Z

Convert ReactDOMInput-test to createRoot

rickhanlonii

LGTM

rickhanlonii · 2024-01-25T17:04:33Z

packages/react-dom/src/__tests__/ReactDOMInput-test.js

-      ReactDOM.render(<input type="text" value="" />, container);
+  it('should warn for controlled value of "" with missing onChange', async () => {
+    await expect(async () => {
+      await act(() => {


fwiw, i've been using flushSync for error assertions, because if a warning or other error is thrown this will crash jest watch mode

rickhanlonii · 2024-01-25T17:05:58Z

packages/react-dom/src/__tests__/ReactDOMInput-test.js

+    await act(() => {
+      root.render(<input type="checkbox" checked={false} readOnly={true} />);
+    });
+    root.unmount();


Could use a key here instead of unmounting and re-creating the root

rickhanlonii · 2024-01-25T17:06:19Z

packages/react-dom/src/__tests__/ReactDOMInput-test.js

-    ReactDOM.render(<input type="checkbox" />, container);
-    ReactDOM.unmountComponentAtNode(container);
-    ReactDOM.render(<input type="checkbox" checked={undefined} />, container);
+  it('should not warn about missing onChange in uncontrolled inputs', async () => {


Ditto here, could probably use a key.

Since this contains so many changes already, I'm trying to keep the changes minimal. Refactoring some tests could still be done as a PR on top of this.

rickhanlonii · 2024-01-25T17:08:07Z

packages/react-dom/src/__tests__/ReactDOMInput-test.js

+    });
+    await act(() => {
+      root.render(<input type="radio" value={0} onChange={function () {}} />);
+    });


Might be worth including another test where they're in the same act, to confirm batching works. Ditto the test below

rickhanlonii · 2024-01-25T17:08:41Z

packages/react-dom/src/__tests__/ReactDOMInput-test.js

-        container,
+    await expect(async () => {
+      await expect(async () => {
+        await act(() => {


flushSync FTW

Convert ReactDOMInput-test to createRoot

Convert ReactDOMInput-test to createRoot DiffTrain build for commit a7fa58d.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 25, 2024

kassens requested review from rickhanlonii and acdlite January 25, 2024 17:06

rickhanlonii approved these changes Jan 25, 2024

View reviewed changes

Convert ReactDOMInput-test to createRoot

07b255f

kassens force-pushed the pr28084 branch from 6f76465 to 07b255f Compare January 25, 2024 18:44

kassens merged commit a7fa58d into facebook:main Jan 25, 2024
36 checks passed

kassens deleted the pr28084 branch February 1, 2024 16:27

gnoff mentioned this pull request Feb 1, 2024

Update React from 60a927d04 to 2bc7d336a vercel/next.js#61522

Merged

ztanner mentioned this pull request Mar 19, 2024

(backport) Update React from 60a927d04 to 14898b6a9 vercel/next.js#63442

Closed

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024

Convert ReactDOMInput-test to createRoot (facebook#28084)

ebcaebe

Convert ReactDOMInput-test to createRoot

bigfootjon pushed a commit that referenced this pull request Apr 18, 2024

Convert ReactDOMInput-test to createRoot (#28084)

81766df

Convert ReactDOMInput-test to createRoot DiffTrain build for commit a7fa58d.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert ReactDOMInput-test to createRoot #28084

Convert ReactDOMInput-test to createRoot #28084

kassens commented Jan 25, 2024

rickhanlonii left a comment

rickhanlonii Jan 25, 2024

rickhanlonii Jan 25, 2024

rickhanlonii Jan 25, 2024

kassens Jan 25, 2024

rickhanlonii Jan 25, 2024

rickhanlonii Jan 25, 2024

Convert ReactDOMInput-test to createRoot #28084

Convert ReactDOMInput-test to createRoot #28084

Conversation

kassens commented Jan 25, 2024

rickhanlonii left a comment

Choose a reason for hiding this comment

rickhanlonii Jan 25, 2024

Choose a reason for hiding this comment

rickhanlonii Jan 25, 2024

Choose a reason for hiding this comment

rickhanlonii Jan 25, 2024

Choose a reason for hiding this comment

kassens Jan 25, 2024

Choose a reason for hiding this comment

rickhanlonii Jan 25, 2024

Choose a reason for hiding this comment

rickhanlonii Jan 25, 2024

Choose a reason for hiding this comment