Skip to content
Open
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
7 changes: 6 additions & 1 deletion src/Portal.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default class Portal extends React.Component {
getContainer: PropTypes.func.isRequired,
children: PropTypes.node.isRequired,
didUpdate: PropTypes.func,
}
};

componentDidMount() {
this.createContainer();
Expand Down Expand Up @@ -37,6 +37,11 @@ export default class Portal extends React.Component {

render() {
if (this._container) {
const currentContainer = this.props.getContainer();
if (currentContainer && this._container !== currentContainer) {
this.removeContainer();
this._container = currentContainer;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this._container = currentContainer;
this.removeContainer();
this._container = currentContainer;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要这么写的话,应该要把上次的结点移除掉,不然其实会有 bug.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

恩 是的,测到了。
关于更新的问题,其实PortalWrapper里面也做了相应的处理,在getDerivedStateFromProps中做了判断和移除操作,只不过Portal内没更新,Portal不做更新复用之前的container在性能上确实会好点,看你们判断吧

}
return ReactDOM.createPortal(this.props.children, this._container);
}
return null;
Expand Down
37 changes: 37 additions & 0 deletions tests/container.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import React from 'react';
import { mount } from 'enzyme';
import PortalWrapper from '../src/PortalWrapper';

describe('container', () => {
let div1;
let div2;
beforeAll(() => {
div1 = document.createElement('div');
div1.id = 'dom1';
div2 = document.createElement('div');
div2.id = 'dom2';
document.body.appendChild(div1);
document.body.appendChild(div2);
});
afterAll(() => {
document.body.removeChild(div1);
document.body.removeChild(div2);
});
it('Same function returns different DOM', async () => {
const wrapper = mount(
<PortalWrapper
visible
getContainer={() => document.getElementById('dom1')}
>
{() => <div id="children">Content</div>}
</PortalWrapper>,
);

expect(document.querySelector('#dom1 #children')).not.toBeNull();

wrapper.setProps({ getContainer: () => document.getElementById('dom2') });

expect(document.querySelector('#dom1 #children')).toBeNull();
expect(document.querySelector('#dom2 #children')).not.toBeNull();
});
});