Skip to content

Commit

Permalink
Disallow comments as DOM containers for createRoot (#23321)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite authored Feb 17, 2022
1 parent e9aa959 commit 54f785b
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 3 deletions.
18 changes: 18 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '<!-- react-mount-point-unstable -->';
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(<div />, commentNode);
});
});
12 changes: 9 additions & 3 deletions packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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.');
}

Expand Down Expand Up @@ -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 '))
);
}

Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 54f785b

Please sign in to comment.