@samitouri / QOS-React-2 / commits / 4996a8fa5c

Remove enableFilterEmptyStringAttributesDOM (#31765)

Base off https://github.com/facebook/react/pull/31764 This has landed everywhere

Ricky committed Dec 13, 2024 at 16:30 UTC 4996a8fa5c5bf9e12e750c46b48f25656fb050cf

12 files changed +209 -249

packages/react-dom-bindings/src/client/ReactDOMComponent.js

+69 -75

@@ -63,10 +63,7 @@ import {validateProperties as validateInputProperties} from '../shared/ReactDOMN

63 import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook';

64 import sanitizeURL from '../shared/sanitizeURL';

66 -import {

67 - enableTrustedTypesIntegration,

68 - enableFilterEmptyStringAttributesDOM,

69 -} from 'shared/ReactFeatureFlags';

66 +import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';

67 import {

68 mediaEventTypes,

69 listenToNonDelegatedEvent,

@@ -400,35 +397,33 @@ function setProp(

397 // fallthrough

398 case 'src':

399 case 'href': {

403 - if (enableFilterEmptyStringAttributesDOM) {

404 - if (

405 - value === '' &&

406 - // <a href=""> is fine for "reload" links.

407 - !(tag === 'a' && key === 'href')

408 - ) {

409 - if (__DEV__) {

410 - if (key === 'src') {

411 - console.error(

412 - 'An empty string ("") was passed to the %s attribute. ' +

413 - 'This may cause the browser to download the whole page again over the network. ' +

414 - 'To fix this, either do not render the element at all ' +

415 - 'or pass null to %s instead of an empty string.',

416 - key,

417 - key,

418 - );

419 - } else {

420 - console.error(

421 - 'An empty string ("") was passed to the %s attribute. ' +

422 - 'To fix this, either do not render the element at all ' +

423 - 'or pass null to %s instead of an empty string.',

424 - key,

425 - key,

426 - );

427 - }

400 + if (

401 + value === '' &&

402 + // <a href=""> is fine for "reload" links.

403 + !(tag === 'a' && key === 'href')

404 + ) {

405 + if (__DEV__) {

406 + if (key === 'src') {

407 + console.error(

408 + 'An empty string ("") was passed to the %s attribute. ' +

409 + 'This may cause the browser to download the whole page again over the network. ' +

410 + 'To fix this, either do not render the element at all ' +

411 + 'or pass null to %s instead of an empty string.',

412 + key,

413 + key,

414 + );

415 + } else {

416 + console.error(

417 + 'An empty string ("") was passed to the %s attribute. ' +

418 + 'To fix this, either do not render the element at all ' +

419 + 'or pass null to %s instead of an empty string.',

420 + key,

421 + key,

422 + );

423 }

429 - domElement.removeAttribute(key);

430 - break;

424 }

425 + domElement.removeAttribute(key);

426 + break;

427 }

428 if (

429 value == null ||

@@ -2489,53 +2484,52 @@ function diffHydratedGenericElement(

2484 // fallthrough

2485 case 'src':

2486 case 'href':

2492 - if (enableFilterEmptyStringAttributesDOM) {

2493 - if (

2494 - value === '' &&

2495 - // <a href=""> is fine for "reload" links.

2496 - !(tag === 'a' && propKey === 'href') &&

2497 - !(tag === 'object' && propKey === 'data')

2498 - ) {

2499 - if (__DEV__) {

2500 - if (propKey === 'src') {

2501 - console.error(

2502 - 'An empty string ("") was passed to the %s attribute. ' +

2503 - 'This may cause the browser to download the whole page again over the network. ' +

2504 - 'To fix this, either do not render the element at all ' +

2505 - 'or pass null to %s instead of an empty string.',

2506 - propKey,

2507 - propKey,

2508 - );

2509 - } else {

2510 - console.error(

2511 - 'An empty string ("") was passed to the %s attribute. ' +

2512 - 'To fix this, either do not render the element at all ' +

2513 - 'or pass null to %s instead of an empty string.',

2514 - propKey,

2515 - propKey,

2516 - );

2517 - }

2487 + if (

2488 + value === '' &&

2489 + // <a href=""> is fine for "reload" links.

2490 + !(tag === 'a' && propKey === 'href') &&

2491 + !(tag === 'object' && propKey === 'data')

2492 + ) {

2493 + if (__DEV__) {

2494 + if (propKey === 'src') {

2495 + console.error(

2496 + 'An empty string ("") was passed to the %s attribute. ' +

2497 + 'This may cause the browser to download the whole page again over the network. ' +

2498 + 'To fix this, either do not render the element at all ' +

2499 + 'or pass null to %s instead of an empty string.',

2500 + propKey,

2501 + propKey,

2502 + );

2503 + } else {

2504 + console.error(

2505 + 'An empty string ("") was passed to the %s attribute. ' +

2506 + 'To fix this, either do not render the element at all ' +

2507 + 'or pass null to %s instead of an empty string.',

2508 + propKey,

2509 + propKey,

2510 + );

2511 }

2519 - hydrateSanitizedAttribute(

2520 - domElement,

2521 - propKey,

2522 - propKey,

2523 - null,

2524 - extraAttributes,

2525 - serverDifferences,

2526 - );

2527 - continue;

2512 }

2513 + hydrateSanitizedAttribute(

2514 + domElement,

2515 + propKey,

2516 + propKey,

2517 + null,

2518 + extraAttributes,

2519 + serverDifferences,

2520 + );

2521 + continue;

2522 + } else {

2523 + hydrateSanitizedAttribute(

2524 + domElement,

2525 + propKey,

2526 + propKey,

2527 + value,

2528 + extraAttributes,

2529 + serverDifferences,

2530 + );

2531 + continue;

2532 }

2530 - hydrateSanitizedAttribute(

2531 - domElement,

2532 - propKey,

2533 - propKey,

2534 - value,

2535 - extraAttributes,

2536 - serverDifferences,

2537 - );

2538 - continue;

2533 case 'action':

2534 case 'formAction': {

2535 const serverValue = domElement.getAttribute(propKey);

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

+32 -43

@@ -27,10 +27,7 @@ import {

28 import {Children} from 'react';

30 -import {

31 - enableFilterEmptyStringAttributesDOM,

32 - enableFizzExternalRuntime,

33 -} from 'shared/ReactFeatureFlags';

30 +import {enableFizzExternalRuntime} from 'shared/ReactFeatureFlags';

32 import type {

33 Destination,

@@ -1210,30 +1207,28 @@ function pushAttribute(

1207 }

1208 case 'src':

1209 case 'href': {

1213 - if (enableFilterEmptyStringAttributesDOM) {

1214 - if (value === '') {

1215 - if (__DEV__) {

1216 - if (name === 'src') {

1217 - console.error(

1218 - 'An empty string ("") was passed to the %s attribute. ' +

1219 - 'This may cause the browser to download the whole page again over the network. ' +

1220 - 'To fix this, either do not render the element at all ' +

1221 - 'or pass null to %s instead of an empty string.',

1222 - name,

1223 - name,

1224 - );

1225 - } else {

1226 - console.error(

1227 - 'An empty string ("") was passed to the %s attribute. ' +

1228 - 'To fix this, either do not render the element at all ' +

1229 - 'or pass null to %s instead of an empty string.',

1230 - name,

1231 - name,

1232 - );

1233 - }

1210 + if (value === '') {

1211 + if (__DEV__) {

1212 + if (name === 'src') {

1213 + console.error(

1214 + 'An empty string ("") was passed to the %s attribute. ' +

1215 + 'This may cause the browser to download the whole page again over the network. ' +

1216 + 'To fix this, either do not render the element at all ' +

1217 + 'or pass null to %s instead of an empty string.',

1218 + name,

1219 + name,

1220 + );

1221 + } else {

1222 + console.error(

1223 + 'An empty string ("") was passed to the %s attribute. ' +

1224 + 'To fix this, either do not render the element at all ' +

1225 + 'or pass null to %s instead of an empty string.',

1226 + name,

1227 + name,

1228 + );

1229 }

1235 - return;

1230 }

1231 + return;

1232 }

1233 }

1234 // Fall through to the last case which shouldn't remove empty strings.

@@ -1633,19 +1628,17 @@ function pushStartObject(

1628 checkAttributeStringCoercion(propValue, 'data');

1629 }

1630 const sanitizedValue = sanitizeURL('' + propValue);

1636 - if (enableFilterEmptyStringAttributesDOM) {

1637 - if (sanitizedValue === '') {

1638 - if (__DEV__) {

1639 - console.error(

1640 - 'An empty string ("") was passed to the %s attribute. ' +

1641 - 'To fix this, either do not render the element at all ' +

1642 - 'or pass null to %s instead of an empty string.',

1643 - propKey,

1644 - propKey,

1645 - );

1646 - }

1647 - break;

1631 + if (sanitizedValue === '') {

1632 + if (__DEV__) {

1633 + console.error(

1634 + 'An empty string ("") was passed to the %s attribute. ' +

1635 + 'To fix this, either do not render the element at all ' +

1636 + 'or pass null to %s instead of an empty string.',

1637 + propKey,

1638 + propKey,

1639 + );

1640 }

1641 + break;

1642 }

1643 target.push(

1644 attributeSeparator,

@@ -3615,11 +3608,7 @@ export function pushStartInstance(

3608 // Fast track very common tags

3609 break;

3610 case 'a':

3618 - if (enableFilterEmptyStringAttributesDOM) {

3619 - return pushStartAnchor(target, props);

3620 - } else {

3621 - break;

3622 - }

3611 + return pushStartAnchor(target, props);

3612 case 'g':

3613 case 'p':

3614 case 'li':

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

+103 -105

@@ -587,133 +587,131 @@ describe('ReactDOMComponent', () => {

587 expect(node.hasAttribute('data-foo')).toBe(false);

588 });

589

590 - if (ReactFeatureFlags.enableFilterEmptyStringAttributesDOM) {

591 - it('should not add an empty src attribute', async () => {

592 - const container = document.createElement('div');

593 - const root = ReactDOMClient.createRoot(container);

594 - await expect(async () => {

595 - await act(() => {

596 - root.render(<img src="" />);

597 - });

598 - }).toErrorDev(

599 - 'An empty string ("") was passed to the src attribute. ' +

600 - 'This may cause the browser to download the whole page again over the network. ' +

601 - 'To fix this, either do not render the element at all ' +

602 - 'or pass null to src instead of an empty string.',

603 - );

604 - const node = container.firstChild;

605 - expect(node.hasAttribute('src')).toBe(false);

606 -

590 + it('should not add an empty src attribute', async () => {

591 + const container = document.createElement('div');

592 + const root = ReactDOMClient.createRoot(container);

593 + await expect(async () => {

594 await act(() => {

608 - root.render(<img src="abc" />);

595 + root.render(<img src="" />);

596 });

610 - expect(node.hasAttribute('src')).toBe(true);

597 + }).toErrorDev(

598 + 'An empty string ("") was passed to the src attribute. ' +

599 + 'This may cause the browser to download the whole page again over the network. ' +

600 + 'To fix this, either do not render the element at all ' +

601 + 'or pass null to src instead of an empty string.',

602 + );

603 + const node = container.firstChild;

604 + expect(node.hasAttribute('src')).toBe(false);

605

612 - await expect(async () => {

613 - await act(() => {

614 - root.render(<img src="" />);

615 - });

616 - }).toErrorDev(

617 - 'An empty string ("") was passed to the src attribute. ' +

618 - 'This may cause the browser to download the whole page again over the network. ' +

619 - 'To fix this, either do not render the element at all ' +

620 - 'or pass null to src instead of an empty string.',

621 - );

622 - expect(node.hasAttribute('src')).toBe(false);

606 + await act(() => {

607 + root.render(<img src="abc" />);

608 });

609 + expect(node.hasAttribute('src')).toBe(true);

610

625 - it('should not add an empty href attribute', async () => {

626 - const container = document.createElement('div');

627 - const root = ReactDOMClient.createRoot(container);

628 - await expect(async () => {

629 - await act(() => {

630 - root.render(<link href="" />);

631 - });

632 - }).toErrorDev(

633 - 'An empty string ("") was passed to the href attribute. ' +

634 - 'To fix this, either do not render the element at all ' +

635 - 'or pass null to href instead of an empty string.',

636 - );

637 - const node = container.firstChild;

638 - expect(node.hasAttribute('href')).toBe(false);

611 + await expect(async () => {

612 + await act(() => {

613 + root.render(<img src="" />);

614 + });

615 + }).toErrorDev(

616 + 'An empty string ("") was passed to the src attribute. ' +

617 + 'This may cause the browser to download the whole page again over the network. ' +

618 + 'To fix this, either do not render the element at all ' +

619 + 'or pass null to src instead of an empty string.',

620 + );

621 + expect(node.hasAttribute('src')).toBe(false);

622 + });

623

624 + it('should not add an empty href attribute', async () => {

625 + const container = document.createElement('div');

626 + const root = ReactDOMClient.createRoot(container);

627 + await expect(async () => {

628 await act(() => {

641 - root.render(<link href="abc" />);

629 + root.render(<link href="" />);

630 });

643 - expect(node.hasAttribute('href')).toBe(true);

631 + }).toErrorDev(

632 + 'An empty string ("") was passed to the href attribute. ' +

633 + 'To fix this, either do not render the element at all ' +

634 + 'or pass null to href instead of an empty string.',

635 + );

636 + const node = container.firstChild;

637 + expect(node.hasAttribute('href')).toBe(false);

638

645 - await expect(async () => {

646 - await act(() => {

647 - root.render(<link href="" />);

648 - });

649 - }).toErrorDev(

650 - 'An empty string ("") was passed to the href attribute. ' +

651 - 'To fix this, either do not render the element at all ' +

652 - 'or pass null to href instead of an empty string.',

653 - );

654 - expect(node.hasAttribute('href')).toBe(false);

639 + await act(() => {

640 + root.render(<link href="abc" />);

641 });

642 + expect(node.hasAttribute('href')).toBe(true);

643

657 - it('should allow an empty href attribute on anchors', async () => {

658 - const container = document.createElement('div');

659 - const root = ReactDOMClient.createRoot(container);

644 + await expect(async () => {

645 await act(() => {

661 - root.render(<a href="" />);

646 + root.render(<link href="" />);

647 });

663 - const node = container.firstChild;

664 - expect(node.getAttribute('href')).toBe('');

648 + }).toErrorDev(

649 + 'An empty string ("") was passed to the href attribute. ' +

650 + 'To fix this, either do not render the element at all ' +

651 + 'or pass null to href instead of an empty string.',

652 + );

653 + expect(node.hasAttribute('href')).toBe(false);

654 + });

655 +

656 + it('should allow an empty href attribute on anchors', async () => {

657 + const container = document.createElement('div');

658 + const root = ReactDOMClient.createRoot(container);

659 + await act(() => {

660 + root.render(<a href="" />);

661 });

662 + const node = container.firstChild;

663 + expect(node.getAttribute('href')).toBe('');

664 + });

665

667 - it('should allow an empty action attribute', async () => {

668 - const container = document.createElement('div');

669 - const root = ReactDOMClient.createRoot(container);

670 - await act(() => {

671 - root.render(<form action="" />);

672 - });

673 - const node = container.firstChild;

674 - expect(node.getAttribute('action')).toBe('');

666 + it('should allow an empty action attribute', async () => {

667 + const container = document.createElement('div');

668 + const root = ReactDOMClient.createRoot(container);

669 + await act(() => {

670 + root.render(<form action="" />);

671 + });

672 + const node = container.firstChild;

673 + expect(node.getAttribute('action')).toBe('');

674

676 - await act(() => {

677 - root.render(<form action="abc" />);

678 - });

679 - expect(node.hasAttribute('action')).toBe(true);

675 + await act(() => {

676 + root.render(<form action="abc" />);

677 + });

678 + expect(node.hasAttribute('action')).toBe(true);

679

681 - await act(() => {

682 - root.render(<form action="" />);

683 - });

684 - expect(node.getAttribute('action')).toBe('');

680 + await act(() => {

681 + root.render(<form action="" />);

682 });

683 + expect(node.getAttribute('action')).toBe('');

684 + });

685

687 - it('allows empty string of a formAction to override the default of a parent', async () => {

688 - const container = document.createElement('div');

689 - const root = ReactDOMClient.createRoot(container);

690 - await act(() => {

691 - root.render(

692 - <form action="hello">

693 - <button formAction="" />,

694 - </form>,

695 - );

696 - });

697 - const node = container.firstChild.firstChild;

698 - expect(node.hasAttribute('formaction')).toBe(true);

699 - expect(node.getAttribute('formaction')).toBe('');

686 + it('allows empty string of a formAction to override the default of a parent', async () => {

687 + const container = document.createElement('div');

688 + const root = ReactDOMClient.createRoot(container);

689 + await act(() => {

690 + root.render(

691 + <form action="hello">

692 + <button formAction="" />,

693 + </form>,

694 + );

695 });

696 + const node = container.firstChild.firstChild;

697 + expect(node.hasAttribute('formaction')).toBe(true);

698 + expect(node.getAttribute('formaction')).toBe('');

699 + });

700

702 - it('should not filter attributes for custom elements', async () => {

703 - const container = document.createElement('div');

704 - const root = ReactDOMClient.createRoot(container);

705 - await act(() => {

706 - root.render(

707 - <some-custom-element action="" formAction="" href="" src="" />,

708 - );

709 - });

710 - const node = container.firstChild;

711 - expect(node.hasAttribute('action')).toBe(true);

712 - expect(node.hasAttribute('formAction')).toBe(true);

713 - expect(node.hasAttribute('href')).toBe(true);

714 - expect(node.hasAttribute('src')).toBe(true);

701 + it('should not filter attributes for custom elements', async () => {

702 + const container = document.createElement('div');

703 + const root = ReactDOMClient.createRoot(container);

704 + await act(() => {

705 + root.render(

706 + <some-custom-element action="" formAction="" href="" src="" />,

707 + );

708 });

716 - }

709 + const node = container.firstChild;

710 + expect(node.hasAttribute('action')).toBe(true);

711 + expect(node.hasAttribute('formAction')).toBe(true);

712 + expect(node.hasAttribute('href')).toBe(true);

713 + expect(node.hasAttribute('src')).toBe(true);

714 + });

715

716 it('should apply React-specific aliases to HTML elements', async () => {

717 const container = document.createElement('div');

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

+1 -3

@@ -7492,9 +7492,7 @@ body {

7492 });

7493 }).toErrorDev(

7494 [

7495 - gate(flags => flags.enableFilterEmptyStringAttributesDOM)

7496 - ? 'An empty string ("") was passed to the href attribute. To fix this, either do not render the element at all or pass null to href instead of an empty string.'

7497 - : undefined,

7495 + 'An empty string ("") was passed to the href attribute. To fix this, either do not render the element at all or pass null to href instead of an empty string.',

7496 'React encountered a `<link rel="stylesheet" .../>` with a `precedence` prop and expected the `href` prop to be a non-empty string but ecountered `undefined` instead. If your intent was to have React hoist and deduplciate this stylesheet using the `precedence` prop ensure there is a non-empty string `href` prop as well, otherwise remove the `precedence` prop.',

7497 'React encountered a `<link rel="stylesheet" .../>` with a `precedence` prop and expected the `href` prop to be a non-empty string but ecountered an empty string instead. If your intent was to have React hoist and deduplciate this stylesheet using the `precedence` prop ensure there is a non-empty string `href` prop as well, otherwise remove the `precedence` prop.',

7498 'React encountered a `<link rel="stylesheet" .../>` with a `precedence` prop and `onLoad` and `onError` props. The presence of loading and error handlers indicates an intent to manage the stylesheet loading state from your from your Component code and React will not hoist or deduplicate this stylesheet. If your intent was to have React hoist and deduplciate this stylesheet using the `precedence` prop remove the `onLoad` and `onError` props, otherwise remove the `precedence` prop.',

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

+4 -12

@@ -11,7 +11,6 @@

11 'use strict';

13 const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

14 -const ReactFeatureFlags = require('shared/ReactFeatureFlags');

15 let React;

16 let ReactDOM;

@@ -54,13 +53,8 @@ describe('ReactDOMServerIntegration', () => {

53 });

55 itRenders('empty src on img', async render => {

57 - const e = await render(

58 - <img src="" />,

59 - ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0,

60 - );

61 - expect(e.getAttribute('src')).toBe(

62 - ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '',

63 - );

56 + const e = await render(<img src="" />, 1);

57 + expect(e.getAttribute('src')).toBe(null);

58 });

60 itRenders('empty href on anchor', async render => {

@@ -78,11 +72,9 @@ describe('ReactDOMServerIntegration', () => {

72 // errors during hydration.

73 // So we use a <div> instead.

74 <div href="" />,

81 - ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? 1 : 0,

82 - );

83 - expect(e.getAttribute('href')).toBe(

84 - ReactFeatureFlags.enableFilterEmptyStringAttributesDOM ? null : '',

75 + 1,

76 );

77 + expect(e.getAttribute('href')).toBe(null);

78 });

80 itRenders('no string prop with true value', async render => {

packages/shared/ReactFeatureFlags.js

-5

@@ -201,11 +201,6 @@ export const disableLegacyContextForFunctionComponents = true;

201 // TODO: clean up legacy <StrictMode /> once tests pass WWW.

202 export const useModernStrictMode = true;

203

204 -// Filter certain DOM attributes (e.g. src, href) if their values are empty

205 -// strings. This prevents e.g. <img src=""> from making an unnecessary HTTP

206 -// request for certain browsers.

207 -export const enableFilterEmptyStringAttributesDOM = true;

208 -

204 // Enable the moveBefore() alternative to insertBefore(). This preserves states of moves.