Skip to content

Commit

Permalink
[Fiber] Add ReactDOM.unstable_createPortal()
Browse files Browse the repository at this point in the history
While facebook#8368 added a version of `ReactDOM.unstable_renderSubtreeIntoContainer()` to Fiber, it is a bit hacky and, more importantly, incompatible with Fiber goals. Since it encourages performing portal work in lifecycles, it stretches the commit phase and prevents slicing that work, potentially negating Fiber benefits.

This PR adds a first version of a declarative API meant to replace `ReactDOM.unstable_renderSubtreeIntoContainer()`. The API is a declarative way to render subtrees into DOM node containers.
  • Loading branch information
gaearon committed Nov 22, 2016
1 parent 8334bb0 commit 3f08d2e
Show file tree
Hide file tree
Showing 14 changed files with 450 additions and 7 deletions.
3 changes: 3 additions & 0 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ src/renderers/art/__tests__/ReactART-test.js
* resolves refs before componentDidMount
* resolves refs before componentDidUpdate

src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should throw with an error code in production

src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js
* should store a listener correctly
* should retrieve a listener correctly
Expand Down
5 changes: 4 additions & 1 deletion scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should use prod React
* should handle a simple flow
* should call lifecycle methods
* should throw with an error code in production

src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render strings as children
Expand All @@ -488,6 +487,10 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* finds the first child when a component returns a fragment
* finds the first child even when fragment is nested
* finds the first child even when first child renders null
* should render portal children
* should pass portal context when rendering subtree elsewhere
* should update portal context if it changes due to setState
* should update portal context if it changes due to re-render

src/renderers/dom/shared/__tests__/CSSProperty-test.js
* should generate browser prefixes for its `isUnitlessNumber`
Expand Down
17 changes: 16 additions & 1 deletion src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import type { Fiber } from 'ReactFiber';
import type { HostChildren } from 'ReactFiberReconciler';
import type { ReactNodeList } from 'ReactTypes';

var ReactControlledComponent = require('ReactControlledComponent');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
Expand All @@ -22,6 +23,8 @@ var ReactDOMFiberComponent = require('ReactDOMFiberComponent');
var ReactDOMInjection = require('ReactDOMInjection');
var ReactFiberReconciler = require('ReactFiberReconciler');
var ReactInstanceMap = require('ReactInstanceMap');
var ReactPortal = require('ReactPortal');
var ReactTypeOfWork = require('ReactTypeOfWork');

var findDOMNode = require('findDOMNode');
var invariant = require('invariant');
Expand All @@ -32,6 +35,9 @@ ReactControlledComponent.injection.injectFiberControlledHostComponent(
ReactDOMFiberComponent
);

var {
Portal,
} = ReactTypeOfWork;
var {
createElement,
setInitialProperties,
Expand Down Expand Up @@ -66,7 +72,11 @@ function recursivelyAppendChildren(parent : Element, child : HostChildren<Instan
/* As a result of the refinement issue this type isn't known. */
let node : any = child;
do {
recursivelyAppendChildren(parent, node.output);
// TODO: this is an implementation detail leaking into the renderer.
// Once we move output traversal to complete phase, we won't need this.
if (node.tag !== Portal) {
recursivelyAppendChildren(parent, node.output);
}
} while (node = node.sibling);
}
}
Expand Down Expand Up @@ -192,6 +202,11 @@ var ReactDOM = {

findDOMNode: findDOMNode,

unstable_createPortal(children: ReactNodeList, container : DOMContainerElement, key : ?string = null) {
// TODO: pass ReactDOM portal implementation as third argument
return ReactPortal.createPortal(children, container, null, key);
},

unstable_batchedUpdates<A>(fn : () => A) : A {
return DOMRenderer.batchedUpdates(fn);
},
Expand Down
224 changes: 224 additions & 0 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ describe('ReactDOMFiber', () => {
container
);
expect(container.textContent).toEqual('foo');

ReactDOM.unmountComponentAtNode(container);
expect(container.textContent).toEqual(''); // TODO
});

