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

[Fizz] Disallow complex children in <title> elements #24679

Merged
merged 4 commits into from
Jun 7, 2022
+264 −1
Merged

[Fizz] Disallow complex children in <title> elements #24679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
125ec3a
[Fizz] Disallow complex children in <title> elements
gnoff May 27, 2022
1ea51af
fixup dev warning conditional logic
gnoff Jun 7, 2022
1810bd5
lints
gnoff Jun 7, 2022
d97aacc
fix bugs
gnoff Jun 7, 2022
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
191 changes: 191 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4425,4 +4425,195 @@ describe('ReactDOMFizzServer', () => {
);
});
});

describe('title children', () => {
function prepareJSDOMForTitle() {
// Test Environment
const jsdom = new JSDOM('<!DOCTYPE html><html><head>\u0000', {
runScripts: 'dangerously',
});
window = jsdom.window;
document = jsdom.window.document;
container = document.getElementsByTagName('head')[0];
}

// @gate experimental
it('should accept a single string child', async () => {
// a Single string child
function App() {
return <title>hello</title>;
}

prepareJSDOMForTitle();
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(<title>hello</title>);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(<title>hello</title>);
});

// @gate experimental
it('should accept children array of length 1 containing a string', async () => {
// a Single string child
function App() {
return <title>{['hello']}</title>;
}

prepareJSDOMForTitle();
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(<title>hello</title>);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(<title>hello</title>);
});

// @gate experimental
it('should warn in dev when given an array of length 2 or more', async () => {
const originalConsoleError = console.error;
const mockError = jest.fn();
console.error = (...args) => {
if (args.length > 1) {
if (typeof args[1] === 'object') {
mockError(args[0].split('\n')[0]);
return;
}
}
mockError(...args.map(normalizeCodeLocInfo));
};

// a Single string child
function App() {
return <title>{['hello1', 'hello2']}</title>;
}

try {
prepareJSDOMForTitle();

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
if (__DEV__) {
expect(mockError).toHaveBeenCalledWith(
'Warning: A title element received an array with more than 1 element as children. ' +
'In browsers title Elements can only have Text Nodes as children. If ' +
'the children being rendered output more than a single text node in aggregate the browser ' +
'will display markup and comments as text in the title and hydration will likely fail and ' +
'fall back to client rendering%s',
'\n' + ' in title (at **)\n' + ' in App (at **)',
);
} else {
expect(mockError).not.toHaveBeenCalled();
}

expect(getVisibleChildren(container)).toEqual(
<title>{'hello1<!-- -->hello2'}</title>,
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual(
[
gate(flags => flags.enableClientRenderFallbackOnTextMismatch)
? 'Text content does not match server-rendered HTML.'
: null,
'Hydration failed because the initial UI does not match what was rendered on the server.',
'There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.',
].filter(Boolean),
);
expect(getVisibleChildren(container)).toEqual(
<title>{['hello1', 'hello2']}</title>,
);
} finally {
console.error = originalConsoleError;
}
});

// @gate experimental
it('should warn in dev if you pass a React Component as a child to <title>', async () => {
const originalConsoleError = console.error;
const mockError = jest.fn();
console.error = (...args) => {
if (args.length > 1) {
if (typeof args[1] === 'object') {
mockError(args[0].split('\n')[0]);
return;
}
}
mockError(...args.map(normalizeCodeLocInfo));
};

function IndirectTitle() {
return 'hello';
}

function App() {
return (
<title>
<IndirectTitle />
</title>
);
}

try {
prepareJSDOMForTitle();

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
if (__DEV__) {
expect(mockError).toHaveBeenCalledWith(
'Warning: A title element received a React element for children. ' +
'In the browser title Elements can only have Text Nodes as children. If ' +
'the children being rendered output more than a single text node in aggregate the browser ' +
'will display markup and comments as text in the title and hydration will likely fail and ' +
'fall back to client rendering%s',
'\n' + ' in title (at **)\n' + ' in App (at **)',
);
} else {
expect(mockError).not.toHaveBeenCalled();
}

expect(getVisibleChildren(container)).toEqual(<title>hello</title>);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(<title>hello</title>);
} finally {
console.error = originalConsoleError;
}
});
});
});
71 changes: 71 additions & 0 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,75 @@ function pushStartMenuItem(
return null;
}

function pushStartTitle(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
): ReactNodeList {
target.push(startChunkForTag('title'));

let children = null;
for (const propKey in props) {
if (hasOwnProperty.call(props, propKey)) {
const propValue = props[propKey];
if (propValue == null) {
continue;
}
switch (propKey) {
case 'children':
children = propValue;
break;
case 'dangerouslySetInnerHTML':
throw new Error(
'`dangerouslySetInnerHTML` does not make sense on <title>.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, responseState, propKey, propValue);
break;
}
}
}
target.push(endOfStartTag);

if (__DEV__) {
const child =
Array.isArray(children) && children.length < 2
? children[0] || null
: children;
if (Array.isArray(children) && children.length > 1) {
console.error(
'A title element received an array with more than 1 element as children. ' +
'In browsers title Elements can only have Text Nodes as children. If ' +
'the children being rendered output more than a single text node in aggregate the browser ' +
'will display markup and comments as text in the title and hydration will likely fail and ' +
'fall back to client rendering',
);
} else if (child != null && child.$$typeof != null) {
console.error(
'A title element received a React element for children. ' +
'In the browser title Elements can only have Text Nodes as children. If ' +
'the children being rendered output more than a single text node in aggregate the browser ' +
'will display markup and comments as text in the title and hydration will likely fail and ' +
'fall back to client rendering',
);
} else if (
child != null &&
typeof child !== 'string' &&
typeof child !== 'number'
) {
console.error(
'A title element received a value that was not a string or number for children. ' +
'In the browser title Elements can only have Text Nodes as children. If ' +
'the children being rendered output more than a single text node in aggregate the browser ' +
'will display markup and comments as text in the title and hydration will likely fail and ' +
'fall back to client rendering',
);
}
}
return children;
}

function pushStartGenericElement(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
Expand Down Expand Up @@ -1390,6 +1459,8 @@ export function pushStartInstance(
return pushInput(target, props, responseState);
case 'menuitem':
return pushStartMenuItem(target, props, responseState);
case 'title':
return pushStartTitle(target, props, responseState);
// Newline eating tags
case 'listing':
case 'pre': {
Expand Down
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,5 +418,6 @@
"430": "ServerContext can only have a value prop and children. Found: %s",
"431": "React elements are not allowed in ServerContext",
"432": "This Suspense boundary was aborted by the server.",
"433": "useId can only be used while React is rendering"
"433": "useId can only be used while React is rendering",
"434": "`dangerouslySetInnerHTML` does not make sense on <title>."
}