Skip to content

Commit

Permalink
Make ReactDOM.createPortal() official (#10675)
Browse files Browse the repository at this point in the history
* Validate portal container early

* Add ReactDOM.createPortal but leave unstable_ alias usable
  • Loading branch information
gaearon authored Sep 11, 2017
1 parent f74981b commit 89508f2
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 39 deletions.
5 changes: 1 addition & 4 deletions src/renderers/__tests__/ReactUpdates-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,7 @@ describe('ReactUpdates', () => {
var portal = null;
// If we're using Fiber, we use Portals instead to achieve this.
if (ReactDOMFeatureFlags.useFiber) {
portal = ReactDOM.unstable_createPortal(
<B ref={n => (b = n)} />,
bContainer,
);
portal = ReactDOM.createPortal(<B ref={n => (b = n)} />, bContainer);
}
return <div>A{this.state.x}{portal}</div>;
}
Expand Down
5 changes: 1 addition & 4 deletions src/renderers/dom/__tests__/ReactDOMProduction-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,7 @@ describe('ReactDOMProduction', () => {
var expectSVG = {ref: el => svgEls.push(el)};
var expectHTML = {ref: el => htmlEls.push(el)};
var usePortal = function(tree) {
return ReactDOM.unstable_createPortal(
tree,
document.createElement('div'),
);
return ReactDOM.createPortal(tree, document.createElement('div'));
};
var assertNamespacesMatch = function(tree) {
var container = document.createElement('div');
Expand Down
26 changes: 18 additions & 8 deletions src/renderers/dom/fiber/ReactDOMFiberEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,22 @@ function renderSubtreeIntoContainer(
return DOMRenderer.getPublicRootInstance(root);
}

function createPortal(
children: ReactNodeList,
container: DOMContainer,
key: ?string = null,
) {
invariant(
isValidContainer(container),
'Target container is not a DOM element.',
);
// TODO: pass ReactDOM portal implementation as third argument
return ReactPortal.createPortal(children, container, null, key);
}

var ReactDOMFiber = {
createPortal,

hydrate(element: React$Node, container: DOMContainer, callback: ?Function) {
// TODO: throw or warn if we couldn't hydrate?
return renderSubtreeIntoContainer(null, element, container, true, callback);
Expand Down Expand Up @@ -742,14 +757,9 @@ var ReactDOMFiber = {

findDOMNode: findDOMNode,

unstable_createPortal(
children: ReactNodeList,
container: DOMContainer,
key: ?string = null,
) {
// TODO: pass ReactDOM portal implementation as third argument
return ReactPortal.createPortal(children, container, null, key);
},
// Temporary alias since we already shipped React 16 RC with it.
// TODO: remove in React 17.
unstable_createPortal: createPortal,

unstable_batchedUpdates: ReactGenericBatching.batchedUpdates,

Expand Down
64 changes: 44 additions & 20 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,7 @@ describe('ReactDOMFiber', () => {
var expectMath = {ref: el => mathEls.push(el)};

var usePortal = function(tree) {
return ReactDOM.unstable_createPortal(
tree,
document.createElement('div'),
);
return ReactDOM.createPortal(tree, document.createElement('div'));
};

var assertNamespacesMatch = function(tree) {
Expand All @@ -212,6 +209,24 @@ describe('ReactDOMFiber', () => {
it('should render one portal', () => {
var portalContainer = document.createElement('div');

ReactDOM.render(
<div>
{ReactDOM.createPortal(<div>portal</div>, portalContainer)}
</div>,
container,
);
expect(portalContainer.innerHTML).toBe('<div>portal</div>');
expect(container.innerHTML).toBe('<div></div>');

ReactDOM.unmountComponentAtNode(container);
expect(portalContainer.innerHTML).toBe('');
expect(container.innerHTML).toBe('');
});

// TODO: remove in React 17
it('should support unstable_createPortal alias', () => {
var portalContainer = document.createElement('div');

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(<div>portal</div>, portalContainer)}
Expand Down Expand Up @@ -260,12 +275,12 @@ describe('ReactDOMFiber', () => {
const {step} = this.props;
return [
<Child key="a" name={`normal[0]:${step}`} />,
ReactDOM.unstable_createPortal(
ReactDOM.createPortal(
<Child key="b" name={`portal1[0]:${step}`} />,
portalContainer1,
),
<Child key="c" name={`normal[1]:${step}`} />,
ReactDOM.unstable_createPortal(
ReactDOM.createPortal(
[
<Child key="d" name={`portal2[0]:${step}`} />,
<Child key="e" name={`portal2[1]:${step}`} />,
Expand Down Expand Up @@ -334,14 +349,14 @@ describe('ReactDOMFiber', () => {
ReactDOM.render(
[
<div key="a">normal[0]</div>,
ReactDOM.unstable_createPortal(
ReactDOM.createPortal(
[
<div key="b">portal1[0]</div>,
ReactDOM.unstable_createPortal(
ReactDOM.createPortal(
<div key="c">portal2[0]</div>,
portalContainer2,
),
ReactDOM.unstable_createPortal(
ReactDOM.createPortal(
<div key="d">portal3[0]</div>,
portalContainer3,
),
Expand Down Expand Up @@ -374,7 +389,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(<div>portal:1</div>, portalContainer)}
{ReactDOM.createPortal(<div>portal:1</div>, portalContainer)}
</div>,
container,
);
Expand All @@ -383,7 +398,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(<div>portal:2</div>, portalContainer)}
{ReactDOM.createPortal(<div>portal:2</div>, portalContainer)}
</div>,
container,
);
Expand All @@ -392,7 +407,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(<p>portal:3</p>, portalContainer)}
{ReactDOM.createPortal(<p>portal:3</p>, portalContainer)}
</div>,
container,
);
Expand All @@ -401,7 +416,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(['Hi', 'Bye'], portalContainer)}
{ReactDOM.createPortal(['Hi', 'Bye'], portalContainer)}
</div>,
container,
);
Expand All @@ -410,7 +425,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(['Bye', 'Hi'], portalContainer)}
{ReactDOM.createPortal(['Bye', 'Hi'], portalContainer)}
</div>,
container,
);
Expand All @@ -419,7 +434,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div>
{ReactDOM.unstable_createPortal(null, portalContainer)}
{ReactDOM.createPortal(null, portalContainer)}
</div>,
container,
);
Expand Down Expand Up @@ -700,7 +715,7 @@ describe('ReactDOMFiber', () => {
}

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

Expand Down Expand Up @@ -741,7 +756,7 @@ describe('ReactDOMFiber', () => {
}

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

Expand Down Expand Up @@ -781,7 +796,7 @@ describe('ReactDOMFiber', () => {
}

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

Expand Down Expand Up @@ -821,7 +836,7 @@ describe('ReactDOMFiber', () => {

ReactDOM.render(
<div onClick={() => ops.push('parent clicked')}>
{ReactDOM.unstable_createPortal(
{ReactDOM.createPortal(
<div
onClick={() => ops.push('portal clicked')}
ref={n => (portal = n)}>
Expand Down Expand Up @@ -874,7 +889,7 @@ describe('ReactDOMFiber', () => {
onMouseEnter={() => ops.push('enter parent')}
onMouseLeave={() => ops.push('leave parent')}>
<div ref={n => (firstTarget = n)} />
{ReactDOM.unstable_createPortal(
{ReactDOM.createPortal(
<div
onMouseEnter={() => ops.push('enter portal')}
onMouseLeave={() => ops.push('leave portal')}
Expand Down Expand Up @@ -909,6 +924,15 @@ describe('ReactDOMFiber', () => {
]);
});

it('should throw on bad createPortal argument', () => {
expect(() => {
ReactDOM.createPortal(<div>portal</div>, null);
}).toThrow('Target container is not a DOM element.');
expect(() => {
ReactDOM.createPortal(<div>portal</div>, document.createTextNode('hi'));
}).toThrow('Target container is not a DOM element.');
});

it('should warn for non-functional event listeners', () => {
spyOn(console, 'error');
class Example extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ describe('renderSubtreeIntoContainer', () => {
'a React 15 tree inside a React 16 tree using ' +
"unstable_renderSubtreeIntoContainer, which isn't supported. Try to " +
'make sure you have only one copy of React (and ideally, switch to ' +
'ReactDOM.unstable_createPortal).',
'ReactDOM.createPortal).',
);
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/native/ReactNativeFiberEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const ReactNativeFiber: ReactNativeType = {
UIManager.removeRootView(containerTag);
},

unstable_createPortal(
createPortal(
children: ReactNodeList,
containerTag: number,
key: ?string = null,
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ if (__DEV__) {
'a React 15 tree inside a React 16 tree using ' +
"unstable_renderSubtreeIntoContainer, which isn't supported. Try " +
'to make sure you have only one copy of React (and ideally, switch ' +
'to ReactDOM.unstable_createPortal).',
'to ReactDOM.createPortal).',
);
},
});
Expand Down

0 comments on commit 89508f2

Please sign in to comment.