From 6444437f11e8732d0b7b48b378cf547206af60a8 Mon Sep 17 00:00:00 2001
From: Haz <hazdiego@gmail.com>
Date: Fri, 27 Mar 2020 21:49:05 -0300
Subject: [PATCH 1/2] Add failing test on slot-fill

---
 .../bubbles-virtually/slot-fill-provider.js   |  3 +-
 .../slot-fill/test/__snapshots__/slot.js.snap |  4 ++
 .../components/src/slot-fill/test/slot.js     | 65 +++++++++++++++++++
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
index 26b4a32c7c600c..aa539a168afba7 100644
--- a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
+++ b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
@@ -25,11 +25,10 @@ function useSlotRegistry() {
 
 	const unregisterSlot = useCallback( ( name, ref ) => {
 		setSlots( ( prevSlots ) => {
-			// eslint-disable-next-line no-unused-vars
 			const { [ name ]: slot, ...nextSlots } = prevSlots;
 			// Make sure we're not unregistering a slot registered by another element
 			// See https://github.com/WordPress/gutenberg/pull/19242#issuecomment-590295412
-			if ( slot?.ref === ref ) {
+			if ( slot.ref === ref ) {
 				return nextSlots;
 			}
 			return prevSlots;
diff --git a/packages/components/src/slot-fill/test/__snapshots__/slot.js.snap b/packages/components/src/slot-fill/test/__snapshots__/slot.js.snap
index 79b43ad4c65c07..c8b4df5c0237de 100644
--- a/packages/components/src/slot-fill/test/__snapshots__/slot.js.snap
+++ b/packages/components/src/slot-fill/test/__snapshots__/slot.js.snap
@@ -26,6 +26,8 @@ Array [
 ]
 `;
 
+exports[`Slot bubblesVirtually false should unmount two slots with the same name 1`] = `"<div data-position=\\"first\\"></div><div data-position=\\"second\\"></div>"`;
+
 exports[`Slot bubblesVirtually true should subsume another slot by the same name 1`] = `
 Array [
   <div
@@ -54,6 +56,8 @@ Array [
 ]
 `;
 
+exports[`Slot bubblesVirtually true should unmount two slots with the same name 1`] = `"<div data-position=\\"first\\"></div><div data-position=\\"second\\"></div>"`;
+
 exports[`Slot should re-render Slot when not bubbling virtually 1`] = `
 Array [
   <div>
diff --git a/packages/components/src/slot-fill/test/slot.js b/packages/components/src/slot-fill/test/slot.js
index db935e6bc2ed29..d168c41da47b43 100644
--- a/packages/components/src/slot-fill/test/slot.js
+++ b/packages/components/src/slot-fill/test/slot.js
@@ -3,6 +3,8 @@
  */
 import { isEmpty } from 'lodash';
 import ReactTestRenderer from 'react-test-renderer';
+import { render, unmountComponentAtNode } from 'react-dom';
+import { act } from 'react-dom/test-utils';
 
 /**
  * Internal dependencies
@@ -36,6 +38,20 @@ class Filler extends Component {
 	}
 }
 
+let container = null;
+beforeEach( () => {
+	// setup a DOM element as a render target
+	container = document.createElement( 'div' );
+	document.body.appendChild( container );
+} );
+
+afterEach( () => {
+	// cleanup on exiting
+	unmountComponentAtNode( container );
+	container.remove();
+	container = null;
+} );
+
 describe( 'Slot', () => {
 	it( 'should render empty Fills', () => {
 		const tree = ReactTestRenderer.create(
@@ -260,6 +276,55 @@ describe( 'Slot', () => {
 
 				expect( testRenderer.toJSON() ).toMatchSnapshot();
 			} );
+
+			it( 'should unmount two slots with the same name', () => {
+				act( () => {
+					render(
+						<Provider>
+							<div data-position="first">
+								<Slot
+									name="egg"
+									bubblesVirtually={ bubblesVirtually }
+								/>
+							</div>
+							<div data-position="second">
+								<Slot
+									name="egg"
+									bubblesVirtually={ bubblesVirtually }
+								/>
+							</div>
+							<Fill name="egg">Content</Fill>
+						</Provider>,
+						container
+					);
+				} );
+				act( () => {
+					render(
+						<Provider>
+							<div data-position="first">
+								<Slot
+									name="egg"
+									bubblesVirtually={ bubblesVirtually }
+								/>
+							</div>
+							<div data-position="second" />
+							<Fill name="egg">Content</Fill>
+						</Provider>,
+						container
+					);
+				} );
+				act( () => {
+					render(
+						<Provider>
+							<div data-position="first" />
+							<div data-position="second" />
+							<Fill name="egg">Content</Fill>
+						</Provider>,
+						container
+					);
+				} );
+				expect( container.innerHTML ).toMatchSnapshot();
+			} );
 		}
 	);
 } );

From 746cbaa33e80670108accb0c89b8657799651b38 Mon Sep 17 00:00:00 2001
From: Haz <hazdiego@gmail.com>
Date: Fri, 27 Mar 2020 21:58:47 -0300
Subject: [PATCH 2/2] Apply #21205 to fix the failing test

---
 .../src/slot-fill/bubbles-virtually/slot-fill-provider.js       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
index aa539a168afba7..6d4b046a6271cf 100644
--- a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
+++ b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js
@@ -28,7 +28,7 @@ function useSlotRegistry() {
 			const { [ name ]: slot, ...nextSlots } = prevSlots;
 			// Make sure we're not unregistering a slot registered by another element
 			// See https://github.com/WordPress/gutenberg/pull/19242#issuecomment-590295412
-			if ( slot.ref === ref ) {
+			if ( slot?.ref === ref ) {
 				return nextSlots;
 			}
 			return prevSlots;