@samitouri / QOS-React-1 Mirror · read-only / commits / db69f95e48
Fork 0

The library for web and native user interfaces.

Mirrored from https://github.com/247TaxiDriver/QOS-React.git

Synced Nov 14, 05:30 UTC

Fix checkbox and radio hydration (#27401)

Fixes whatever part of https://github.com/facebook/react/issues/26876 and https://github.com/vercel/next.js/issues/49499 that https://github.com/facebook/react/pull/27394 didn't fix, probably. From manual tests I believe this behavior brings us back to parity with latest stable release (18.2.0). It's awkward that we keep the user's state even for controlled inputs, so the DOM is out of sync with React state. Previously the .defaultChecked assignment done in updateInput() was changing the actual checkedness because the dirty flag wasn't getting set, meaning that hydrating could change which radio button is checked, even in the absence of user interaction! Now we go back to always detaching again.

Sophie Alpert committed Oct 2, 2023 at 11:38 UTC db69f95e4876ec3c24117f58d55cbb4f315b9fa7
2 files changed +181 -6
packages/react-dom-bindings/src/client/ReactDOMInput.js
+4 -6
@@ -297,12 +297,10 @@ export function initInput(
297 typeof checkedOrDefault !== 'symbol' &&
298 !!checkedOrDefault;
299
300 - // The checked property never gets assigned. It must be manually set.
301 - // We don't want to do this when hydrating so that existing user input isn't
302 - // modified
303 - // TODO: I'm pretty sure this is a bug because initialValueTracking won't be
304 - // correct for the hydration case then.
305 - if (!isHydrating) {
300 + if (isHydrating) {
301 + // Detach .checked from .defaultChecked but leave user input alone
302 + node.checked = node.checked;
303 + } else {
304 node.checked = !!initialChecked;
305 }
306
packages/react-dom/src/__tests__/ReactDOMInput-test.js
+177
@@ -17,7 +17,11 @@ function emptyFunction() {}
17 describe('ReactDOMInput', () => {
18 let React;
19 let ReactDOM;
20 + let ReactDOMClient;
21 let ReactDOMServer;
22 + let Scheduler;
23 + let act;
24 + let assertLog;
25 let setUntrackedValue;
26 let setUntrackedChecked;
27 let container;
@@ -87,7 +91,11 @@ describe('ReactDOMInput', () => {
91
92 React = require('react');
93 ReactDOM = require('react-dom');
94 + ReactDOMClient = require('react-dom/client');
95 ReactDOMServer = require('react-dom/server');
96 + Scheduler = require('scheduler');
97 + act = require('internal-test-utils').act;
98 + assertLog = require('internal-test-utils').assertLog;
99
100 container = document.createElement('div');
101 document.body.appendChild(container);
@@ -1235,6 +1243,175 @@ describe('ReactDOMInput', () => {
1243 assertInputTrackingIsCurrent(container);
1244 });
1245
1246 + it('should hydrate controlled radio buttons', async () => {
1247 + function App() {
1248 + const [current, setCurrent] = React.useState('a');
1249 + return (
1250 + <>
1251 + <input
1252 + type="radio"
1253 + name="fruit"
1254 + checked={current === 'a'}
1255 + onChange={() => {
1256 + Scheduler.log('click a');
1257 + setCurrent('a');
1258 + }}
1259 + />
1260 + <input
1261 + type="radio"
1262 + name="fruit"
1263 + checked={current === 'b'}
1264 + onChange={() => {
1265 + Scheduler.log('click b');
1266 + setCurrent('b');
1267 + }}
1268 + />
1269 + <input
1270 + type="radio"
1271 + name="fruit"
1272 + checked={current === 'c'}
1273 + onChange={() => {
1274 + Scheduler.log('click c');
1275 + // Let's say the user can't pick C
1276 + }}
1277 + />
1278 + </>
1279 + );
1280 + }
1281 + const html = ReactDOMServer.renderToString(<App />);
1282 + container.innerHTML = html;
1283 + const [a, b, c] = container.querySelectorAll('input');
1284 + expect(a.checked).toBe(true);
1285 + expect(b.checked).toBe(false);
1286 + expect(c.checked).toBe(false);
1287 + expect(isCheckedDirty(a)).toBe(false);
1288 + expect(isCheckedDirty(b)).toBe(false);
1289 + expect(isCheckedDirty(c)).toBe(false);
1290 +
1291 + // Click on B before hydrating
1292 + b.checked = true;
1293 + expect(isCheckedDirty(a)).toBe(true);
1294 + expect(isCheckedDirty(b)).toBe(true);
1295 + expect(isCheckedDirty(c)).toBe(false);
1296 +
1297 + await act(async () => {
1298 + ReactDOMClient.hydrateRoot(container, <App />);
1299 + });
1300 +
1301 + // Currently, we don't fire onChange when hydrating
1302 + assertLog([]);
1303 + // Strangely, we leave `b` checked even though we rendered A with
1304 + // checked={true} and B with checked={false}. Arguably this is a bug.
1305 + expect(a.checked).toBe(false);
1306 + expect(b.checked).toBe(true);
1307 + expect(c.checked).toBe(false);
1308 + expect(isCheckedDirty(a)).toBe(true);
1309 + expect(isCheckedDirty(b)).toBe(true);
1310 + expect(isCheckedDirty(c)).toBe(true);
1311 + assertInputTrackingIsCurrent(container);
1312 +
1313 + // If we click on C now though...
1314 + await act(async () => {
1315 + setUntrackedChecked.call(c, true);
1316 + dispatchEventOnNode(c, 'click');
1317 + });
1318 +
1319 + // then since C's onClick doesn't set state, A becomes rechecked.
1320 + assertLog(['click c']);
1321 + expect(a.checked).toBe(true);
1322 + expect(b.checked).toBe(false);
1323 + expect(c.checked).toBe(false);
1324 + expect(isCheckedDirty(a)).toBe(true);
1325 + expect(isCheckedDirty(b)).toBe(true);
1326 + expect(isCheckedDirty(c)).toBe(true);
1327 + assertInputTrackingIsCurrent(container);
1328 +
1329 + // And we can also change to B properly after hydration.
1330 + await act(async () => {
1331 + setUntrackedChecked.call(b, true);
1332 + dispatchEventOnNode(b, 'click');
1333 + });
1334 + assertLog(['click b']);
1335 + expect(a.checked).toBe(false);
1336 + expect(b.checked).toBe(true);
1337 + expect(c.checked).toBe(false);
1338 + expect(isCheckedDirty(a)).toBe(true);
1339 + expect(isCheckedDirty(b)).toBe(true);
1340 + expect(isCheckedDirty(c)).toBe(true);
1341 + assertInputTrackingIsCurrent(container);
1342 + });
1343 +
1344 + it('should hydrate uncontrolled radio buttons', async () => {
1345 + function App() {
1346 + return (
1347 + <>
1348 + <input
1349 + type="radio"
1350 + name="fruit"
1351 + defaultChecked={true}
1352 + onChange={() => Scheduler.log('click a')}
1353 + />
1354 + <input
1355 + type="radio"
1356 + name="fruit"
1357 + defaultChecked={false}
1358 + onChange={() => Scheduler.log('click b')}
1359 + />
1360 + <input
1361 + type="radio"
1362 + name="fruit"
1363 + defaultChecked={false}
1364 + onChange={() => Scheduler.log('click c')}
1365 + />
1366 + </>
1367 + );
1368 + }
1369 + const html = ReactDOMServer.renderToString(<App />);
1370 + container.innerHTML = html;
1371 + const [a, b, c] = container.querySelectorAll('input');
1372 + expect(a.checked).toBe(true);
1373 + expect(b.checked).toBe(false);
1374 + expect(c.checked).toBe(false);
1375 + expect(isCheckedDirty(a)).toBe(false);
1376 + expect(isCheckedDirty(b)).toBe(false);
1377 + expect(isCheckedDirty(c)).toBe(false);
1378 +
1379 + // Click on B before hydrating
1380 + b.checked = true;
1381 + expect(isCheckedDirty(a)).toBe(true);
1382 + expect(isCheckedDirty(b)).toBe(true);
1383 + expect(isCheckedDirty(c)).toBe(false);
1384 +
1385 + await act(async () => {
1386 + ReactDOMClient.hydrateRoot(container, <App />);
1387 + });
1388 +
1389 + // Currently, we don't fire onChange when hydrating
1390 + assertLog([]);
1391 + expect(a.checked).toBe(false);
1392 + expect(b.checked).toBe(true);
1393 + expect(c.checked).toBe(false);
1394 + expect(isCheckedDirty(a)).toBe(true);
1395 + expect(isCheckedDirty(b)).toBe(true);
1396 + expect(isCheckedDirty(c)).toBe(true);
1397 + assertInputTrackingIsCurrent(container);
1398 +
1399 + // Click back to A
1400 + await act(async () => {
1401 + setUntrackedChecked.call(a, true);
1402 + dispatchEventOnNode(a, 'click');
1403 + });
1404 +
1405 + assertLog(['click a']);
1406 + expect(a.checked).toBe(true);
1407 + expect(b.checked).toBe(false);
1408 + expect(c.checked).toBe(false);
1409 + expect(isCheckedDirty(a)).toBe(true);
1410 + expect(isCheckedDirty(b)).toBe(true);
1411 + expect(isCheckedDirty(c)).toBe(true);
1412 + assertInputTrackingIsCurrent(container);
1413 + });
1414 +
1415 it('should check the correct radio when the selected name moves', () => {
1416 class App extends React.Component {
1417 state = {