Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ReactDOM.createPortal() official #10675

Merged
merged 2 commits into from
Sep 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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