Don't update textarea defaultValue and input checked unnecessarily by sebmarkbage · Pull Request #26580 · react/react

sebmarkbage · 2023-04-10T00:39:45Z

In #26573 I changed it so that textareas get their defaultValue reset if you don't specify one.

However, the way that was implemented, it always set it for any update even if it hasn't changed.

We have a test for that, but that test only works if no properties update at all so that no update was scheduled. This fixes the test so that it updates some unrelated prop.

I also found a test for checked that needed a similar fix.

Interestingly, we don't do this deduping for defaultValue or defaultChecked on inputs and there's no test for that.

react-sizebot · 2023-04-10T00:43:15Z

Comparing: e5146cb...c6e4925

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	+0.01%	164.55 kB	164.58 kB	=	51.70 kB	51.69 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.01%	166.13 kB	166.15 kB	+0.05%	52.17 kB	52.20 kB
facebook-www/ReactDOM-prod.classic.js	+0.01%	553.99 kB	554.05 kB	+0.01%	98.16 kB	98.17 kB
facebook-www/ReactDOM-prod.modern.js	+0.01%	537.83 kB	537.89 kB	+0.01%	95.49 kB	95.51 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against c6e4925

This is in line with value. We had a test for it but it wasn't passing if any other related prop is updated.

sophiebits

seems like if no value is specified, then we still set defaultValue on every rerender? that seems fine to me but then why care about the number of sets at all?

sebmarkbage · 2023-04-10T02:17:37Z

Yea, something is fishy. Maybe it doesn't rerender often in practice so it doesn't reset any selection state or something. I have a diff to avoid the diff in render which might surface that problem more frequently.

…26580) In #26573 I changed it so that textareas get their defaultValue reset if you don't specify one. However, the way that was implemented, it always set it for any update even if it hasn't changed. We have a test for that, but that test only works if no properties update at all so that no update was scheduled. This fixes the test so that it updates some unrelated prop. I also found a test for `checked` that needed a similar fix. Interestingly, we don't do this deduping for `defaultValue` or `defaultChecked` on inputs and there's no test for that. DiffTrain build for [9a9da77](9a9da77)

sophiebits · 2023-04-10T02:21:55Z

Setting .value to the same value isn't meant to affect the selection:

input: https://html.spec.whatwg.org/multipage/input.html#dom-input-value-value
textarea: https://html.spec.whatwg.org/multipage/form-elements.html#dom-textarea-value

