Skip to content

Commit

Permalink
Replace instances of lodash.range with native code (#2760)
Browse files Browse the repository at this point in the history
  • Loading branch information
carbonrobot authored Jan 31, 2024
1 parent c8c2eb2 commit a53059f
Show file tree
Hide file tree
Showing 18 changed files with 190 additions and 67 deletions.
14 changes: 14 additions & 0 deletions .changeset/shiny-cows-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"victory-area": patch
"victory-bar": patch
"victory-candlestick": patch
"victory-core": patch
"victory-errorbar": patch
"victory-histogram": patch
"victory-line": patch
"victory-pie": patch
"victory-scatter": patch
"victory-voronoi": patch
---

Replace instances of lodash.range with equivalent native code
6 changes: 3 additions & 3 deletions packages/victory-area/src/victory-area.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { fireEvent, render, screen } from "@testing-library/react";
import { range } from "lodash";
import React from "react";
import { Area, VictoryArea, VictoryAreaProps } from "victory-area";
import { VictoryChart } from "victory-chart";
import { Helpers } from "victory-core";
import { curveCatmullRom } from "victory-vendor/d3-shape";
import { calculateD3Path } from "../../../test/helpers/svg";

Expand Down Expand Up @@ -115,7 +115,7 @@ describe("components/victory-area", () => {
scale: "linear",
interpolation: "linear",
sortKey: "x",
data: range(5)
data: Helpers.range(5)
// eslint-disable-next-line max-nested-callbacks
.map((i) => ({ x: i, y: i, y0: 0 }))
.reverse(),
Expand Down Expand Up @@ -145,7 +145,7 @@ describe("components/victory-area", () => {
interpolation: "linear",
sortKey: "x",
sortOrder: "descending",
data: range(5)
data: Helpers.range(5)
.map((i) => ({ x: i, y: i, y0: 0 }))
.reverse(),
};
Expand Down
21 changes: 12 additions & 9 deletions packages/victory-bar/src/victory-bar.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import * as React from "react";
import { render, fireEvent, screen } from "@testing-library/react";
import { range } from "lodash";
import { VictoryChart } from "victory-chart";
import { Bar, VictoryBar } from "victory-bar";
import { Helpers } from "victory-core";

import { isBar, getBarHeight } from "../../../test/helpers";

describe("components/victory-bar", () => {
Expand Down Expand Up @@ -70,14 +71,14 @@ describe("components/victory-bar", () => {

describe("rendering data", () => {
it("renders bars for {x, y} shaped data (default)", () => {
const data = range(10).map((i) => ({ x: i, y: i }));
const data = Helpers.range(10).map((i) => ({ x: i, y: i }));
const { container } = render(<VictoryBar data={data} />);
const bars = container.querySelectorAll("path");
expect(bars.length).toEqual(10);
});

it("renders ordered bars when sortKey is passed", () => {
const data = range(5)
const data = Helpers.range(5)
.map((i) => ({ x: i, y: i }))
.reverse();
const { container } = render(<VictoryBar data={data} sortKey="x" />);
Expand All @@ -94,7 +95,7 @@ describe("components/victory-bar", () => {
});

it("renders reverse ordered bars when sortOrder is descending", () => {
const data = range(5)
const data = Helpers.range(5)
.map((i) => ({ x: i, y: i }))
.reverse();
const { container } = render(
Expand All @@ -112,14 +113,16 @@ describe("components/victory-bar", () => {
});

it("renders bars for array-shaped data", () => {
const data = range(20).map((i) => [i, i]);
const data = Helpers.range(20).map((i) => [i, i]);
const { container } = render(<VictoryBar data={data} x={0} y={1} />);
const bars = container.querySelectorAll("path");
expect(bars).toHaveLength(20);
});

it("renders bars for deeply-nested data", () => {
const data = range(8).map((i) => ({ a: { b: [{ x: i, y: i }] } }));
const data = Helpers.range(8).map((i) => ({
a: { b: [{ x: i, y: i }] },
}));
const { container } = render(
<VictoryBar data={data} x="a.b[0].x" y="a.b[0].y" />,
);
Expand All @@ -128,7 +131,7 @@ describe("components/victory-bar", () => {
});

it("renders bars values with null accessor", () => {
const data = range(8);
const data = Helpers.range(8);
const { container } = render(
// @ts-expect-error "'null' is not assignable to 'x'"
<VictoryBar data={data} x={null} y={null} />,
Expand Down Expand Up @@ -249,14 +252,14 @@ describe("components/victory-bar", () => {

describe("accessibility", () => {
it("adds an aria role to each bar in the series", () => {
const data = range(10).map((y, x) => ({ x, y }));
const data = Helpers.range(10).map((y, x) => ({ x, y }));
render(<VictoryBar data={data} />);
const presentationElements = screen.getAllByRole("presentation");
expect(presentationElements).toHaveLength(11); // bars plus container
});

it("applies aria-label and tabIndex to the Bar primitive", () => {
const data = range(5, 11).map((y, x) => ({ y, x }));
const data = Helpers.range(5, 11).map((y, x) => ({ y, x }));
const { container } = render(
<VictoryBar
data={data}
Expand Down
4 changes: 2 additions & 2 deletions packages/victory-candlestick/src/helper-methods.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { range } from "lodash";
import { fromJS } from "immutable";
import { getData, getDomain } from "victory-candlestick/lib/helper-methods";
import { Helpers } from "victory-core";

const immutableGetDataTest = {
createData: (x) => fromJS(x),
Expand All @@ -16,7 +16,7 @@ const getDataTest = {
describe("getData", () => {
it("sorts data by sortKey", () => {
const data = createData(
range(5)
Helpers.range(5)
.map((i) => ({ x: i, open: i, close: i, high: i, low: i }))
.reverse(),
);
Expand Down
16 changes: 8 additions & 8 deletions packages/victory-candlestick/src/victory-candlestick.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { fireEvent, render, screen } from "@testing-library/react";
import { range } from "lodash";
import React from "react";
import { Candle, VictoryCandlestick } from "victory-candlestick";
import { VictoryChart } from "victory-chart";
import { Helpers } from "victory-core";

const MyCandle = () => <div data-testid="my-candle" />;

Expand Down Expand Up @@ -66,7 +66,7 @@ describe("components/victory-candlestick", () => {

describe("rendering data", () => {
it("renders injected points for {x, y} shaped data (default)", () => {
const data = range(5).map((i) => ({
const data = Helpers.range(5).map((i) => ({
x: i,
open: i,
close: i,
Expand All @@ -80,7 +80,7 @@ describe("components/victory-candlestick", () => {
});

it("renders points for {x, y} shaped data (default)", () => {
const data = range(5).map((i) => ({
const data = Helpers.range(5).map((i) => ({
x: i,
open: i,
close: i,
Expand All @@ -93,7 +93,7 @@ describe("components/victory-candlestick", () => {
});

it("renders ordered bars when sortKey is passed", () => {
const data = range(5)
const data = Helpers.range(5)
.map((i) => ({ x: i, open: i, close: i, high: i, low: i }))
.reverse();
const { container } = render(
Expand All @@ -108,7 +108,7 @@ describe("components/victory-candlestick", () => {
});

it("renders reverse ordered bars when sortOrder is descending", () => {
const data = range(5)
const data = Helpers.range(5)
.map((i) => ({ x: i, open: i, close: i, high: i, low: i }))
.reverse();
const { container } = render(
Expand All @@ -123,7 +123,7 @@ describe("components/victory-candlestick", () => {
});

it("renders points for array-shaped data", () => {
const data = range(10).map((i) => [i, i, i, i, i]);
const data = Helpers.range(10).map((i) => [i, i, i, i, i]);
const { container } = render(
<VictoryCandlestick
data={data}
Expand All @@ -139,7 +139,7 @@ describe("components/victory-candlestick", () => {
});

it("renders points for deeply-nested data", () => {
const data = range(20).map((i) => ({
const data = Helpers.range(20).map((i) => ({
a: { b: [{ x: i, open: i, close: i, high: i, low: i }] },
}));
const { container } = render(
Expand All @@ -157,7 +157,7 @@ describe("components/victory-candlestick", () => {
});

it("renders data values with null accessor", () => {
const data = range(10);
const data = Helpers.range(10);
const { container } = render(
// @ts-expect-error "'null' is not assignable to 'x'"
<VictoryCandlestick
Expand Down
1 change: 1 addition & 0 deletions packages/victory-core/src/exports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ describe("victory-core", () => {
"modifyProps": [Function],
"omit": [Function],
"radiansToDegrees": [Function],
"range": [Function],
"reduceChildren": [Function],
"scalePoint": [Function],
},
Expand Down
3 changes: 1 addition & 2 deletions packages/victory-core/src/victory-util/axis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
isObject,
invert,
uniq,
range,
orderBy,
values,
includes,
Expand Down Expand Up @@ -207,7 +206,7 @@ function getTickArray(props) {
if (tickValues && Collection.containsStrings(tickValues)) {
ticks = stringMap
? tickValues.map((tick) => stringMap[tick])
: range(1, tickValues.length + 1);
: Helpers.range(1, tickValues.length + 1);
}
const tickArray = ticks ? uniq(ticks) : getTicksFromFormat();
const buildTickArray = (arr: number[]) => {
Expand Down
10 changes: 10 additions & 0 deletions packages/victory-core/src/victory-util/data.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,16 @@ describe("victory-util/data", () => {
expect(returnData).toEqual(generatedReturn);
});

it("generates a dataset from negative domain", () => {
const generatedReturn = [
{ x: -10, y: 0 },
{ x: 10, y: 10 },
];
const props = { x: "x", y: "y", domain: { x: [-10, 10], y: [0, 10] } };
const returnData = Data.generateData(props);
expect(returnData).toEqual(generatedReturn);
});

it("generates a dataset from domain and samples", () => {
const generatedReturn = [
{ x: 0, y: 0 },
Expand Down
3 changes: 1 addition & 2 deletions packages/victory-core/src/victory-util/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import React from "react";
import {
uniq,
range,
last,
isFunction,
isPlainObject,
Expand Down Expand Up @@ -46,7 +45,7 @@ function generateDataArray(props, axis) {
const domainMax = Math.max(...domain);
const domainMin = Math.min(...domain);
const step = (domainMax - domainMin) / samples;
const values = range(domainMin, domainMax, step);
const values = Helpers.range(domainMin, domainMax, step);
return last(values) === domainMax ? values : values.concat(domainMax);
}

Expand Down
46 changes: 46 additions & 0 deletions packages/victory-core/src/victory-util/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,50 @@ describe("victory-util/helpers", () => {
expect(undefinedAccessor(14)).toEqual(14);
});
});

describe("range", () => {
it("returns an array of integers", () => {
expect(Helpers.range(4)).toEqual([0, 1, 2, 3]);
});

it("returns an array of integers for negative n", () => {
expect(Helpers.range(-4)).toEqual([0, -1, -2, -3]);
});

it("returns an array of integers from start to end", () => {
expect(Helpers.range(1, 5)).toEqual([1, 2, 3, 4]);
});

it("returns an array of integers from negative start to end", () => {
expect(Helpers.range(-5, 5)).toEqual([-5, -4, -3, -2, -1, 0, 1, 2, 3, 4]);
});

it("returns an array of integers from start to negative end", () => {
expect(Helpers.range(5, -5)).toEqual([5, 4, 3, 2, 1, 0, -1, -2, -3, -4]);
});

it("returns an array of integers using an increment", () => {
expect(Helpers.range(0, 20, 5)).toEqual([0, 5, 10, 15]);
});

it("returns an array of integers using an increment and negative start", () => {
expect(Helpers.range(-10, 20, 5)).toEqual([-10, -5, 0, 5, 10, 15]);
});

it("returns an array of numbers from a floating point increment", () => {
expect(Helpers.range(0, 1, 0.2).length).toEqual(5);
});

it("should parse non-integer values", () => {
expect(Helpers.range(4.7)).toEqual([0, 1, 2, 3, 4]);
});

it("should not throw on undefined start value", () => {
expect(Helpers.range(undefined as any)).toEqual([]);
});

it("should not throw on NaN start value", () => {
expect(Helpers.range(NaN as any)).toEqual([]);
});
});
});
31 changes: 31 additions & 0 deletions packages/victory-core/src/victory-util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,37 @@ export function getCurrentAxis(axis, horizontal) {
return horizontal ? otherAxis : axis;
}

/**
* Creates an array of numbers (positive and/or negative) progressing
* from start up to, but not including, end.
* A step of -1 is used if a negative start is specified without an end or step.
* If end is not specified, it's set to start with start then set to 0.
*
* @param start The length of the array to create, or the start value
* @param end [The end value] If this is defined, start is the start value
* @returns An array of the given length
*/
export function range(start: number, end?: number, increment?: number) {
// when the end index is not given, start from 0
const startIndex = end ? start : 0;

// when the end index is not given, the end of the range is the start index
let endIndex = end ? end : start;

// ensure endIndex is not a falsy value
if (!endIndex) endIndex = 0;

const k = endIndex - startIndex; // the value range
const length = Math.abs(k); // the length of the range
const sign = k / length || 1; // the sign of the range (negative or positive)
const inc = increment || 1; // the step size of each increment

// normalize the array length when dealing with floating point values
const arrayLength = Math.max(Math.ceil(length / inc), 0);

return Array.from(Array(arrayLength), (_, i) => startIndex + i * sign * inc);
}

/**
* @param {Array} children: an array of child components
* @param {Function} iteratee: a function with arguments "child", "childName", and "parent"
Expand Down
5 changes: 3 additions & 2 deletions packages/victory-core/src/victory-util/point-path-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint no-magic-numbers: ["error", { "ignore": [0, 1, 2, 2.5, 3] }]*/
import { range } from "lodash";

import * as Helpers from "./helpers";

export function circle(x: number, y: number, size: number) {
return `M ${x}, ${y}
Expand Down Expand Up @@ -110,7 +111,7 @@ export function star(x: number, y: number, size: number) {
const baseSize = 1.35 * size; // eslint-disable-line no-magic-numbers
const angle = Math.PI / 5; // eslint-disable-line no-magic-numbers
// eslint-disable-next-line no-magic-numbers
const starCoords = range(10).map((index) => {
const starCoords = Helpers.range(10).map((index) => {
const length = index % 2 === 0 ? baseSize : baseSize / 2;
return `${length * Math.sin(angle * (index + 1)) + x},
${length * Math.cos(angle * (index + 1)) + y}`;
Expand Down
Loading

0 comments on commit a53059f

Please sign in to comment.