Skip to content

Commit b58fd28

Browse files
committed
[Fiber] Add ReactDOMFiber.unstable_createPortal()
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.
1 parent 8334bb0 commit b58fd28

File tree

11 files changed

+443
-4
lines changed

11 files changed

+443
-4
lines changed

scripts/fiber/tests-passing.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,10 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
488488
* finds the first child when a component returns a fragment
489489
* finds the first child even when fragment is nested
490490
* finds the first child even when first child renders null
491+
* should render portal children
492+
* should pass portal context when rendering subtree elsewhere
493+
* should update portal context if it changes due to setState
494+
* should update portal context if it changes due to re-render
491495

492496
src/renderers/dom/shared/__tests__/CSSProperty-test.js
493497
* should generate browser prefixes for its `isUnitlessNumber`

src/renderers/dom/fiber/ReactDOMFiber.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import type { Fiber } from 'ReactFiber';
1616
import type { HostChildren } from 'ReactFiberReconciler';
17+
import type { ReactNodeList } from 'ReactTypes';
1718

1819
var ReactControlledComponent = require('ReactControlledComponent');
1920
var ReactDOMComponentTree = require('ReactDOMComponentTree');
@@ -22,6 +23,8 @@ var ReactDOMFiberComponent = require('ReactDOMFiberComponent');
2223
var ReactDOMInjection = require('ReactDOMInjection');
2324
var ReactFiberReconciler = require('ReactFiberReconciler');
2425
var ReactInstanceMap = require('ReactInstanceMap');
26+
var ReactPortal = require('ReactPortal');
27+
var ReactTypeOfWork = require('ReactTypeOfWork');
2528

2629
var findDOMNode = require('findDOMNode');
2730
var invariant = require('invariant');
@@ -32,6 +35,9 @@ ReactControlledComponent.injection.injectFiberControlledHostComponent(
3235
ReactDOMFiberComponent
3336
);
3437

