From fea06f61a53988357d452c0919560f1cd37e7ec7 Mon Sep 17 00:00:00 2001 From: Samuel Alev Date: Thu, 18 Apr 2024 14:49:10 +0200 Subject: [PATCH 1/2] fix: remove deep import from visx as it causes problems --- .../components/LineChart/LineChart.spec.tsx | 4 +-- .../Tooltip/helpers/getCrosshairBarWidth.ts | 2 +- .../Tooltip/helpers/getTooltipCoordinates.ts | 2 +- .../components/Tooltip/hooks/usePositions.ts | 4 +-- .../helpers/getLinearScaleTicks.spec.ts | 26 ++++++++-------- .../common/helpers/getLinearScaleTicks.ts | 4 +-- .../common/helpers/getScaleBandwith.ts | 11 +++++++ .../charts/src/utils/isValidNumber.spec.ts | 31 +++++++++++++++++++ packages/charts/src/utils/isValidNumber.ts | 5 +++ 9 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 packages/charts/src/components/common/helpers/getScaleBandwith.ts create mode 100644 packages/charts/src/utils/isValidNumber.spec.ts create mode 100644 packages/charts/src/utils/isValidNumber.ts diff --git a/packages/charts/src/components/LineChart/LineChart.spec.tsx b/packages/charts/src/components/LineChart/LineChart.spec.tsx index a49e6db46e..78c80bd87e 100644 --- a/packages/charts/src/components/LineChart/LineChart.spec.tsx +++ b/packages/charts/src/components/LineChart/LineChart.spec.tsx @@ -21,8 +21,8 @@ const dataWithMissingValues = [ name: 'name', }, ]; -vi.mock('@visx/scale/lib/scales/linear', () => ({ - default: () => ({ +vi.mock('@visx/scale', () => ({ + createScale: () => ({ ticks: vi.fn().mockReturnValue([20, 40, 60, 80, 100]), }), })); diff --git a/packages/charts/src/components/common/components/Tooltip/helpers/getCrosshairBarWidth.ts b/packages/charts/src/components/common/components/Tooltip/helpers/getCrosshairBarWidth.ts index da81e36d0b..2a339f067f 100644 --- a/packages/charts/src/components/common/components/Tooltip/helpers/getCrosshairBarWidth.ts +++ b/packages/charts/src/components/common/components/Tooltip/helpers/getCrosshairBarWidth.ts @@ -1,9 +1,9 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ import { type AxisScale } from '@visx/xychart'; -import getScaleBandwidth from '@visx/xychart/lib/utils/getScaleBandwidth'; import { isBandScale } from '@components/BarChart/types'; +import { getScaleBandwidth } from '@components/common/helpers/getScaleBandwith'; export const getCrosshairBarWidth = ( horizontal: boolean, diff --git a/packages/charts/src/components/common/components/Tooltip/helpers/getTooltipCoordinates.ts b/packages/charts/src/components/common/components/Tooltip/helpers/getTooltipCoordinates.ts index fd158e79b2..7582b51a99 100644 --- a/packages/charts/src/components/common/components/Tooltip/helpers/getTooltipCoordinates.ts +++ b/packages/charts/src/components/common/components/Tooltip/helpers/getTooltipCoordinates.ts @@ -1,6 +1,6 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ -import isValidNumber from '@visx/xychart/lib/typeguards/isValidNumber'; +import { isValidNumber } from '../../../../../utils/isValidNumber'; export const getTooltipCoordinates = ( datumCoords: { left?: number; top?: number }, diff --git a/packages/charts/src/components/common/components/Tooltip/hooks/usePositions.ts b/packages/charts/src/components/common/components/Tooltip/hooks/usePositions.ts index 04fb988442..414288d108 100644 --- a/packages/charts/src/components/common/components/Tooltip/hooks/usePositions.ts +++ b/packages/charts/src/components/common/components/Tooltip/hooks/usePositions.ts @@ -1,13 +1,13 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ import { DataContext, TooltipContext, type TooltipContextType } from '@visx/xychart'; -import isValidNumber from '@visx/xychart/lib/typeguards/isValidNumber'; -import getScaleBandwidth from '@visx/xychart/lib/utils/getScaleBandwidth'; import { useCallback, useContext } from 'react'; import { type BarChartDataPoint } from '@components/BarChart'; import { type LineChartDataPoint } from '@components/LineChart'; +import { getScaleBandwidth } from '@components/common/helpers/getScaleBandwith'; +import { isValidNumber } from '../../../../../utils/isValidNumber'; import { xAccessor } from '../../../../LineChart/helpers'; import { getTooltipCoordinates } from '../helpers/getTooltipCoordinates'; diff --git a/packages/charts/src/components/common/helpers/getLinearScaleTicks.spec.ts b/packages/charts/src/components/common/helpers/getLinearScaleTicks.spec.ts index 2a1670cd40..7fd479f1de 100644 --- a/packages/charts/src/components/common/helpers/getLinearScaleTicks.spec.ts +++ b/packages/charts/src/components/common/helpers/getLinearScaleTicks.spec.ts @@ -4,8 +4,8 @@ import { type Mock, afterEach, beforeEach, describe, expect, it, vi } from 'vite import { getLinearScaleTicks } from '@components/common/helpers/getLinearScaleTicks'; -vi.mock('@visx/scale/lib/scales/linear', () => ({ - default: vi.fn(), +vi.mock('@visx/scale', () => ({ + createScale: vi.fn(), })); const getTicksWithEqualSteps = ({ domain }: { domain: [number, number] }) => { @@ -22,11 +22,11 @@ const getTicksWithEqualSteps = ({ domain }: { domain: [number, number] }) => { }; describe('getLinearScaleTicks', () => { - let createLinearScaleMock: Mock; + let createScaleMock: Mock; beforeEach(async () => { - const { default: createLinearScale } = await import('@visx/scale/lib/scales/linear'); - vi.mocked(createLinearScale).mockImplementation(getTicksWithEqualSteps as any); - createLinearScaleMock = vi.mocked(createLinearScale); + const { createScale } = await import('@visx/scale'); + vi.mocked(createScale).mockImplementation(getTicksWithEqualSteps as any); + createScaleMock = vi.mocked(createScale); }); afterEach(() => { @@ -35,43 +35,43 @@ describe('getLinearScaleTicks', () => { it('passes correct values for positive domain', () => { const ticks = getLinearScaleTicks([0, 100], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [0, 100] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [0, 100] }); expect(ticks).toEqual([0, 25, 50, 75, 100]); }); it('passes correct values for positive domain above zero', () => { const ticks = getLinearScaleTicks([10, 100], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [0, 100] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [0, 100] }); expect(ticks).toEqual([0, 25, 50, 75, 100]); }); it('passes correct values for negative domain', () => { const ticks = getLinearScaleTicks([-100, 0], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [-100, 0] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [-100, 0] }); expect(ticks).toEqual([-100, -75, -50, -25, 0]); }); it('passes correct values for negative domain below zero', () => { const ticks = getLinearScaleTicks([-100, -10], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [-100, 0] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [-100, 0] }); expect(ticks).toEqual([-100, -75, -50, -25, 0]); }); it('passes correct values for mixed domain', () => { const ticks = getLinearScaleTicks([-100, 100], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [-100, 100] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [-100, 100] }); expect(ticks).toEqual([-100, -50, 0, 50, 100]); }); it('returns decimal tick values by default', () => { const ticks = getLinearScaleTicks([0, 2], 5); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [0, 2] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [0, 2] }); expect(ticks).toEqual([0, 0.5, 1, 1.5, 2]); }); it('returns only non-decimal tick values when allowDecimalValues is false', () => { const ticks = getLinearScaleTicks([0, 2], 5, false); - expect(createLinearScaleMock).toHaveBeenCalledWith({ domain: [0, 2] }); + expect(createScaleMock).toHaveBeenCalledWith({ type: 'linear', domain: [0, 2] }); expect(ticks).toEqual([0, 1, 2]); }); }); diff --git a/packages/charts/src/components/common/helpers/getLinearScaleTicks.ts b/packages/charts/src/components/common/helpers/getLinearScaleTicks.ts index 96c235c745..2ee6331ac2 100644 --- a/packages/charts/src/components/common/helpers/getLinearScaleTicks.ts +++ b/packages/charts/src/components/common/helpers/getLinearScaleTicks.ts @@ -1,6 +1,6 @@ /* (c) Copyright Frontify Ltd., all rights reserved. */ -import createLinearScale from '@visx/scale/lib/scales/linear'; +import { createScale } from '@visx/scale'; export const getLinearScaleTicks = ( domain: [number, number], @@ -9,7 +9,7 @@ export const getLinearScaleTicks = ( ): number[] => { const min = domain[0] > 0 ? 0 : domain[0]; const max = domain[1] < 0 ? 0 : domain[1]; - const scale = createLinearScale({ domain: [min, max] }); + const scale = createScale({ type: 'linear', domain: [min, max] }); const ticks = scale.ticks(maxNumberOfTicks); return allowDecimalValues ? ticks : getNonDecimalTickValues(ticks); diff --git a/packages/charts/src/components/common/helpers/getScaleBandwith.ts b/packages/charts/src/components/common/helpers/getScaleBandwith.ts new file mode 100644 index 0000000000..685ab7d273 --- /dev/null +++ b/packages/charts/src/components/common/helpers/getScaleBandwith.ts @@ -0,0 +1,11 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { type AxisScale } from '@visx/axis'; + +// As we can't import from sub folder, we inlined the function here +// https://github.com/airbnb/visx/blob/master/packages/visx-xychart/src/utils/getScaleBandwidth.ts +export const getScaleBandwidth = (scale?: Scale) => { + // Broaden type before using 'xxx' in s as typeguard. + const s = scale as AxisScale; + return s && 'bandwidth' in s ? s?.bandwidth() ?? 0 : 0; +}; diff --git a/packages/charts/src/utils/isValidNumber.spec.ts b/packages/charts/src/utils/isValidNumber.spec.ts new file mode 100644 index 0000000000..cba9b96785 --- /dev/null +++ b/packages/charts/src/utils/isValidNumber.spec.ts @@ -0,0 +1,31 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +import { describe, expect, it } from 'vitest'; + +import { isValidNumber } from './isValidNumber'; + +describe('isValidNumber', () => { + it('should return true for valid numbers', () => { + expect(isValidNumber(123)).toBe(true); + expect(isValidNumber(0)).toBe(true); + expect(isValidNumber(-456)).toBe(true); + expect(isValidNumber(3.14)).toBe(true); + }); + + it('should return false for non-number values', () => { + expect(isValidNumber(null)).toBe(false); + expect(isValidNumber(undefined)).toBe(false); + expect(isValidNumber('')).toBe(false); + expect(isValidNumber('123')).toBe(false); + expect(isValidNumber([])).toBe(false); + expect(isValidNumber({})).toBe(false); + expect(isValidNumber(true)).toBe(false); + expect(isValidNumber(false)).toBe(false); + }); + + it('should return false for NaN and Infinity', () => { + expect(isValidNumber(NaN)).toBe(false); + expect(isValidNumber(Infinity)).toBe(false); + expect(isValidNumber(-Infinity)).toBe(false); + }); +}); diff --git a/packages/charts/src/utils/isValidNumber.ts b/packages/charts/src/utils/isValidNumber.ts new file mode 100644 index 0000000000..a183302d47 --- /dev/null +++ b/packages/charts/src/utils/isValidNumber.ts @@ -0,0 +1,5 @@ +/* (c) Copyright Frontify Ltd., all rights reserved. */ + +export const isValidNumber = (value: unknown): value is number => { + return value !== null && typeof value === 'number' && !Number.isNaN(value) && Number.isFinite(value); +}; From 74214f8956a44e2a491bff1e3146d78cf1daef71 Mon Sep 17 00:00:00 2001 From: Samuel Alev Date: Thu, 18 Apr 2024 14:50:35 +0200 Subject: [PATCH 2/2] changeset --- .changeset/brave-schools-grow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/brave-schools-grow.md diff --git a/.changeset/brave-schools-grow.md b/.changeset/brave-schools-grow.md new file mode 100644 index 0000000000..47225c981f --- /dev/null +++ b/.changeset/brave-schools-grow.md @@ -0,0 +1,5 @@ +--- +"@frontify/fondue-charts": patch +--- + +fix: remove deep import from visx as it causes problems