From 4759d335d7c2946ad84910c0437c85f5987409e6 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Mon, 3 Jun 2019 20:00:54 +0200 Subject: [PATCH 1/4] tests: add failing scenario tests --- compat/test/browser/portals.test.js | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/compat/test/browser/portals.test.js b/compat/test/browser/portals.test.js index 2350bbe135a..c79efeadeb9 100644 --- a/compat/test/browser/portals.test.js +++ b/compat/test/browser/portals.test.js @@ -51,6 +51,54 @@ describe('Portal', () => { expect(scratch.innerHTML).to.equal('

Hello

'); }); + it('should toggle the portal', () => { + let toggle; + + function Foo(props) { + const [mounted, setMounted] = useState(true); + toggle = () => setMounted((s) => !s); + return ( +
+

Hello

+ {mounted && createPortal(props.children, scratch)} +
+ ); + } + + render(
foobar
, scratch); + expect(scratch.innerHTML).to.equal('
foobar

Hello

'); + + toggle(); + rerender(); + expect(scratch.innerHTML).to.equal('

Hello

'); + + toggle(); + rerender(); + expect(scratch.innerHTML).to.equal('
foobar

Hello

'); + }); + + it('should notice prop changes on the portal', () => { + let set; + + function Foo(props) { + const [additionalProps, setProps] = useState({ style: { background: 'red' } }); + set = (c) => setProps(c); + return ( +
+

Hello

+ {createPortal(

Foo

, scratch)} +
+ ); + } + + render(, scratch); + expect(scratch.firstChild.style.background).to.equal('red'); + + set({}); + rerender(); + expect(scratch.firstChild.style.background).to.equal(''); + }); + it('should not render for Portal nodes', () => { let root = document.createElement('div'); let dialog = document.createElement('div'); From dce1952fccb3cd094489b7287f28e433045ff889 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 7 Jun 2019 20:43:12 +0200 Subject: [PATCH 2/4] fix: portals should notice prop changes and properly unmount thanks @developit --- compat/src/index.js | 7 +++--- compat/test/browser/portals.test.js | 34 ++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/compat/src/index.js b/compat/src/index.js index a3f131a8877..3ae4249ec47 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -20,9 +20,7 @@ options.event = e => { }; let oldCatchRender = options.catchRender; -options.catchRender = (error, component) => { - return oldCatchRender && oldCatchRender(error, component) || catchRender(error, component); -}; +options.catchRender = (error, component) => oldCatchRender && oldCatchRender(error, component) || catchRender(error, component); /** * Legacy version of createElement. @@ -78,7 +76,8 @@ class ContextProvider { function Portal(props) { let wrap = h(ContextProvider, { context: this.context }, props.vnode); let container = props.container; - hydrate(wrap, container); + hydrate('', container); + render(wrap, container); this.componentWillUnmount = () => { render(null, container); }; diff --git a/compat/test/browser/portals.test.js b/compat/test/browser/portals.test.js index c79efeadeb9..cd85468dcb2 100644 --- a/compat/test/browser/portals.test.js +++ b/compat/test/browser/portals.test.js @@ -1,4 +1,4 @@ -import { createElement as h, render, createPortal, useState } from '../../src'; +import { createElement as h, render, createPortal, useState, Component } from '../../src'; import { setupScratch } from '../../../test/_util/helpers'; import { setupRerender, teardown } from 'preact/test-utils'; /* eslint-disable react/jsx-boolean-value, react/display-name, prefer-arrow-callback */ @@ -99,6 +99,38 @@ describe('Portal', () => { expect(scratch.firstChild.style.background).to.equal(''); }); + it('should not unmount the portal component', () => { + let spy = sinon.spy(); + let set; + class Child extends Component { + componentWillUnmount() { + spy(); + } + + render(props) { + return props.children; + } + } + + function Foo(props) { + const [additionalProps, setProps] = useState({ style: { background: 'red' } }); + set = (c) => setProps(c); + return ( +
+

Hello

+ {createPortal(Foo, scratch)} +
+ ); + } + + render(, scratch); + expect(spy).not.to.be.called; + + set({}); + rerender(); + expect(spy).not.to.be.called; + }); + it('should not render for Portal nodes', () => { let root = document.createElement('div'); let dialog = document.createElement('div'); From 633495f77b4a8181a021d91604d17d4c0923534a Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 7 Jun 2019 21:07:56 +0200 Subject: [PATCH 3/4] fix: only hydrate when mounting --- compat/src/index.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compat/src/index.js b/compat/src/index.js index b180a90c3fc..114ec32c1f7 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -78,8 +78,14 @@ class ContextProvider { function Portal(props) { let wrap = h(ContextProvider, { context: this.context }, props.vnode); let container = props.container; - hydrate('', container); + if (!this.mounted) { + hydrate('', container); + } render(wrap, container); + this.componentDidMount = () => { + this.mounted = true; + }; + this.componentWillUnmount = () => { render(null, container); }; From b3f3f77920d785b2e63e8779ba5740f212bc1432 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sat, 8 Jun 2019 11:08:01 +0200 Subject: [PATCH 4/4] catch reparenting --- compat/src/index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compat/src/index.js b/compat/src/index.js index 114ec32c1f7..7d8585c85bd 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -78,14 +78,13 @@ class ContextProvider { function Portal(props) { let wrap = h(ContextProvider, { context: this.context }, props.vnode); let container = props.container; - if (!this.mounted) { + + if (props.container !== this.container) { hydrate('', container); + this.container = container; } - render(wrap, container); - this.componentDidMount = () => { - this.mounted = true; - }; + render(wrap, container); this.componentWillUnmount = () => { render(null, container); };