Modify Babel React JSX Duplicate Children Fix (#17101)
If a JSX element has both a children prop and children (ie. <div children={childOne}>{childTwo}</div>), IE throws an Multiple definitions of a property not allowed in strict mode. This modifies the previous fix (which used an Object.assign) by making the duplicate children a sequence expression on the next prop/child instead so that ordering is preserved. For example: ``` <Component children={useA()} foo={useB()} children={useC()}>{useD()}</Component> ``` should compile to ``` React.jsx(Component, {foo: (useA(), useB()), children: (useC(), useD)}) ```
Luna Ruan committed
Oct 15, 2019 at 17:13 UTC
3ac0eb075d82b19a912c6665eb4f6adb9245cbcb
3 files changed
+87
-24
packages/babel-plugin-react-jsx/__tests__/TransformJSXToReactJSX-test.js
+16
-1
@@ -481,9 +481,24 @@ describe('transform react to jsx', () => {
481
).toMatchSnapshot();
482
});
483
484
- it('should not contain duplicate children key in props object', () => {
484
+ it('duplicate children prop should transform into sequence expression with actual children', () => {
485
expect(
486
transform(`<Component children={1}>2</Component>`)
487
).toMatchSnapshot();
488
});
489
+ it('duplicate children prop should transform into sequence expression with next prop', () => {
490
+ expect(
491
+ transform(`<Component children={1} foo={3}>2</Component>`)
492
+ ).toMatchSnapshot();
493
+ });
494
+ it('duplicate children props should transform into sequence expression with next prop', () => {
495
+ expect(
496
+ transform(`<Component children={1} children={4} foo={3}>2</Component>`)
497
+ ).toMatchSnapshot();
498
+ });
499
+ it('duplicate children prop should transform into sequence expression with spread', () => {
500
+ expect(
501
+ transform(`<Component children={1} {...x}>2</Component>`)
502
+ ).toMatchSnapshot();
503
+ });
504
});
packages/babel-plugin-react-jsx/__tests__/__snapshots__/TransformJSXToReactJSX-test.js.snap
+26
-8
@@ -46,6 +46,32 @@ var x = React.jsxs("div", {
46
});
47
`;
48
49
+exports[`transform react to jsx duplicate children prop should transform into sequence expression with actual children 1`] = `
50
+React.jsx(Component, {
51
+ children: (1, "2")
52
+});
53
+`;
54
+
55
+exports[`transform react to jsx duplicate children prop should transform into sequence expression with next prop 1`] = `
56
+React.jsx(Component, {
57
+ foo: (1, 3),
58
+ children: "2"
59
+});
60
+`;
61
+
62
+exports[`transform react to jsx duplicate children prop should transform into sequence expression with spread 1`] = `
63
+React.jsx(Component, Object.assign({}, (1, x), {
64
+ children: "2"
65
+}));
66
+`;
67
+
68
+exports[`transform react to jsx duplicate children props should transform into sequence expression with next prop 1`] = `
69
+React.jsx(Component, {
70
+ foo: (1, 4, 3),
71
+ children: "2"
72
+});
73
+`;
74
+
75
exports[`transform react to jsx fragment with no children 1`] = `var x = React.jsx(React.Fragment, {});`;
76
77
exports[`transform react to jsx fragments 1`] = `
@@ -250,14 +276,6 @@ var e = React.jsx(F, {
276
});
277
`;
278
253
-exports[`transform react to jsx should not contain duplicate children key in props object 1`] = `
254
-React.jsx(Component, Object.assign({
255
- children: 1
256
-}, {
257
- children: "2"
258
-}));
259
-`;
260
-
279
exports[`transform react to jsx should not strip nbsp even couple with other whitespace 1`] = `
280
React.jsx("div", {
281
children: "\\xA0 "
packages/babel-plugin-react-jsx/src/TransformJSXToReactBabelPlugin.js
+45
-15
@@ -119,8 +119,8 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
119
}
120
}
121
122
- function convertAttribute(node) {
123
- const value = convertAttributeValue(node.value || t.booleanLiteral(true));
122
+ function convertAttribute(node, duplicateChildren) {
123
+ let value = convertAttributeValue(node.value || t.booleanLiteral(true));
124
125
if (t.isStringLiteral(value) && !t.isJSXExpressionContainer(node.value)) {
126
value.value = value.value.replace(/\n\s+/g, ' ');
@@ -130,6 +130,9 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
130
delete value.extra.raw;
131
}
132
}
133
+ if (duplicateChildren && duplicateChildren.length > 0) {
134
+ value = t.sequenceExpression([...duplicateChildren, value]);
135
+ }
136
137
if (t.isJSXNamespacedName(node.name)) {
138
node.name = t.stringLiteral(
@@ -281,6 +284,17 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
284
function buildJSXOpeningElementAttributes(attribs, file, children) {
285
let _props = [];
286
const objs = [];
287
+
288
+ // In order to avoid having duplicate "children" keys, we avoid
289
+ // pushing the "children" prop if we have actual children. However,
290
+ // the children prop may have side effects, so to be certain
291
+ // these side effects are evaluated, we add them to the following prop
292
+ // as a sequence expression to preserve order. So:
293
+ // <div children={x++} foo={y}>{child}</div> becomes
294
+ // React.jsx('div', {foo: (x++, y), children: child});
295
+ // duplicateChildren contains the extra children prop values
296
+ let duplicateChildren = [];
297
+
298
const hasChildren = children && children.length > 0;
299
300
const useBuiltIns = file.opts.useBuiltIns || false;
@@ -293,19 +307,23 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
307
308
while (attribs.length) {
309
const prop = attribs.shift();
296
- if (t.isJSXSpreadAttribute(prop)) {
310
+ if (hasChildren && isChildrenProp(prop)) {
311
+ duplicateChildren.push(convertAttributeValue(prop.value));
312
+ } else if (t.isJSXSpreadAttribute(prop)) {
313
_props = pushProps(_props, objs);
298
- objs.push(prop.argument);
299
- } else if (hasChildren && isChildrenProp(prop)) {
300
- // In order to avoid having duplicate "children" keys, we avoid
301
- // pushing the "children" prop if we have actual children. Instead
302
- // we put the children into a separate object and then rely on
303
- // the Object.assign logic below to ensure the correct object is
304
- // formed.
305
- _props = pushProps(_props, objs);
306
- objs.push(t.objectExpression([convertAttribute(prop)]));
314
+ if (duplicateChildren.length > 0) {
315
+ objs.push(
316
+ t.sequenceExpression([...duplicateChildren, prop.argument]),
317
+ );
318
+ duplicateChildren = [];
319
+ } else {
320
+ objs.push(prop.argument);
321
+ }
322
} else {
308
- _props.push(convertAttribute(prop));
323
+ _props.push(convertAttribute(prop, duplicateChildren));
324
+ if (duplicateChildren.length > 0) {
325
+ duplicateChildren = [];
326
+ }
327
}
328
}
329
@@ -313,12 +331,24 @@ You can turn on the 'throwIfNamespace' flag to bypass this warning.`,
331
// through the argument object
332
if (hasChildren) {
333
if (children.length === 1) {
316
- _props.push(t.objectProperty(t.identifier('children'), children[0]));
334
+ _props.push(
335
+ t.objectProperty(
336
+ t.identifier('children'),
337
+ duplicateChildren.length > 0
338
+ ? t.sequenceExpression([...duplicateChildren, children[0]])
339
+ : children[0],
340
+ ),
341
+ );
342
} else {
343
_props.push(
344
t.objectProperty(
345
t.identifier('children'),
321
- t.arrayExpression(children),
346
+ duplicateChildren.length > 0
347
+ ? t.sequenceExpression([
348
+ ...duplicateChildren,
349
+ t.arrayExpression(children),
350
+ ])
351
+ : t.arrayExpression(children),
352
),
353
);
354
}