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

Modify Babel React JSX Duplicate Children Fix #17101

Merged
merged 2 commits into from
Oct 16, 2019
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
Original file line number Diff line number Diff line change
Expand Up @@ -481,9 +481,24 @@ describe('transform react to jsx', () => {
).toMatchSnapshot();
});

it('should not contain duplicate children key in props object', () => {
it('duplicate children prop should transform into sequence expression with actual children', () => {
expect(
transform(`<Component children={1}>2</Component>`)
).toMatchSnapshot();
});
it('duplicate children prop should transform into sequence expression with next prop', () => {
expect(
transform(`<Component children={1} foo={3}>2</Component>`)
).toMatchSnapshot();
});
it('duplicate children props should transform into sequence expression with next prop', () => {
expect(
transform(`<Component children={1} children={4} foo={3}>2</Component>`)
).toMatchSnapshot();
});
it('duplicate children prop should transform into sequence expression with spread', () => {
expect(
transform(`<Component children={1} {...x}>2</Component>`)
).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,32 @@ var x = React.jsxs("div", {
});
`;

exports[`transform react to jsx duplicate children prop should transform into sequence expression with actual children 1`] = `
React.jsx(Component, {
children: (1, "2")
});
`;

exports[`transform react to jsx duplicate children prop should transform into sequence expression with next prop 1`] = `
React.jsx(Component, {
foo: (1, 3),
children: "2"
});
`;

exports[`transform react to jsx duplicate children prop should transform into sequence expression with spread 1`] = `
React.jsx(Component, Object.assign({}, (1, x), {
children: "2"
}));
`;

exports[`transform react to jsx duplicate children props should transform into sequence expression with next prop 1`] = `
React.jsx(Component, {
foo: (1, 4, 3),
children: "2"
});
`;

exports[`transform react to jsx fragment with no children 1`] = `var x = React.jsx(React.Fragment, {});`;

exports[`transform react to jsx fragments 1`] = `
Expand Down Expand Up @@ -250,14 +276,6 @@ var e = React.jsx(F, {
});
`;

exports[`transform react to jsx should not contain duplicate children key in props object 1`] = `
React.jsx(Component, Object.assign({
children: 1
}, {
children: "2"
}));
`;

exports[`transform react to jsx should not strip nbsp even couple with other whitespace 1`] = `
React.jsx("div", {
children: "\\xA0 "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
}
}

function convertAttribute(node) {
const value = convertAttributeValue(node.value || t.booleanLiteral(true));
function convertAttribute(node, duplicateChildren) {
let value = convertAttributeValue(node.value || t.booleanLiteral(true));

if (t.isStringLiteral(value) && !t.isJSXExpressionContainer(node.value)) {
value.value = value.value.replace(/\n\s+/g, ' ');
Expand All @@ -130,6 +130,9 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
delete value.extra.raw;
}
}
if (duplicateChildren && duplicateChildren.length > 0) {
value = t.sequenceExpression([...duplicateChildren, value]);
}

if (t.isJSXNamespacedName(node.name)) {
node.name = t.stringLiteral(
Expand Down Expand Up @@ -281,6 +284,17 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
function buildJSXOpeningElementAttributes(attribs, file, children) {
let _props = [];
const objs = [];

// In order to avoid having duplicate "children" keys, we avoid
// pushing the "children" prop if we have actual children. However,
// the children prop may have side effects, so to be certain
// these side effects are evaluated, we add them to the following prop
// as a sequence expression to preserve order. So:
// <div children={x++} foo={y}>{child}</div> becomes
// React.jsx('div', {foo: (x++, y), children: child});
// duplicateChildren contains the extra children prop values
let duplicateChildren = [];

const hasChildren = children && children.length > 0;

const useBuiltIns = file.opts.useBuiltIns || false;
Expand All @@ -293,32 +307,48 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,

while (attribs.length) {
const prop = attribs.shift();
if (t.isJSXSpreadAttribute(prop)) {
if (hasChildren && isChildrenProp(prop)) {
duplicateChildren.push(convertAttributeValue(prop.value));
} else if (t.isJSXSpreadAttribute(prop)) {
_props = pushProps(_props, objs);
objs.push(prop.argument);
} else if (hasChildren && isChildrenProp(prop)) {
// In order to avoid having duplicate "children" keys, we avoid
// pushing the "children" prop if we have actual children. Instead
// we put the children into a separate object and then rely on
// the Object.assign logic below to ensure the correct object is
// formed.
_props = pushProps(_props, objs);
objs.push(t.objectExpression([convertAttribute(prop)]));
if (duplicateChildren.length > 0) {
objs.push(
t.sequenceExpression([...duplicateChildren, prop.argument]),
);
duplicateChildren = [];
} else {
objs.push(prop.argument);
}
} else {
_props.push(convertAttribute(prop));
_props.push(convertAttribute(prop, duplicateChildren));
if (duplicateChildren.length > 0) {
duplicateChildren = [];
}
}
}

// In React.JSX, children is no longer a separate argument, but passed in
// through the argument object
if (hasChildren) {
if (children.length === 1) {
_props.push(t.objectProperty(t.identifier('children'), children[0]));
_props.push(
t.objectProperty(
t.identifier('children'),
duplicateChildren.length > 0
? t.sequenceExpression([...duplicateChildren, children[0]])
: children[0],
),
);
} else {
_props.push(
t.objectProperty(
t.identifier('children'),
t.arrayExpression(children),
duplicateChildren.length > 0
? t.sequenceExpression([
...duplicateChildren,
t.arrayExpression(children),
])
: t.arrayExpression(children),
),
);
}
Expand Down