38+
var {
39+
Portal,
40+
} = ReactTypeOfWork;
3541
var {
3642
createElement,
3743
setInitialProperties,
@@ -66,7 +72,11 @@ function recursivelyAppendChildren(parent : Element, child : HostChildren<Instan
6672
/* As a result of the refinement issue this type isn't known. */
6773
let node : any = child;
6874
do {
69-
recursivelyAppendChildren(parent, node.output);
75+
// TODO: this is an implementation detail leaking into the renderer.
76+
// Once we move output traversal to complete phase, we won't need this.
77+
if (node.tag !== Portal) {
78+
recursivelyAppendChildren(parent, node.output);
79+
}
7080
} while (node = node.sibling);
7181
}
7282
}
@@ -192,6 +202,11 @@ var ReactDOM = {
192202

193203
findDOMNode: findDOMNode,
194204

205+
unstable_createPortal(children: ReactNodeList, container : DOMContainerElement, key : ?string = null) {
206+
// TODO: pass ReactDOM portal implementation as third argument
207+
return ReactPortal.createPortal(children, container, null, key);
208+
},
209+
195210
unstable_batchedUpdates<A>(fn : () => A) : A {
196211
return DOMRenderer.batchedUpdates(fn);
197212
},

src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,4 +186,225 @@ describe('ReactDOMFiber', () => {
186186
expect(firstNode.tagName).toBe('DIV');
187187
});
188188
}
189+
190+
if (ReactDOMFeatureFlags.useFiber) {
191+
it('should render portal children', () => {
192+
var portalContainer1 = document.createElement('div');
193+
var portalContainer2 = document.createElement('div');
194+
195+
var ops = [];
196+
class Child extends React.Component {
197+
componentDidMount() {
198+
ops.push(`${this.props.name} componentDidMount`);
199+
}
200+
componentDidUpdate() {
201+
ops.push(`${this.props.name} componentDidUpdate`);
202+
}
203+
componentWillUnmount() {
204+
ops.push(`${this.props.name} componentWillUnmount`);
205+
}
206+
render() {
207+
return <div>{this.props.name}</div>;
208+
}
209+
}
210+
211+
class Parent extends React.Component {
212+
componentDidMount() {
213+
ops.push(`Parent:${this.props.step} componentDidMount`);
214+
}
215+
componentDidUpdate() {
216+
ops.push(`Parent:${this.props.step} componentDidUpdate`);
217+
}
218+
componentWillUnmount() {
219+
ops.push(`Parent:${this.props.step} componentWillUnmount`);
220+
}
221+
render() {
222+
const {step} = this.props;
223+
return [
224+
<Child name={`normal[0]:${step}`} />,
225+
ReactDOM.unstable_createPortal(
226+
<Child name={`portal1[0]:${step}`} />,
227+
portalContainer1
228+
),
229+
<Child name={`normal[1]:${step}`} />,
230+
ReactDOM.unstable_createPortal(
231+
[
232+
<Child name={`portal2[0]:${step}`} />,
233+
<Child name={`portal2[1]:${step}`} />,
234+
],
235+
portalContainer2
236+
),
237+
];
238+
}
239+
}
240+
241+
ReactDOM.render(<Parent step="a" />, container);
242+
expect(portalContainer1.innerHTML).toBe('<div>portal1[0]:a</div>');
243+
expect(portalContainer2.innerHTML).toBe('<div>portal2[0]:a</div><div>portal2[1]:a</div>');
244+
expect(container.innerHTML).toBe('<div>normal[0]:a</div><div>normal[1]:a</div>');
245+
expect(ops).toEqual([
246+
'normal[0]:a componentDidMount',
247+
'portal1[0]:a componentDidMount',
248+
'normal[1]:a componentDidMount',
249+
'portal2[0]:a componentDidMount',
250+
'portal2[1]:a componentDidMount',
251+
'Parent:a componentDidMount',
252+
]);
253+
254+
ops.length = 0;
255+
ReactDOM.render(<Parent step="b" />, container);
256+
expect(portalContainer1.innerHTML).toBe('<div>portal1[0]:b</div>');
257+
expect(portalContainer2.innerHTML).toBe('<div>portal2[0]:b</div><div>portal2[1]:b</div>');
258+
expect(container.innerHTML).toBe('<div>normal[0]:b</div><div>normal[1]:b</div>');
259+
expect(ops).toEqual([
260+
'normal[0]:b componentDidUpdate',
261+
'portal1[0]:b componentDidUpdate',
262+
'normal[1]:b componentDidUpdate',
263+
'portal2[0]:b componentDidUpdate',
264+
'portal2[1]:b componentDidUpdate',
265+
'Parent:b componentDidUpdate',
266+
]);
267+
268+
ops.length = 0;
269+
ReactDOM.unmountComponentAtNode(container);
270+
expect(portalContainer1.innerHTML).toBe('');
271+
expect(portalContainer2.innerHTML).toBe('');
272+
expect(container.innerHTML).toBe('');
273+
expect(ops).toEqual([
274+
'Parent:b componentWillUnmount',
275+
'normal[0]:b componentWillUnmount',
276+
'portal1[0]:b componentWillUnmount',
277+
'normal[1]:b componentWillUnmount',
278+
'portal2[0]:b componentWillUnmount',
279+
'portal2[1]:b componentWillUnmount',
280+
]);
281+
});
282+
283+
it('should pass portal context when rendering subtree elsewhere', () => {
284+
var portalContainer = document.createElement('div');
285+
286+
class Component extends React.Component {
287+
static contextTypes = {
288+
foo: React.PropTypes.string.isRequired,
289+
};
290+
291+
render() {
292+
return <div>{this.context.foo}</div>;
293+
}
294+
}
295+
296+
class Parent extends React.Component {
297+
static childContextTypes = {
298+
foo: React.PropTypes.string.isRequired,
299+
};
300+
301+
getChildContext() {
302+
return {
303+
foo: 'bar',
304+
};
305+
}
306+
307+
render() {
308+
return ReactDOM.unstable_createPortal(
309+
<Component />,
310+
portalContainer
311+
);
312+
}
313+
}
314+
315+
ReactDOM.render(<Parent />, container);
316+
expect(container.innerHTML).toBe('');
317+
expect(portalContainer.innerHTML).toBe('<div>bar</div>');
318+
});
319+
320+
it('should update portal context if it changes due to setState', () => {
321+
var portalContainer = document.createElement('div');
322+
323+
class Component extends React.Component {
324+
static contextTypes = {
325+
foo: React.PropTypes.string.isRequired,
326+
getFoo: React.PropTypes.func.isRequired,
327+
};
328+
329+
render() {
330+
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
331+
}
332+
}
333+
334+
class Parent extends React.Component {
335+
static childContextTypes = {
336+
foo: React.PropTypes.string.isRequired,
337+
getFoo: React.PropTypes.func.isRequired,
338+
};
339+
340+
state = {
341+
bar: 'initial',
342+
};
343+
344+
getChildContext() {
345+
return {
346+
foo: this.state.bar,
347+
getFoo: () => this.state.bar,
348+
};
349+
}
350+
351+
render() {
352+
return ReactDOM.unstable_createPortal(
353+
<Component />,
354+
portalContainer
355+
);
356+
}
357+
}
358+
359+
var instance = ReactDOM.render(<Parent />, container);
360+
expect(portalContainer.innerHTML).toBe('<div>initial-initial</div>');
361+
expect(container.innerHTML).toBe('');
362+
instance.setState({bar: 'changed'});
363+
expect(portalContainer.innerHTML).toBe('<div>changed-changed</div>');
364+
expect(container.innerHTML).toBe('');
365+
});
366+
367+
it('should update portal context if it changes due to re-render', () => {
368+
var portalContainer = document.createElement('div');
369+
370+
class Component extends React.Component {
371+
static contextTypes = {
372+
foo: React.PropTypes.string.isRequired,
373+
getFoo: React.PropTypes.func.isRequired,
374+
};
375+
376+
render() {
377+
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
378+
}
379+
}
380+
381+
class Parent extends React.Component {
382+
static childContextTypes = {
383+
foo: React.PropTypes.string.isRequired,
384+
getFoo: React.PropTypes.func.isRequired,
385+
};
386+
387+
getChildContext() {
388+
return {
389+
foo: this.props.bar,
390+
getFoo: () => this.props.bar,
391+
};
392+
}
393+
394+
render() {
395+
return ReactDOM.unstable_createPortal(
396+
<Component />,
397+
portalContainer
398+
);
399+
}
400+
}
401+
402+
ReactDOM.render(<Parent bar="initial" />, container);
403+
expect(portalContainer.innerHTML).toBe('<div>initial-initial</div>');
404+
expect(container.innerHTML).toBe('');
405+
ReactDOM.render(<Parent bar="changed" />, container);
406+
expect(portalContainer.innerHTML).toBe('<div>changed-changed</div>');
407+
expect(container.innerHTML).toBe('');
408+
});
409+
}
189410
});

0 commit comments

Comments
 (0)