From 25648a41d5d709f5b43352f78f1bbda659315c08 Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Tue, 30 Apr 2019 10:58:40 -0400 Subject: [PATCH 1/5] fix: allow falsely children in route config --- src/makeRouteConfig.js | 3 +++ test/__snapshots__/makeRouteConfig.test.js.snap | 2 ++ test/makeRouteConfig.test.js | 11 +++++++++++ 3 files changed, 16 insertions(+) diff --git a/src/makeRouteConfig.js b/src/makeRouteConfig.js index 004ed6e8..f1fd6b2c 100644 --- a/src/makeRouteConfig.js +++ b/src/makeRouteConfig.js @@ -3,6 +3,9 @@ import React from 'react'; function buildRouteConfig(node, routeConfig) { React.Children.forEach(node, child => { + // falsely children are turned into `null` by `forEach` + if (child === null) return; + invariant( React.isValidElement(child), '`%s` is not a valid React element', diff --git a/test/__snapshots__/makeRouteConfig.test.js.snap b/test/__snapshots__/makeRouteConfig.test.js.snap index 7275042e..b320001f 100644 --- a/test/__snapshots__/makeRouteConfig.test.js.snap +++ b/test/__snapshots__/makeRouteConfig.test.js.snap @@ -1,3 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`makeRouteConfig should allow empty children 1`] = `"\`,\` is not a valid React element"`; + exports[`makeRouteConfig should error on non-element children 1`] = `"\`,\` is not a valid React element"`; diff --git a/test/makeRouteConfig.test.js b/test/makeRouteConfig.test.js index d62a66d6..0e01f43b 100644 --- a/test/makeRouteConfig.test.js +++ b/test/makeRouteConfig.test.js @@ -202,6 +202,17 @@ describe('makeRouteConfig', () => { }).toThrowErrorMatchingSnapshot(); }); + it('should allow empty children', () => { + expect(() => { + makeRouteConfig( + + {false} + + , + ); + }).not.toThrow(); + }); + ['react-proxy', 'react-stand-in'].forEach(packageName => { it(`should work with proxies from ${packageName}`, () => { // eslint-disable-next-line global-require, import/no-dynamic-require From 7286fe8d38832b13536fe42130fa834357d6266e Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Tue, 30 Apr 2019 11:00:59 -0400 Subject: [PATCH 2/5] nit --- src/makeRouteConfig.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/makeRouteConfig.js b/src/makeRouteConfig.js index f1fd6b2c..30cd093a 100644 --- a/src/makeRouteConfig.js +++ b/src/makeRouteConfig.js @@ -3,8 +3,9 @@ import React from 'react'; function buildRouteConfig(node, routeConfig) { React.Children.forEach(node, child => { - // falsely children are turned into `null` by `forEach` - if (child === null) return; + if (!child) { + return; + } invariant( React.isValidElement(child), From 005fbd2c2f29b17dfcc23447d647f2663e8359ad Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Tue, 30 Apr 2019 11:10:06 -0400 Subject: [PATCH 3/5] oops --- src/makeRouteConfig.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/makeRouteConfig.js b/src/makeRouteConfig.js index 30cd093a..8614821b 100644 --- a/src/makeRouteConfig.js +++ b/src/makeRouteConfig.js @@ -3,7 +3,9 @@ import React from 'react'; function buildRouteConfig(node, routeConfig) { React.Children.forEach(node, child => { - if (!child) { + // Falsy children get coerced to null. We check for this instead of + // implicit falsiness because we don't want to allow empty strings or 0. + if (child === null) { return; } From 47f5e79599688f547ff93e4c3569d477d546eb2e Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Tue, 30 Apr 2019 11:16:45 -0400 Subject: [PATCH 4/5] update snapshot --- test/__snapshots__/makeRouteConfig.test.js.snap | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/__snapshots__/makeRouteConfig.test.js.snap b/test/__snapshots__/makeRouteConfig.test.js.snap index b320001f..7275042e 100644 --- a/test/__snapshots__/makeRouteConfig.test.js.snap +++ b/test/__snapshots__/makeRouteConfig.test.js.snap @@ -1,5 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`makeRouteConfig should allow empty children 1`] = `"\`,\` is not a valid React element"`; - exports[`makeRouteConfig should error on non-element children 1`] = `"\`,\` is not a valid React element"`; From 01eb48c8e605b8188499e847c78af1b9957ca17f Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Tue, 30 Apr 2019 11:17:23 -0400 Subject: [PATCH 5/5] fix tests --- test/__snapshots__/makeRouteConfig.test.js.snap | 4 ++-- test/makeRouteConfig.test.js | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/__snapshots__/makeRouteConfig.test.js.snap b/test/__snapshots__/makeRouteConfig.test.js.snap index b320001f..b6a33444 100644 --- a/test/__snapshots__/makeRouteConfig.test.js.snap +++ b/test/__snapshots__/makeRouteConfig.test.js.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`makeRouteConfig should allow empty children 1`] = `"\`,\` is not a valid React element"`; - exports[`makeRouteConfig should error on non-element children 1`] = `"\`,\` is not a valid React element"`; + +exports[`makeRouteConfig should error on other falsy children 1`] = `"\`0\` is not a valid React element"`; diff --git a/test/makeRouteConfig.test.js b/test/makeRouteConfig.test.js index 0e01f43b..8eaef8cb 100644 --- a/test/makeRouteConfig.test.js +++ b/test/makeRouteConfig.test.js @@ -213,6 +213,17 @@ describe('makeRouteConfig', () => { }).not.toThrow(); }); + it('should error on other falsy children', () => { + expect(() => { + makeRouteConfig( + + {0} + + , + ); + }).toThrowErrorMatchingSnapshot(); + }); + ['react-proxy', 'react-stand-in'].forEach(packageName => { it(`should work with proxies from ${packageName}`, () => { // eslint-disable-next-line global-require, import/no-dynamic-require