From b77ecb14237d87f8c2beb859d44252cccdb5044c Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 24 Jun 2024 10:48:42 -0400 Subject: [PATCH 1/3] Clean up disableIEWorkarounds flag --- .../src/client/ReactDOMComponent.js | 25 +----- .../src/client/setInnerHTML.js | 82 ------------------- .../__tests__/dangerouslySetInnerHTML-test.js | 2 - .../__tests__/trustedTypes-test.internal.js | 1 - packages/shared/ReactFeatureFlags.js | 4 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - scripts/jest/setupTests.www.js | 1 - 12 files changed, 3 insertions(+), 118 deletions(-) delete mode 100644 packages/react-dom-bindings/src/client/setInnerHTML.js diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 160546d9b78..5c3ed4e6dd2 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -50,7 +50,6 @@ import { } from './ReactDOMTextarea'; import {validateTextNesting} from './validateDOMNesting'; import {track} from './inputValueTracking'; -import setInnerHTML from './setInnerHTML'; import setTextContent from './setTextContent'; import { createDangerousStringForStyles, @@ -66,7 +65,6 @@ import {validateProperties as validateUnknownProperties} from '../shared/ReactDO import sanitizeURL from '../shared/sanitizeURL'; import { - disableIEWorkarounds, enableTrustedTypesIntegration, enableFilterEmptyStringAttributesDOM, } from 'shared/ReactFeatureFlags'; @@ -86,16 +84,7 @@ let didWarnPopoverTargetObject = false; let canDiffStyleForHydrationWarning; if (__DEV__) { didWarnForNewBooleanPropsWithEmptyValue = {}; - // IE 11 parses & normalizes the style attribute as opposed to other - // browsers. It adds spaces and sorts the properties in some - // non-alphabetical order. Handling that would require sorting CSS - // properties in the client & server versions or applying - // `expectedStyle` to a temporary DOM node to read its `style` attribute - // normalized. Since it only affects IE, we're skipping style warnings - // in that browser completely in favor of doing all that work. - // See https://github.com/facebook/react/issues/11807 - canDiffStyleForHydrationWarning = - disableIEWorkarounds || (canUseDOM && !document.documentMode); + canDiffStyleForHydrationWarning = canUseDOM && !document.documentMode; } function validatePropertiesInDevelopment(type: string, props: any) { @@ -579,11 +568,7 @@ function setProp( 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', ); } - if (disableIEWorkarounds) { - domElement.innerHTML = nextHtml; - } else { - setInnerHTML(domElement, nextHtml); - } + domElement.innerHTML = nextHtml; } } break; @@ -939,11 +924,7 @@ function setPropOnCustomElement( 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', ); } - if (disableIEWorkarounds) { - domElement.innerHTML = nextHtml; - } else { - setInnerHTML(domElement, nextHtml); - } + domElement.innerHTML = nextHtml; } } break; diff --git a/packages/react-dom-bindings/src/client/setInnerHTML.js b/packages/react-dom-bindings/src/client/setInnerHTML.js deleted file mode 100644 index 5f0d630fd38..00000000000 --- a/packages/react-dom-bindings/src/client/setInnerHTML.js +++ /dev/null @@ -1,82 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -/* globals MSApp */ - -import {SVG_NAMESPACE} from './DOMNamespaces'; -import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; - -// SVG temp container for IE lacking innerHTML -let reusableSVGContainer: HTMLElement; - -function setInnerHTMLImpl( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, -): void { - if (node.namespaceURI === SVG_NAMESPACE) { - if (__DEV__) { - if (enableTrustedTypesIntegration) { - // TODO: reconsider the text of this warning and when it should show - // before enabling the feature flag. - if (typeof trustedTypes !== 'undefined') { - console.error( - "Using 'dangerouslySetInnerHTML' in an svg element with " + - 'Trusted Types enabled in an Internet Explorer will cause ' + - 'the trusted value to be converted to string. Assigning string ' + - "to 'innerHTML' will throw an error if Trusted Types are enforced. " + - "You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " + - 'on the enclosing div instead.', - ); - } - } - } - if (!('innerHTML' in node)) { - // IE does not have innerHTML for SVG nodes, so instead we inject the - // new markup in a temp node and then move the child nodes across into - // the target node - reusableSVGContainer = - reusableSVGContainer || document.createElement('div'); - reusableSVGContainer.innerHTML = - '' + html.valueOf().toString() + ''; - const svgNode = reusableSVGContainer.firstChild; - while (node.firstChild) { - node.removeChild(node.firstChild); - } - // $FlowFixMe[incompatible-use] - // $FlowFixMe[incompatible-type] - while (svgNode.firstChild) { - node.appendChild(svgNode.firstChild); - } - return; - } - } - node.innerHTML = (html: any); -} - -let setInnerHTML: ( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, -) => void = setInnerHTMLImpl; -// $FlowFixMe[cannot-resolve-name] -if (typeof MSApp !== 'undefined' && MSApp.execUnsafeLocalFunction) { - /** - * Create a function which has 'unsafe' privileges (required by windows8 apps) - */ - setInnerHTML = function ( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, - ): void { - // $FlowFixMe[cannot-resolve-name] - return MSApp.execUnsafeLocalFunction(function () { - return setInnerHTMLImpl(node, html); - }); - }; -} - -export default setInnerHTML; diff --git a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js index 64b4c0b0f7c..5c5286ec8f7 100644 --- a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js +++ b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js @@ -59,7 +59,6 @@ describe('dangerouslySetInnerHTML', () => { ); }); - // @gate !disableIEWorkarounds it('sets innerHTML on it', async () => { const html = ''; const container = document.createElementNS( @@ -74,7 +73,6 @@ describe('dangerouslySetInnerHTML', () => { expect(circle.tagName).toBe('circle'); }); - // @gate !disableIEWorkarounds it('clears previous children', async () => { const firstHtml = ''; const secondHtml = ''; diff --git a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js index abe707badd5..ac8a158de22 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -232,7 +232,6 @@ describe('when Trusted Types are available in global object', () => { ); }); - // @gate !disableIEWorkarounds it('should log a warning', async () => { class Component extends React.Component { render() { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8b2d0800cb9..ac02caf4290 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -159,10 +159,6 @@ export const disableLegacyContext = true; // TODO: clean up legacy once tests pass WWW. export const useModernStrictMode = true; -// Not ready to break experimental yet. -// Remove IE and MsApp specific workarounds for innerHTML -export const disableIEWorkarounds = true; - // Filter certain DOM attributes (e.g. src, href) if their values are empty // strings. This prevents e.g. from making an unnecessary HTTP // request for certain browsers. diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index a9e2b146e1b..9dddf03893d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -35,7 +35,6 @@ export const allowConcurrentByDefault = false; export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 88e49d2bc94..a7a35163649 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -25,7 +25,6 @@ export const consoleManagedByDevToolsDuringStrictMode = true; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = true; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index e40351ae1fc..87b84463bd5 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -27,7 +27,6 @@ export const enableTaint = true; export const enablePostpone = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; -export const disableIEWorkarounds = true; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index ec4a96f9c86..208be076b51 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -17,7 +17,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = false; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 2bd4e079891..90e07d7de8e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -27,7 +27,6 @@ export const enableTaint = true; export const enablePostpone = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; -export const disableIEWorkarounds = true; export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 9404b877f53..3cdcddfbb0a 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -50,7 +50,6 @@ export const enableUpdaterTracking = __PROFILE__; export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallbackFizz = false; -export const disableIEWorkarounds = true; export const enableCPUSuspense = true; export const enableUseMemoCacheHook = true; export const enableUseEffectEventHook = true; diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index 485f1b80b68..75a78ca1c0f 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -18,7 +18,6 @@ jest.mock('shared/ReactFeatureFlags', () => { // These are hardcoded to true for the next release, // but still run the tests against both variants until // we remove the flag. - actual.disableIEWorkarounds = __VARIANT__; actual.disableClientCache = __VARIANT__; return actual; From d694e15b7bc1572b026c92abb9b5c3f12a6a6c3e Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 24 Jun 2024 11:24:52 -0400 Subject: [PATCH 2/3] Clean up IE behavior test in trustedTypes-test --- .../__tests__/trustedTypes-test.internal.js | 49 ------------------- 1 file changed, 49 deletions(-) diff --git a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js index ac8a158de22..923ee1f5d81 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -206,55 +206,6 @@ describe('when Trusted Types are available in global object', () => { } }); - describe('dangerouslySetInnerHTML in svg elements in Internet Explorer', () => { - let innerHTMLDescriptor; - - // simulate svg elements in Internet Explorer which don't have 'innerHTML' property - beforeEach(() => { - innerHTMLDescriptor = Object.getOwnPropertyDescriptor( - Element.prototype, - 'innerHTML', - ); - delete Element.prototype.innerHTML; - Object.defineProperty( - HTMLDivElement.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - afterEach(() => { - delete HTMLDivElement.prototype.innerHTML; - Object.defineProperty( - Element.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - it('should log a warning', async () => { - class Component extends React.Component { - render() { - return ; - } - } - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - "Using 'dangerouslySetInnerHTML' in an svg element with " + - 'Trusted Types enabled in an Internet Explorer will cause ' + - 'the trusted value to be converted to string. Assigning string ' + - "to 'innerHTML' will throw an error if Trusted Types are enforced. " + - "You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " + - 'on the enclosing div instead.', - ); - expect(container.innerHTML).toBe('unsafe html'); - }); - }); - it('should warn once when rendering script tag in jsx on client', async () => { const root = ReactDOMClient.createRoot(container); await expect(async () => { From a2abe68488881fa38cee88cdf0915e52ab0156c9 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 24 Jun 2024 11:31:14 -0400 Subject: [PATCH 3/3] Fix dangerouslySetInnterHTML-test --- .../__tests__/dangerouslySetInnerHTML-test.js | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js index 5c5286ec8f7..49d1bae9139 100644 --- a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js +++ b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js @@ -58,40 +58,5 @@ describe('dangerouslySetInnerHTML', () => { innerHTMLDescriptor, ); }); - - it('sets innerHTML on it', async () => { - const html = ''; - const container = document.createElementNS( - 'http://www.w3.org/2000/svg', - 'svg', - ); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - const circle = container.firstChild.firstChild; - expect(circle.tagName).toBe('circle'); - }); - - it('clears previous children', async () => { - const firstHtml = ''; - const secondHtml = ''; - - const container = document.createElementNS( - 'http://www.w3.org/2000/svg', - 'svg', - ); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - const rect = container.firstChild.firstChild; - expect(rect.tagName).toBe('rect'); - await act(() => { - root.render(); - }); - const circle = container.firstChild.firstChild; - expect(circle.tagName).toBe('circle'); - }); }); });