From 54f785bc51800556dead12aaedf9594b2f15e836 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 17 Feb 2022 16:44:22 -0500 Subject: [PATCH] Disallow comments as DOM containers for createRoot (#23321) This is an old feature that we no longer support. `hydrateRoot` already throws if you pass a comment node; this change makes `createRoot` throw, too. Still enabled in the Facebook build until we migrate the callers. --- .../src/__tests__/ReactDOMRoot-test.js | 18 ++++++++++++++++++ packages/react-dom/src/client/ReactDOMRoot.js | 12 +++++++++--- packages/shared/ReactFeatureFlags.js | 4 ++++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 4 ++++ 11 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index c13c2805a5ccd..40c9666835036 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -402,4 +402,22 @@ describe('ReactDOMRoot', () => { 'already rendering.', ); }); + + // @gate disableCommentsAsDOMContainers + it('errors if container is a comment node', () => { + // This is an old feature used by www. Disabled in the open source build. + const div = document.createElement('div'); + div.innerHTML = ''; + const commentNode = div.childNodes[0]; + + expect(() => ReactDOM.createRoot(commentNode)).toThrow( + 'createRoot(...): Target container is not a DOM element.', + ); + expect(() => ReactDOM.hydrateRoot(commentNode)).toThrow( + 'hydrateRoot(...): Target container is not a DOM element.', + ); + + // Still works in the legacy API + ReactDOM.render(
, commentNode); + }); }); diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index e10f9e5448176..9665cf07f97e3 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -68,7 +68,10 @@ import { isAlreadyRendering, } from 'react-reconciler/src/ReactFiberReconciler'; import {ConcurrentRoot} from 'react-reconciler/src/ReactRootTags'; -import {allowConcurrentByDefault} from 'shared/ReactFeatureFlags'; +import { + allowConcurrentByDefault, + disableCommentsAsDOMContainers, +} from 'shared/ReactFeatureFlags'; /* global reportError */ const defaultOnRecoverableError = @@ -153,7 +156,7 @@ export function createRoot( container: Container, options?: CreateRootOptions, ): RootType { - if (!isValidContainerLegacy(container)) { + if (!isValidContainer(container)) { throw new Error('createRoot(...): Target container is not a DOM element.'); } @@ -293,7 +296,10 @@ export function isValidContainer(node: any): boolean { node && (node.nodeType === ELEMENT_NODE || node.nodeType === DOCUMENT_NODE || - node.nodeType === DOCUMENT_FRAGMENT_NODE) + node.nodeType === DOCUMENT_FRAGMENT_NODE || + (!disableCommentsAsDOMContainers && + node.nodeType === COMMENT_NODE && + (node: any).nodeValue === ' react-mount-point-unstable ')) ); } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index fe9d77766cdb7..94910e5c81967 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -68,6 +68,10 @@ export const enableSchedulerDebugging = false; // Disable javascript: URL strings in href for XSS protection. export const disableJavaScriptURLs = false; +// Disable support for comment nodes as React DOM containers. Only supported +// by www builds. +export const disableCommentsAsDOMContainers = true; + // Experimental Scope support. export const enableScopeAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 699d9a571ff6d..c7a842532edf8 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -33,6 +33,7 @@ export const enableCache = false; export const enableSchedulerDebugging = false; export const debugRenderPhaseSideEffectsForStrictMode = true; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const warnAboutDeprecatedLifecycles = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 0328a98aef589..81e0fbbbbc62e 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -25,6 +25,7 @@ export const enableSelectiveHydration = false; export const enableLazyElements = false; export const enableCache = false; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b1afbda8ec594..5d132163e615e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -25,6 +25,7 @@ export const enableSelectiveHydration = false; export const enableLazyElements = false; export const enableCache = __EXPERIMENTAL__; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 6c1df25af2425..b800db8ef462e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -25,6 +25,7 @@ export const enableSelectiveHydration = false; export const enableLazyElements = false; export const enableCache = false; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e64d09d5d6697..670fda8182080 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -26,6 +26,7 @@ export const enableLazyElements = false; export const enableCache = true; export const enableSchedulerDebugging = false; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index f0d4a9acc5605..cf8b06b119575 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -25,6 +25,7 @@ export const enableSelectiveHydration = false; export const enableLazyElements = false; export const enableCache = __EXPERIMENTAL__; export const disableJavaScriptURLs = false; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 43ce351301ce5..5f77b27d36919 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -25,6 +25,7 @@ export const enableSelectiveHydration = true; export const enableLazyElements = false; export const enableCache = true; export const disableJavaScriptURLs = true; +export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; export const enableSchedulerDebugging = false; export const enableScopeAPI = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index c0545d2aacf3b..0c67855b3c3cd 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -74,6 +74,10 @@ export const enableCache = true; export const disableJavaScriptURLs = true; +// TODO: www currently relies on this feature. It's disabled in open source. +// Need to remove it. +export const disableCommentsAsDOMContainers = false; + export const disableModulePatternComponents = true; export const enableCreateEventHandleAPI = true;