Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove deep import from visx as it causes problems #1820

Merged
merged 2 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brave-schools-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@frontify/fondue-charts": patch
---

fix: remove deep import from visx as it causes problems
4 changes: 2 additions & 2 deletions packages/charts/src/components/LineChart/LineChart.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
}),
}));
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 },
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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] }) => {
Expand All @@ -22,11 +22,11 @@ const getTicksWithEqualSteps = ({ domain }: { domain: [number, number] }) => {
};

describe('getLinearScaleTicks', () => {
let createLinearScaleMock: Mock<any[], any>;
let createScaleMock: Mock<any[], any>;
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(() => {
Expand All @@ -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]);
});
});
Original file line number Diff line number Diff line change
@@ -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],
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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 extends AxisScale>(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;
};
31 changes: 31 additions & 0 deletions packages/charts/src/utils/isValidNumber.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
5 changes: 5 additions & 0 deletions packages/charts/src/utils/isValidNumber.ts
Original file line number Diff line number Diff line change
@@ -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);
};
Loading