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.

Sign up for GitHub

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

Jump to bottom

support title as a singleton #25507

Closed
wants to merge 1 commit into from
Closed

support title as a singleton #25507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ function setInitialDOMProperties(
// show within the <textarea> until it has been focused and blurred again.
// https://github.com/facebook/react/issues/6731#issuecomment-254874553
const canSetTextContent =
(!enableHostSingletons || tag !== 'body') &&
(!enableHostSingletons || (tag !== 'body' && tag !== 'title')) &&
(tag !== 'textarea' || nextProp !== '');
if (canSetTextContent) {
setTextContent(domElement, nextProp);
Expand Down
32 changes: 28 additions & 4 deletions packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ export function removeChildFromContainer(
): void {
if (container.nodeType === COMMENT_NODE) {
(container.parentNode: any).removeChild(child);
} else {
} else if (child.parentNode !== null) {
container.removeChild(child);
}
}
Expand Down Expand Up @@ -726,7 +726,8 @@ export function clearContainer(container: Container): void {
switch (nodeName) {
case 'HTML':
case 'HEAD':
case 'BODY': {
case 'BODY':
case 'TITLE': {
clearContainer((node: any));
// If these singleton instances had previously been rendered with React they
// may still hold on to references to the previous fiber tree. We detatch them
Expand Down Expand Up @@ -923,6 +924,7 @@ function getNextHydratable(node) {
}
break;
}
case 'TITLE':
case 'HTML':
case 'HEAD':
case 'BODY': {
Expand Down Expand Up @@ -970,7 +972,12 @@ function getNextHydratable(node) {
} else if (enableHostSingletons) {
if (nodeType === ELEMENT_NODE) {
const tag: string = (node: any).tagName;
if (tag === 'HTML' || tag === 'HEAD' || tag === 'BODY') {
if (
tag === 'HTML' ||
tag === 'HEAD' ||
tag === 'BODY' ||
tag === 'TITLE'
) {
continue;
}
break;
Expand Down Expand Up @@ -1547,7 +1554,9 @@ export {
export const supportsSingletons = true;

export function isHostSingletonType(type: string): boolean {
return type === 'html' || type === 'head' || type === 'body';
return (
type === 'html' || type === 'head' || type === 'body' || type === 'title'
);
}

export function resolveSingletonInstance(
Expand Down Expand Up @@ -1600,6 +1609,19 @@ export function resolveSingletonInstance(
}
return body;
}
case 'title': {
if (!ownerDocument.head) {
throw new Error(
'React expected a <head> element (document.head) to exist in the Document but one was' +
' not found. React never removes the head for any Document it renders into so' +
' the cause is likely in some other script running on this page.',
);
}
// This will initialize a title element if it didn't already exist
ownerDocument.title = ownerDocument.title;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may be too cute and also, mutates more than necessary. not sure if browsers are smart and noop when not changed. either way I could rewrite to be more conventional about constructing a title if missing. unlike the other singletons we cannot expect the document to always contain one

// We assert the type here because we ensured the title existed above
return (ownerDocument.querySelector('title'): any);
}
default: {
throw new Error(
'resolveSingletonInstance was called with an element type that is not supported. This is a bug in React.',
Expand Down Expand Up @@ -1629,6 +1651,7 @@ export function acquireSingletonInstance(
);
}
switch (type) {
case 'title':
case 'html':
case 'head':
case 'body': {
Expand Down Expand Up @@ -1671,6 +1694,7 @@ export function clearSingleton(instance: Instance): void {
} else if (
nodeName === 'HEAD' ||
nodeName === 'BODY' ||
nodeName === 'TITLE' ||
nodeName === 'STYLE' ||
(nodeName === 'LINK' &&
((node: any): HTMLLinkElement).rel.toLowerCase() === 'stylesheet')
Expand Down
17 changes: 15 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFiber-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ describe('ReactDOMFiber', () => {
// It's an error of type 'NotFoundError' with no message
container.innerHTML = '<div>MEOW.</div>';

expect(() => {
if (gate(flags => flags.enableHostSingletons)) {
expect(() =>
ReactDOM.render(<div key="2">baz</div>, container),
).toErrorDev(
Expand All @@ -1203,7 +1203,20 @@ describe('ReactDOMFiber', () => {
'to empty a container.',
{withoutStack: true},
);
}).toThrowError();
} else {
expect(() => {
expect(() =>
ReactDOM.render(<div key="2">baz</div>, container),
).toErrorDev(
'render(...): ' +
'It looks like the React-rendered content of this container was ' +
'removed without using React. This is not supported and will ' +
'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
'to empty a container.',
{withoutStack: true},
);
}).toThrowError();
}
});

it('should warn when doing an update to a container manually updated outside of React', () => {
Expand Down
28 changes: 16 additions & 12 deletions packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,13 @@ describe('ReactDOM HostSingleton', () => {
);
expect(() => {
expect(Scheduler).toFlushWithoutYielding();
}).toErrorDev(
}).toErrorDev([
'Warning: You are mounting a new head component when a previous one has not first unmounted. It is an error to render more than one head component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <head> and if you need to mount a new one, ensure any previous ones have unmounted first',
);
'Warning: You are mounting a new title component when a previous one has not first unmounted. It is an error to render more than one title component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <title> and if you need to mount a new one, ensure any previous ones have unmounted first',
]);
expect(getVisibleChildren(document)).toEqual(
<html>
<head lang="es" data-foo="foo">
<title>Hello</title>
<title>Hola</title>
</head>
<body />
Expand Down Expand Up @@ -288,9 +288,9 @@ describe('ReactDOM HostSingleton', () => {
<html data-client-foo="foo">
<head>
<link rel="stylesheet" href="resource" />
<title>a client title</title>
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
<title>a client title</title>
</head>
<body data-client-baz="baz">
<style>
Expand Down Expand Up @@ -327,9 +327,9 @@ describe('ReactDOM HostSingleton', () => {
<html data-client-foo="foo">
<head>
<link rel="stylesheet" href="resource" />
<title>a client title</title>
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
<title>a client title</title>
<meta />
</head>
<body data-client-baz="baz">
Expand Down Expand Up @@ -366,9 +366,9 @@ describe('ReactDOM HostSingleton', () => {
<html data-client-foo="foo">
<head>
<link rel="stylesheet" href="resource" />
<title>a client title</title>
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
<title>a client title</title>
</head>
<body data-client-baz="baz">
<style>
Expand Down Expand Up @@ -402,9 +402,9 @@ describe('ReactDOM HostSingleton', () => {
<html data-client-foo="foo">
<head>
<link rel="stylesheet" href="resource" />
<title>a client title</title>
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
<title>a client title</title>
</head>
<body>
<style>
Expand All @@ -429,6 +429,7 @@ describe('ReactDOM HostSingleton', () => {
<html>
<head>
<link rel="stylesheet" href="resource" />
<title />
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
</head>
Expand Down Expand Up @@ -472,7 +473,7 @@ describe('ReactDOM HostSingleton', () => {
expect(Scheduler).toFlushWithoutYielding();
}).toErrorDev(
[
`Warning: Expected server HTML to contain a matching <title> in <head>.
`Warning: Expected server HTML to contain a matching text node for \"a client title\" in <title>.
in title (at **)
in head (at **)
in html (at **)`,
Expand Down Expand Up @@ -503,9 +504,9 @@ describe('ReactDOM HostSingleton', () => {
<html data-client-foo="foo">
<head>
<link rel="stylesheet" href="resource" />
<title>a client title</title>
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
<title>a client title</title>
</head>
<body data-client-baz="baz">
<style>
Expand Down Expand Up @@ -570,6 +571,7 @@ describe('ReactDOM HostSingleton', () => {
<html>
<head>
<link rel="stylesheet" href="resource" />
<title />
<link rel="stylesheet" href="3rdparty" />
<link rel="stylesheet" href="3rdparty2" />
</head>
Expand All @@ -594,7 +596,9 @@ describe('ReactDOM HostSingleton', () => {
]);
expect(getVisibleChildren(document)).toEqual(
<html>
<head />
<head>
<title />
</head>
<body />
</html>,
);
Expand Down Expand Up @@ -769,8 +773,8 @@ describe('ReactDOM HostSingleton', () => {
<html>
<head>
<link rel="stylesheet" href="headbefore" />
<link rel="stylesheet" href="headafter" />
<title>something new</title>
<link rel="stylesheet" href="headafter" />
</head>
<body>
<link rel="stylesheet" href="bodybefore" />
Expand Down Expand Up @@ -805,8 +809,8 @@ describe('ReactDOM HostSingleton', () => {
<html>
<head>
<link rel="stylesheet" href="before" />
<link rel="stylesheet" href="after" />
<title>something new</title>
<link rel="stylesheet" href="after" />
</head>
<body />
</html>,
Expand Down
27 changes: 21 additions & 6 deletions packages/react-dom/src/__tests__/ReactRenderDocument-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,17 @@ describe('rendering React components at document', () => {

const originalDocEl = testDocument.documentElement;
const originalHead = testDocument.head;
const originalTitle = testDocument.head.querySelector('title');
const originalBody = testDocument.body;

// When we unmount everything is removed except the singleton nodes of html, head, and body
// When we unmount everything is removed except the singleton nodes of html, head, body, and title
ReactDOM.unmountComponentAtNode(testDocument);
expect(testDocument.firstChild).toBe(originalDocEl);
expect(testDocument.head).toBe(originalHead);
expect(testDocument.body).toBe(originalBody);
expect(originalBody.firstChild).toEqual(null);
expect(originalHead.firstChild).toEqual(null);
expect(originalHead.firstChild).toBe(originalTitle);
expect(originalTitle.firstChild).toEqual(null);
});

// @gate !enableHostSingletons
Expand Down Expand Up @@ -253,10 +255,23 @@ describe('rendering React components at document', () => {
}
}

// getTestDocument() has an extra <meta> that we didn't render.
expect(() =>
ReactDOM.hydrate(<Component text="Hello world" />, testDocument),
).toErrorDev('Did not expect server HTML to contain a <meta> in <head>.');
if (gate(flags => flags.enableHostSingletons)) {
// with singletons the title hydrates out of band and the extra meta tag is
// treated like a trailing extra node. We still get a hydration warning because
// the title content did not match
expect(() =>
ReactDOM.hydrate(<Component text="Hello world" />, testDocument),
).toErrorDev(
'Text content did not match. Server: "test doc" Client: "Hello World"',
);
} else {
// getTestDocument() has an extra <meta> that we didn't render.
expect(() =>
ReactDOM.hydrate(<Component text="Hello world" />, testDocument),
).toErrorDev(
'Did not expect server HTML to contain a <meta> in <head>.',
);
}
expect(testDocument.body.innerHTML).toBe('Hello world');
});

Expand Down
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1995,7 +1995,11 @@ function commitDeletionEffects(
let parent: null | Fiber = returnFiber;
findParent: while (parent !== null) {
switch (parent.tag) {
case HostSingleton:
case HostSingleton: {
hostParent = parent.stateNode;
hostParentIsContainer = true;
break findParent;
}
case HostComponent: {
hostParent = parent.stateNode;
hostParentIsContainer = false;
Expand Down Expand Up @@ -2080,6 +2084,7 @@ function commitDeletionEffectsOnFiber(
const prevHostParent = hostParent;
const prevHostParentIsContainer = hostParentIsContainer;
hostParent = deletedFiber.stateNode;
hostParentIsContainer = true;
recursivelyTraverseDeletionEffects(
finishedRoot,
nearestMountedAncestor,
Expand Down
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1995,7 +1995,11 @@ function commitDeletionEffects(
let parent: null | Fiber = returnFiber;
findParent: while (parent !== null) {
switch (parent.tag) {
case HostSingleton:
case HostSingleton: {
hostParent = parent.stateNode;
hostParentIsContainer = true;
break findParent;
}
case HostComponent: {
hostParent = parent.stateNode;
hostParentIsContainer = false;
Expand Down Expand Up @@ -2080,6 +2084,7 @@ function commitDeletionEffectsOnFiber(
const prevHostParent = hostParent;
const prevHostParentIsContainer = hostParentIsContainer;
hostParent = deletedFiber.stateNode;
hostParentIsContainer = true;
recursivelyTraverseDeletionEffects(
finishedRoot,
nearestMountedAncestor,
Expand Down