Skip to content

Commit

Permalink
Add Fragment as a named export to React (facebook#10783)
Browse files Browse the repository at this point in the history
* Add Fragment as a named export to React

* Remove extra tests for Fragment

* Change React.Fragment export to be a string '#fragment'

* Fix fragment special case to work with 1 child

* Add single child test for fragment export

* Move fragment definition to ReactEntry.js and render components for key warning tests

* Inline createFiberFromElementType into createFiberFromElement

* Update reconciliation to special case fragments

* Use same semantics as implicit childsets for ReactFragment

* Add more fragment state preservation tests

* Export symbol instead of string for fragments

* Fix rebase breakages

* Re-apply prettier at 1.2.2

* Merge branches in updateElement

* Remove unnecessary check

* Re-use createFiberFromFragment for fragment case

* Simplyify branches by adding type field to fragment fiber

* Move branching logic for fragments to broader methods when possible.

* Add more tests for fragments

* Address Dan's feedback

* Move REACT_FRAGMENT_TYPE into __DEV__ block for DCE

* Change hex representation of REACT_FRAGMENT_TYPE to follow convention

* Remove unnecessary branching and isArray checks

* Update test for preserving children state when keys are same

* Fix updateSlot bug and add more tests

* Make fragment tests more robust by using ops pattern

* Update jsx element validator to allow numbers and symbols

* Remove type field from fragment fiber

* Fork reconcileChildFibers instead of recursing

* Use ternary if condition

* Revamp fragment test suite:

- Add more coverage to fragment tests
- Use better names
- Remove useless Fragment component inside tests
- Remove useless tests so that tests are more concise

* Check output of renderer in fragment tests to ensure no silly business despite states being preserved

* Finish implementation of fragment reconciliation with desired behavior

* Add reverse render direction for fragment tests

* Remove unneeded fragment branch in updateElement

* Add more test cases for ReactFragment

* Handle childless fragment in reconciler

* Support fragment flattening in SSR

* Clean up ReactPartialRenderer

* Warn when non-key and children props are passed to fragments

* Add non-null key check back to updateSlot's array's case

* Add test for positional reconciliation in fragments

* Add warning for refs in fragments with stack trace
  • Loading branch information
clemmy authored and Ethan-Arrowood committed Dec 8, 2017
1 parent 65adc20 commit e11dc40
Show file tree
Hide file tree
Showing 10 changed files with 1,120 additions and 149 deletions.
50 changes: 50 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegration-test.js
Expand Up @@ -403,6 +403,55 @@ describe('ReactDOMServerIntegration', () => {
expect(parent.childNodes[2].tagName).toBe('P');
});

itRenders('a fragment with one child', async render => {
let e = await render(<React.Fragment><div>text1</div></React.Fragment>);
let parent = e.parentNode;
expect(parent.childNodes[0].tagName).toBe('DIV');
});

itRenders('a fragment with several children', async render => {
let Header = props => {
return <p>header</p>;
};
let Footer = props => {
return <React.Fragment><h2>footer</h2><h3>about</h3></React.Fragment>;
};
let e = await render(
<React.Fragment>
<div>text1</div>
<span>text2</span>
<Header />
<Footer />
</React.Fragment>,
);
let parent = e.parentNode;
expect(parent.childNodes[0].tagName).toBe('DIV');
expect(parent.childNodes[1].tagName).toBe('SPAN');
expect(parent.childNodes[2].tagName).toBe('P');
expect(parent.childNodes[3].tagName).toBe('H2');
expect(parent.childNodes[4].tagName).toBe('H3');
});

itRenders('a nested fragment', async render => {
let e = await render(
<React.Fragment>
<React.Fragment>
<div>text1</div>
</React.Fragment>
<span>text2</span>
<React.Fragment>
<React.Fragment>
<React.Fragment>{null}<p /></React.Fragment>{false}
</React.Fragment>
</React.Fragment>
</React.Fragment>,
);
let parent = e.parentNode;
expect(parent.childNodes[0].tagName).toBe('DIV');
expect(parent.childNodes[1].tagName).toBe('SPAN');
expect(parent.childNodes[2].tagName).toBe('P');
});

itRenders('an iterable', async render => {
const threeDivIterable = {
'@@iterator': function() {
Expand Down Expand Up @@ -435,6 +484,7 @@ describe('ReactDOMServerIntegration', () => {
// but server returns empty HTML. So we compare parent text.
expect((await render(<div>{''}</div>)).textContent).toBe('');

expect(await render(<React.Fragment />)).toBe(null);
expect(await render([])).toBe(null);
expect(await render(false)).toBe(null);
expect(await render(true)).toBe(null);
Expand Down
86 changes: 59 additions & 27 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Expand Up @@ -31,6 +31,12 @@ var escapeTextContentForBrowser = require('../shared/escapeTextContentForBrowser
var isCustomComponent = require('../shared/isCustomComponent');
var omittedCloseTags = require('../shared/omittedCloseTags');

var REACT_FRAGMENT_TYPE =
(typeof Symbol === 'function' &&
Symbol.for &&
Symbol.for('react.fragment')) ||
0xeacb;

// Based on reading the React.Children implementation. TODO: type this somewhere?
type ReactNode = string | number | ReactElement;
type FlatReactChildren = Array<null | ReactNode>;
Expand Down Expand Up @@ -206,6 +212,22 @@ function getNonChildrenInnerMarkup(props) {
return null;
}

function flattenTopLevelChildren(children: mixed): FlatReactChildren {
if (!React.isValidElement(children)) {
return toArray(children);
}
const element = ((children: any): ReactElement);
if (element.type !== REACT_FRAGMENT_TYPE) {
return [element];
}
const fragmentChildren = element.props.children;
if (!React.isValidElement(fragmentChildren)) {
return toArray(fragmentChildren);
}
const fragmentChildElement = ((fragmentChildren: any): ReactElement);
return [fragmentChildElement];
}

function flattenOptionChildren(children: mixed): string {
var content = '';
// Flatten children and warn if they aren't strings or numbers;
Expand Down Expand Up @@ -482,14 +504,8 @@ class ReactDOMServerRenderer {
makeStaticMarkup: boolean;

constructor(children: mixed, makeStaticMarkup: boolean) {
var flatChildren;
if (React.isValidElement(children)) {
// Safe because we just checked it's an element.
var element = ((children: any): ReactElement);
flatChildren = [element];
} else {
flatChildren = toArray(children);
}
const flatChildren = flattenTopLevelChildren(children);

var topFrame: Frame = {
// Assume all trees start in the HTML namespace (not totally true, but
// this is what we did historically)
Expand Down Expand Up @@ -569,26 +585,42 @@ class ReactDOMServerRenderer {
({child: nextChild, context} = resolve(child, context));
if (nextChild === null || nextChild === false) {
return '';
} else {
if (React.isValidElement(nextChild)) {
// Safe because we just checked it's an element.
var nextElement = ((nextChild: any): ReactElement);
return this.renderDOM(nextElement, context, parentNamespace);
} else {
var nextChildren = toArray(nextChild);
var frame: Frame = {
domNamespace: parentNamespace,
children: nextChildren,
childIndex: 0,
context: context,
footer: '',
};
if (__DEV__) {
((frame: any): FrameDev).debugElementStack = [];
}
this.stack.push(frame);
return '';
} else if (!React.isValidElement(nextChild)) {
const nextChildren = toArray(nextChild);
const frame: Frame = {
domNamespace: parentNamespace,
children: nextChildren,
childIndex: 0,
context: context,
footer: '',
};
if (__DEV__) {
((frame: any): FrameDev).debugElementStack = [];
}
this.stack.push(frame);
return '';
} else if (
((nextChild: any): ReactElement).type === REACT_FRAGMENT_TYPE
) {
const nextChildren = toArray(
((nextChild: any): ReactElement).props.children,
);
const frame: Frame = {
domNamespace: parentNamespace,
children: nextChildren,
childIndex: 0,
context: context,
footer: '',
};
if (__DEV__) {
((frame: any): FrameDev).debugElementStack = [];
}
this.stack.push(frame);
return '';
} else {
// Safe because we just checked it's an element.
var nextElement = ((nextChild: any): ReactElement);
return this.renderDOM(nextElement, context, parentNamespace);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/react-noop-renderer/src/ReactNoop.js
Expand Up @@ -512,7 +512,8 @@ var ReactNoop = {
log(
' '.repeat(depth) +
'- ' +
(fiber.type ? fiber.type.name || fiber.type : '[root]'),
// need to explicitly coerce Symbol to a string
(fiber.type ? fiber.type.name || fiber.type.toString() : '[root]'),
'[' + fiber.expirationTime + (fiber.pendingProps ? '*' : '') + ']',
);
if (fiber.updateQueue) {
Expand Down

0 comments on commit e11dc40

Please sign in to comment.