Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow comments as DOM containers for createRoot #23321

Merged
merged 1 commit into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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