it('should render numbers as children', () => {
Expand Down Expand Up @@ -186,4 +189,225 @@ describe('ReactDOMFiber', () => {
expect(firstNode.tagName).toBe('DIV');
});
}

if (ReactDOMFeatureFlags.useFiber) {
it('should render portal children', () => {
var portalContainer1 = document.createElement('div');
var portalContainer2 = document.createElement('div');

var ops = [];
class Child extends React.Component {
componentDidMount() {
ops.push(`${this.props.name} componentDidMount`);
}
componentDidUpdate() {
ops.push(`${this.props.name} componentDidUpdate`);
}
componentWillUnmount() {
ops.push(`${this.props.name} componentWillUnmount`);
}
render() {
return <div>{this.props.name}</div>;
}
}

class Parent extends React.Component {
componentDidMount() {
ops.push(`Parent:${this.props.step} componentDidMount`);
}
componentDidUpdate() {
ops.push(`Parent:${this.props.step} componentDidUpdate`);
}
componentWillUnmount() {
ops.push(`Parent:${this.props.step} componentWillUnmount`);
}
render() {
const {step} = this.props;
return [
<Child name={`normal[0]:${step}`} />,
ReactDOM.unstable_createPortal(
<Child name={`portal1[0]:${step}`} />,
portalContainer1
),
<Child name={`normal[1]:${step}`} />,
ReactDOM.unstable_createPortal(
[
<Child name={`portal2[0]:${step}`} />,
<Child name={`portal2[1]:${step}`} />,
],
portalContainer2
),
];
}
}

ReactDOM.render(<Parent step="a" />, container);
expect(portalContainer1.innerHTML).toBe('<div>portal1[0]:a</div>');
expect(portalContainer2.innerHTML).toBe('<div>portal2[0]:a</div><div>portal2[1]:a</div>');
expect(container.innerHTML).toBe('<div>normal[0]:a</div><div>normal[1]:a</div>');
expect(ops).toEqual([
'normal[0]:a componentDidMount',
'portal1[0]:a componentDidMount',
'normal[1]:a componentDidMount',
'portal2[0]:a componentDidMount',
'portal2[1]:a componentDidMount',
'Parent:a componentDidMount',
]);

ops.length = 0;
ReactDOM.render(<Parent step="b" />, container);
expect(portalContainer1.innerHTML).toBe('<div>portal1[0]:b</div>');
expect(portalContainer2.innerHTML).toBe('<div>portal2[0]:b</div><div>portal2[1]:b</div>');
expect(container.innerHTML).toBe('<div>normal[0]:b</div><div>normal[1]:b</div>');
expect(ops).toEqual([
'normal[0]:b componentDidUpdate',
'portal1[0]:b componentDidUpdate',
'normal[1]:b componentDidUpdate',
'portal2[0]:b componentDidUpdate',
'portal2[1]:b componentDidUpdate',
'Parent:b componentDidUpdate',
]);

ops.length = 0;
ReactDOM.unmountComponentAtNode(container);
expect(portalContainer1.innerHTML).toBe('');
expect(portalContainer2.innerHTML).toBe('');
expect(container.innerHTML).toBe('');
expect(ops).toEqual([
'Parent:b componentWillUnmount',
'normal[0]:b componentWillUnmount',
'portal1[0]:b componentWillUnmount',
'normal[1]:b componentWillUnmount',
'portal2[0]:b componentWillUnmount',
'portal2[1]:b componentWillUnmount',
]);
});

it('should pass portal context when rendering subtree elsewhere', () => {
var portalContainer = document.createElement('div');

class Component extends React.Component {
static contextTypes = {
foo: React.PropTypes.string.isRequired,
};

render() {
return <div>{this.context.foo}</div>;
}
}

class Parent extends React.Component {
static childContextTypes = {
foo: React.PropTypes.string.isRequired,
};

getChildContext() {
return {
foo: 'bar',
};
}

render() {
return ReactDOM.unstable_createPortal(
<Component />,
portalContainer
);
}
}

ReactDOM.render(<Parent />, container);
expect(container.innerHTML).toBe('');
expect(portalContainer.innerHTML).toBe('<div>bar</div>');
});

it('should update portal context if it changes due to setState', () => {
var portalContainer = document.createElement('div');

class Component extends React.Component {
static contextTypes = {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
};

render() {
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
}
}

class Parent extends React.Component {
static childContextTypes = {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
};

state = {
bar: 'initial',
};

getChildContext() {
return {
foo: this.state.bar,
getFoo: () => this.state.bar,
};
}

render() {
return ReactDOM.unstable_createPortal(
<Component />,
portalContainer
);
}
}

var instance = ReactDOM.render(<Parent />, container);
expect(portalContainer.innerHTML).toBe('<div>initial-initial</div>');
expect(container.innerHTML).toBe('');
instance.setState({bar: 'changed'});
expect(portalContainer.innerHTML).toBe('<div>changed-changed</div>');
expect(container.innerHTML).toBe('');
});

it('should update portal context if it changes due to re-render', () => {
var portalContainer = document.createElement('div');

class Component extends React.Component {
static contextTypes = {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
};

render() {
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
}
}

class Parent extends React.Component {
static childContextTypes = {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
};

getChildContext() {
return {
foo: this.props.bar,
getFoo: () => this.props.bar,
};
}

render() {
return ReactDOM.unstable_createPortal(
<Component />,
portalContainer
);
}
}

ReactDOM.render(<Parent bar="initial" />, container);
expect(portalContainer.innerHTML).toBe('<div>initial-initial</div>');
expect(container.innerHTML).toBe('');
ReactDOM.render(<Parent bar="changed" />, container);
expect(portalContainer.innerHTML).toBe('<div>changed-changed</div>');
expect(container.innerHTML).toBe('');
});
}
});
2 changes: 1 addition & 1 deletion src/renderers/dom/shared/ReactDOMFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

var ReactDOMFeatureFlags = {
useCreateElement: true,
useFiber: false,
useFiber: true,
};

module.exports = ReactDOMFeatureFlags;

0 comments on commit 3f08d2e

Please sign in to comment.