fix(shape): conditionally render Arc without data (#937)

* new(shape): conditionally render Arc without data * deps(annotation): fix @visx/text dep
airbnb · Nov 20, 2020 · 9f79dcd · 9f79dcd
1 parent e5adfc1
commit 9f79dcd
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 31 deletions.
diff --git a/packages/visx-annotation/package.json b/packages/visx-annotation/package.json
@@ -37,7 +37,7 @@
     "@visx/group": "1.0.0",
     "@visx/point": "1.0.0",
     "@visx/shape": "1.1.0",
-    "@visx/text": "1.0.0",
+    "@visx/text": "1.1.0",
     "classnames": "^2.2.5",
     "prop-types": "^15.5.10",
     "react-use-measure": "2.0.1"

diff --git a/packages/visx-shape/src/shapes/Arc.tsx b/packages/visx-shape/src/shapes/Arc.tsx
@@ -41,13 +41,21 @@ export default function Arc<Datum>({
 
   // eslint-disable-next-line react/jsx-no-useless-fragment
   if (children) return <>{children({ path })}</>;
-  if (!data) return null;
+  if (
+    !data &&
+    (startAngle == null || endAngle == null || innerRadius == null || outerRadius == null)
+  ) {
+    console.warn(
+      '[@visx/shape/Arc]: expected data because one of startAngle, endAngle, innerRadius, outerRadius is undefined. Bailing.',
+    );
+    return null;
+  }
 
   return (
     <path
       ref={innerRef}
       className={cx('visx-arc', className)}
-      d={path(data) || ''}
+      d={path(data as Datum) || ''}
       {...restProps}
     />
   );

diff --git a/packages/visx-shape/test/Arc.test.tsx b/packages/visx-shape/test/Arc.test.tsx
@@ -1,4 +1,5 @@
 import React from 'react';
+import mockConsole from 'jest-mock-console';
 import { shallow, mount } from 'enzyme';
 
 import { Arc } from '../src';
@@ -22,7 +23,8 @@ const data: Datum = {
   padAngle: 0,
 };
 
-const ArcWrapper = (overrideProps = {}) => shallow(<Arc data={data} {...overrideProps} />);
+const ArcWrapper = (overrideProps: Partial<ArcProps<Datum>> = { data }) =>
+  shallow(<Arc {...overrideProps} />);
 
 const ArcChildren = ({ children, ...restProps }: Partial<ArcProps<Datum>>) =>
   shallow(
@@ -32,166 +34,179 @@ const ArcChildren = ({ children, ...restProps }: Partial<ArcProps<Datum>>) =>
   );
 
 describe('<Arc />', () => {
-  test('it should be defined', () => {
+  it('should be defined', () => {
     expect(Arc).toBeDefined();
   });
 
-  test('it should have the .visx-arcs-group class', () => {
+  it('should have the .visx-arcs-group class', () => {
     expect(ArcWrapper().prop('className')).toBe('visx-arc');
   });
 
-  test('it should contain paths', () => {
-    expect(ArcWrapper().find('path').length).toBeGreaterThan(0);
+  it('should render a path', () => {
+    expect(ArcWrapper().find('path')).toHaveLength(1);
   });
 
-  test('it should take a children as function prop', () => {
+  it('should warn and render null when none of data, radii, and angles are passed', () => {
+    const restoreConsole = mockConsole();
+    expect(ArcWrapper({}).find('path')).toHaveLength(0);
+    expect(console.warn).toHaveBeenCalledTimes(1);
+    restoreConsole();
+  });
+
+  it('should render a path without data when radii + angles are defined', () => {
+    expect(
+      ArcWrapper({ startAngle: 0, endAngle: 6, innerRadius: 5, outerRadius: 10 }).find('path'),
+    ).toHaveLength(1);
+  });
+
+  it('should take a children as function prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn });
     expect(fn).toHaveBeenCalled();
   });
 
-  test('it should call children function with { path }', () => {
+  it('should call children function with { path }', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn });
     const args = fn.mock.calls[0][0];
     const keys = Object.keys(args);
     expect(keys).toContain('path');
   });
 
-  test('it should take an innerRadius number prop', () => {
+  it('should take an innerRadius number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, innerRadius: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.innerRadius()()).toBe(42);
   });
 
-  test('it should take an innerRadius fn prop', () => {
+  it('should take an innerRadius fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, innerRadius: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.innerRadius()()).toBe(42);
   });
 
-  test('it should take an outerRadius number prop', () => {
+  it('should take an outerRadius number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, outerRadius: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.outerRadius()()).toBe(42);
   });
 
-  test('it should take an outerRadius fn prop', () => {
+  it('should take an outerRadius fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, outerRadius: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.outerRadius()()).toBe(42);
   });
 
-  test('it should take a cornerRadius number prop', () => {
+  it('should take a cornerRadius number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, cornerRadius: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.cornerRadius()()).toBe(42);
   });
 
-  test('it should take a cornerRadius fn prop', () => {
+  it('should take a cornerRadius fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, cornerRadius: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.cornerRadius()()).toBe(42);
   });
 
-  test('it should take a startAngle number prop', () => {
+  it('should take a startAngle number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, startAngle: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.startAngle()()).toBe(42);
   });
 
-  test('it should take a startAngle 0 prop', () => {
+  it('should take a startAngle 0 prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, startAngle: 0 });
     const args = fn.mock.calls[0][0];
     expect(args.path.startAngle()()).toBe(0);
   });
 
-  test('it should take a startAngle fn prop', () => {
+  it('should take a startAngle fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, startAngle: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.startAngle()()).toBe(42);
   });
 
-  test('it should take a endAngle number prop', () => {
+  it('should take a endAngle number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, endAngle: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.endAngle()()).toBe(42);
   });
 
-  test('it should take a endAngle 0 prop', () => {
+  it('should take a endAngle 0 prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, endAngle: 0 });
     const args = fn.mock.calls[0][0];
     expect(args.path.endAngle()()).toBe(0);
   });
 
-  test('it should take a endAngle fn prop', () => {
+  it('should take a endAngle fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, endAngle: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.endAngle()()).toBe(42);
   });
 
-  test('it should take a padAngle number prop', () => {
+  it('should take a padAngle number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padAngle: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padAngle()()).toBe(42);
   });
 
-  test('it should take a padAngle 0 prop', () => {
+  it('should take a padAngle 0 prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padAngle: 0 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padAngle()()).toBe(0);
   });
 
-  test('it should take a padAngle fn prop', () => {
+  it('should take a padAngle fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padAngle: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padAngle()()).toBe(42);
   });
 
-  test('it should take a padRadius number prop', () => {
+  it('should take a padRadius number prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padRadius: 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padRadius()()).toBe(42);
   });
 
-  test('it should take a padRadius 0 prop', () => {
+  it('should take a padRadius 0 prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padRadius: 0 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padRadius()()).toBe(0);
   });
 
-  test('it should take a padRadius fn prop', () => {
+  it('should take a padRadius fn prop', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn, padRadius: () => 42 });
     const args = fn.mock.calls[0][0];
     expect(args.path.padRadius()()).toBe(42);
   });
 
-  test('calling path with data returns a string', () => {
+  it('calling path with data returns a string', () => {
     const fn = jest.fn();
     ArcChildren({ children: fn });
     const args = fn.mock.calls[0][0];
     expect(typeof args.path(data)).toBe('string');
   });
 
-  test('it should expose its ref via an innerRef prop', () => {
+  it('should expose its ref via an innerRef prop', () => {
     // eslint-disable-next-line jest/no-test-return-statement
     return new Promise(done => {
       const refCallback = (ref: SVGPathElement) => {

diff --git a/yarn.lock b/yarn.lock
@@ -12116,7 +12116,7 @@ reserved-words@^0.1.2:
   resolved "https://registry.yarnpkg.com/reserved-words/-/reserved-words-0.1.2.tgz#00a0940f98cd501aeaaac316411d9adc52b31ab1"
   integrity sha1-AKCUD5jNUBrqqsMWQR2a3FKzGrE=
 
-resize-observer-polyfill@1.5.1:
+resize-observer-polyfill@1.5.1, resize-observer-polyfill@^1.5.1:
   version "1.5.1"
   resolved "https://registry.yarnpkg.com/resize-observer-polyfill/-/resize-observer-polyfill-1.5.1.tgz#0e9020dd3d21024458d4ebd27e23e40269810464"
   integrity sha512-LwZrotdHOo12nQuZlHEmtuXdqGoOD0OhaxopaNFxWzInpEgaLWoVuAMbTzixuosCx2nEG58ngzW3vxdWoxIgdg==