defaultValue seems more ambiguous. (https://html.spec.whatwg.org/multipage/input.html#attr-input-value https://html.spec.whatwg.org/multipage/form-elements.html#dom-textarea-defaultvalue)

Summary: This sync includes the following changes: - **[58742c2](react/react@58742c21b )**: Delete unused `eventTimes` Fiber field ([#26599](react/react#26599)) //<Andrew Clark>// - **[0b931f9](react/react@0b931f90e )**: Remove JND delay for non-transition updates ([#26597](react/react#26597)) //<Andrew Clark>// - **[ac43bf6](react/react@ac43bf687 )**: Move validation of text nesting into ReactDOMComponent ([#26594](react/react#26594)) //<Sebastian Markbåge>// - **[ca41adb](react/react@ca41adb8c )**: Diff properties in the commit phase instead of generating an update payload ([#26583](react/react#26583)) //<Sebastian Markbåge>// - **[dd0619b](react/react@dd0619b2e )**: rename $$$hostConfig to $$$config ([#26593](react/react#26593)) //<Josh Story>// - **[b55d319](react/react@b55d31955 )**: Rename HostConfig files to FiberConfig to clarify they are configs fo… ([#26592](react/react#26592)) //<Josh Story>// - **[ffb8eac](react/react@ffb8eaca5 )**: Rename ReactServerFormatConfig to ReactFizzConfig ([#26591](react/react#26591)) //<Josh Story>// - **[f4f873f](react/react@f4f873f62 )**: Implements wiring for Flight to have it's own "HostConfig" ([#26590](react/react#26590)) //<Josh Story>// - **[44db16a](react/react@44db16afc )**: Normalize ReactFlightServerConfig and related files ([#26589](react/react#26589)) //<Josh Story>// - **[fec97ec](react/react@fec97ecbc )**: act: Move didScheduleLegacyUpdate to ensureRootIsScheduled ([#26552](react/react#26552)) //<Andrew Clark>// - **[9a9da77](react/react@9a9da7721 )**: Don't update textarea defaultValue and input checked unnecessarily ([#26580](react/react#26580)) //<Sebastian Markbåge>// - **[e5146cb](react/react@e5146cb52 )**: Refactor some controlled component stuff ([#26573](react/react#26573)) //<Sebastian Markbåge>// - **[657698e](react/react@657698e48 )**: [Tests] `waitForThrow` should diff strings ([#26568](react/react#26568)) //<Josh Story>// - **[85bb7b6](react/react@85bb7b685 )**: Fix: Move `destroy` field to shared instance object ([#26561](react/react#26561)) //<Andrew Clark>// - **[9cfba0f](react/react@9cfba0f6e )**: Clean up discrete event replaying ([#26558](react/react#26558)) //<Sebastian Markbåge>// - **[790ebc9](react/react@790ebc962 )**: Remove no-fallthrough lint suppressions ([#26553](react/react#26553)) //<Sophie Alpert>// - **[c155796](react/react@c15579631 )**: Put common aliases in Map/Set instead of switch over strings ([#26551](react/react#26551)) //<Sebastian Markbåge>// - **[d5fd60f](react/react@d5fd60f7e )**: Remove findInstanceBlockingEvent unused parameters ([#26534](react/react#26534)) //<Mohammad Ghorbani>// - **[eeabb73](react/react@eeabb7312 )**: Refactor DOM Bindings Completely Off of DOMProperty Meta Programming ([#26546](react/react#26546)) //<Sebastian Markbåge>// - **[da94e8b](react/react@da94e8b24 )**: Revert "Cleanup enableSyncDefaultUpdate flag ([#26236](react/react#26236))" ([#26528](react/react#26528)) //<Jan Kassens>// - **[0700dd5](react/react@0700dd50b )**: Implement public instances for text nodes in Fabric ([#26516](react/react#26516)) //<Rubén Norte>// - **[4a1cc2d](react/react@4a1cc2ddd )**: Fix logic around attribute seralization ([#26526](react/react#26526)) //<Josh Story>// - **[7329ea8](react/react@7329ea81c )**: Fix suspense replaying forward refs ([#26535](react/react#26535)) //<Hans Otto Wirtz>// - **[0ae3480](react/react@0ae348018 )**: [Float] Suspend unstyled content for up to 1 minute ([#26532](react/react#26532)) //<Andrew Clark>// - **[8888746](react/react@888874673 )**: Allow transitions to interrupt Suspensey commits ([#26531](react/react#26531)) //<Andrew Clark>// - **[09c8d25](react/react@09c8d2563 )**: Move update scheduling to microtask ([#26512](react/react#26512)) //<Andrew Clark>// - **[8310854](react/react@8310854ce )**: Clean up enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay ([#26521](react/react#26521)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions ca01f35...58742c2 jest_e2e[run_all_tests] bypass-github-export-checks Reviewed By: sammy-SC Differential Revision: D44872333 fbshipit-source-id: 0695e86645955aac7a20afdaf3ed02ad33592f5c

…arily (react#26580)" This reverts commit 9a9da77.

…arily (#26580)" This reverts commit 9a9da77.

…6595, react#26596, react#26627 - Refactor some controlled component stuff (react#26573) - Don't update textarea defaultValue and input checked unnecessarily (react#26580) - Diff properties in the commit phase instead of generating an update payload (react#26583) - Move validation of text nesting into ReactDOMComponent (react#26594) - Remove initOption special case (react#26595) - Use already extracted values instead of reading off props for controlled components (react#26596) - Fix input tracking bug (react#26627)

- Refactor some controlled component stuff (#26573) - Don't update textarea defaultValue and input checked unnecessarily (#26580) - Diff properties in the commit phase instead of generating an update payload (#26583) - Move validation of text nesting into ReactDOMComponent (#26594) - Remove initOption special case (#26595) - Use already extracted values instead of reading off props for controlled components (#26596) - Fix input tracking bug (#26627) DiffTrain build for [ded4a78](ded4a78)

- Refactor some controlled component stuff (#26573) - Don't update textarea defaultValue and input checked unnecessarily (#26580) - Diff properties in the commit phase instead of generating an update payload (#26583) - Move validation of text nesting into ReactDOMComponent (#26594) - Remove initOption special case (#26595) - Use already extracted values instead of reading off props for controlled components (#26596) - Fix input tracking bug (#26627)

…eact#26580) In react#26573 I changed it so that textareas get their defaultValue reset if you don't specify one. However, the way that was implemented, it always set it for any update even if it hasn't changed. We have a test for that, but that test only works if no properties update at all so that no update was scheduled. This fixes the test so that it updates some unrelated prop. I also found a test for `checked` that needed a similar fix. Interestingly, we don't do this deduping for `defaultValue` or `defaultChecked` on inputs and there's no test for that.

…26580) In #26573 I changed it so that textareas get their defaultValue reset if you don't specify one. However, the way that was implemented, it always set it for any update even if it hasn't changed. We have a test for that, but that test only works if no properties update at all so that no update was scheduled. This fixes the test so that it updates some unrelated prop. I also found a test for `checked` that needed a similar fix. Interestingly, we don't do this deduping for `defaultValue` or `defaultChecked` on inputs and there's no test for that. DiffTrain build for commit 9a9da77.

Don't update textarea defaultValue unnecessarily

00572cc

sebmarkbage requested a review from sophiebits April 10, 2023 00:39

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 10, 2023

sebmarkbage changed the title ~~Don't update textarea defaultValue unnecessarily~~ Don't update textarea defaultValue and input checked unnecessarily Apr 10, 2023

Don't set checked if it's already the correct value

c6e4925

This is in line with value. We had a test for it but it wasn't passing if any other related prop is updated.

sebmarkbage force-pushed the textareaupdates branch from 8fd20a2 to c6e4925 Compare April 10, 2023 01:05

sophiebits approved these changes Apr 10, 2023

View reviewed changes

sebmarkbage merged commit 9a9da77 into react:main Apr 10, 2023

kassens added a commit to kassens/react that referenced this pull request Apr 17, 2023

Revert "Don't update textarea defaultValue and input checked unnecess…

c4683a4

…arily (react#26580)" This reverts commit 9a9da77.

kassens added a commit that referenced this pull request Apr 20, 2023

Revert "Don't update textarea defaultValue and input checked unnecess…

ad3318e

…arily (#26580)" This reverts commit 9a9da77.

marksteward mentioned this pull request Dec 10, 2025

[Bug]: Spelling correction in Chrome doesn't work mattermost/mattermost#34666

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Don't update textarea defaultValue and input checked unnecessarily#26580

Don't update textarea defaultValue and input checked unnecessarily#26580
sebmarkbage merged 2 commits into
react:mainfrom
sebmarkbage:textareaupdates

sebmarkbage commented Apr 10, 2023 •

edited

Loading

Uh oh!

react-sizebot commented Apr 10, 2023 •

edited

Loading

Uh oh!

sophiebits left a comment

Uh oh!

sebmarkbage commented Apr 10, 2023

Uh oh!

sophiebits commented Apr 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sebmarkbage commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

react-sizebot commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Critical size changes

Significant size changes

Uh oh!

sophiebits left a comment

Choose a reason for hiding this comment

Uh oh!

sebmarkbage commented Apr 10, 2023

Uh oh!

sophiebits commented Apr 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sebmarkbage commented Apr 10, 2023 •

edited

Loading

react-sizebot commented Apr 10, 2023 •

edited

Loading