From 53da43ea9b1ad4d092cfa08e7b0f3b73b98347dd Mon Sep 17 00:00:00 2001
From: Grzegorz Ziolkowski <grzegorz@gziolo.pl>
Date: Mon, 27 Aug 2018 13:10:54 +0200
Subject: [PATCH] Components: Improve empty elements filters in Slot
 implementation

---
 packages/components/src/slot-fill/slot.js     | 22 +++++++++++++---
 .../components/src/slot-fill/test/utils.js    | 26 +++++++++++++++++++
 packages/components/src/slot-fill/utils.js    | 19 ++++++++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 packages/components/src/slot-fill/test/utils.js
 create mode 100644 packages/components/src/slot-fill/utils.js

diff --git a/packages/components/src/slot-fill/slot.js b/packages/components/src/slot-fill/slot.js
index cf6a51cb1ca188..9d927253a133b8 100644
--- a/packages/components/src/slot-fill/slot.js
+++ b/packages/components/src/slot-fill/slot.js
@@ -1,13 +1,24 @@
 /**
  * External dependencies
  */
-import { noop, map, isString, isFunction } from 'lodash';
+import {
+	isFunction,
+	isString,
+	map,
+	negate,
+	noop,
+} from 'lodash';
 
 /**
  * WordPress dependencies
  */
 import { Component, Children, cloneElement } from '@wordpress/element';
 
+/**
+ * Internal dependencies
+ */
+import { isEmptyElement } from './utils';
+
 class Slot extends Component {
 	constructor() {
 		super( ...arguments );
@@ -64,11 +75,16 @@ class Slot extends Component {
 				const childKey = `${ fillKey }---${ child.key || childIndex }`;
 				return cloneElement( child, { key: childKey } );
 			} );
-		} );
+		} ).filter(
+			// In some cases fills are rendered only when some conditions apply.
+			// This ensures that we only use non-empty fills when rendering, i.e.,
+			// it allows us to render wrappers only when the fills are actually present.
+			negate( isEmptyElement )
+		);
 
 		return (
 			<div ref={ this.bindNode } role="presentation">
-				{ isFunction( children ) ? children( fills.filter( Boolean ) ) : fills }
+				{ isFunction( children ) ? children( fills ) : fills }
 			</div>
 		);
 	}
diff --git a/packages/components/src/slot-fill/test/utils.js b/packages/components/src/slot-fill/test/utils.js
new file mode 100644
index 00000000000000..0a4320acbfc00d
--- /dev/null
+++ b/packages/components/src/slot-fill/test/utils.js
@@ -0,0 +1,26 @@
+/**
+ * WordPress dependencies
+ */
+import { createElement } from '@wordpress/element';
+
+/**
+ * Internal dependencies
+ */
+import { isEmptyElement } from '../utils';
+
+describe( 'isEmptyElement', () => {
+	test( 'should be empty', () => {
+		expect( isEmptyElement( undefined ) ).toBe( true );
+		expect( isEmptyElement( false ) ).toBe( true );
+		expect( isEmptyElement( '' ) ).toBe( true );
+		expect( isEmptyElement( [] ) ).toBe( true );
+	} );
+
+	test( 'should not be empty', () => {
+		expect( isEmptyElement( 0 ) ).toBe( false );
+		expect( isEmptyElement( 100 ) ).toBe( false );
+		expect( isEmptyElement( 'test' ) ).toBe( false );
+		expect( isEmptyElement( createElement( 'div' ) ) ).toBe( false );
+		expect( isEmptyElement( [ 'x' ] ) ).toBe( false );
+	} );
+} );
diff --git a/packages/components/src/slot-fill/utils.js b/packages/components/src/slot-fill/utils.js
new file mode 100644
index 00000000000000..c97346e601cabb
--- /dev/null
+++ b/packages/components/src/slot-fill/utils.js
@@ -0,0 +1,19 @@
+/**
+ * External dependencies
+ */
+import {
+	isArray,
+	isNumber,
+} from 'lodash';
+
+export const isEmptyElement = ( element ) => {
+	if ( isNumber( element ) ) {
+		return false;
+	}
+
+	if ( isArray( element ) ) {
+		return ! element.length;
+	}
+
+	return ! element;
+};