Skip to content

Commit 4b3b417

Browse files
carbonrobotcarbonrobot
authored
Correctly type props in Victory Primitives (#2695)
1 parent 238972d commit 4b3b417

File tree

9 files changed

+133
-100
lines changed

9 files changed

+133
-100
lines changed

‎.changeset/clean-needles-grow.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"victory-core": patch
3+
---
4+
5+
Correctly type props in Victory Primitives

‎packages/victory-core/src/victory-primitives/circle.tsx

+24-14
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
1-
import React from "react";
1+
import React, { forwardRef } from "react";
2+
import { evaluateProp } from "../victory-util/helpers";
23
import { VictoryPrimitiveShapeProps } from "./types";
34

4-
export const Circle = (props: VictoryPrimitiveShapeProps) => {
5-
// eslint-disable-next-line react/prop-types
6-
const { desc, ...rest } = props;
7-
return desc ? (
8-
// @ts-expect-error FIXME: "id cannot be a number"
9-
<circle vectorEffect="non-scaling-stroke" {...rest}>
10-
<desc>{desc}</desc>
11-
</circle>
12-
) : (
13-
// @ts-expect-error FIXME: "id cannot be a number"
14-
<circle vectorEffect="non-scaling-stroke" {...rest} />
15-
);
16-
};
5+
export const Circle = forwardRef<SVGCircleElement, VictoryPrimitiveShapeProps>(
6+
(props, ref) => {
7+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
8+
* origin conflicts with the SVG element's origin attribute
9+
*/
10+
const { desc, id, tabIndex, origin, ...rest } = props;
11+
12+
const svgProps: React.SVGProps<SVGCircleElement> = {
13+
vectorEffect: "non-scaling-stroke",
14+
id: evaluateProp(id, props)?.toString(),
15+
tabIndex: evaluateProp(tabIndex, props),
16+
...rest,
17+
};
18+
return desc ? (
19+
<circle {...svgProps} ref={ref}>
20+
<desc>{desc}</desc>
21+
</circle>
22+
) : (
23+
<circle {...svgProps} ref={ref} />
24+
);
25+
},
26+
);

‎packages/victory-core/src/victory-primitives/clip-path.tsx

+1-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import React from "react";
2-
import PropTypes from "prop-types";
32
import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
43

54
export interface ClipPathProps extends VictoryCommonPrimitiveProps {
@@ -9,17 +8,6 @@ export interface ClipPathProps extends VictoryCommonPrimitiveProps {
98

109
export const ClipPath = (props: ClipPathProps) => (
1110
<defs>
12-
{
13-
// @ts-expect-error FIXME: "id cannot be a number"
14-
<clipPath id={props.clipId}>{props.children}</clipPath>
15-
}
11+
{<clipPath id={props.clipId?.toString()}>{props.children}</clipPath>}
1612
</defs>
1713
);
18-
19-
ClipPath.propTypes = {
20-
children: PropTypes.oneOfType([
21-
PropTypes.arrayOf(PropTypes.node),
22-
PropTypes.node,
23-
]),
24-
clipId: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
25-
};

‎packages/victory-core/src/victory-primitives/line.tsx

+25-14
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
1-
import React from "react";
1+
import React, { forwardRef } from "react";
2+
import { evaluateProp } from "../victory-util/helpers";
23
import { VictoryPrimitiveShapeProps } from "./types";
34

4-
export const Line = (props: VictoryPrimitiveShapeProps) => {
5-
// eslint-disable-next-line react/prop-types
6-
const { desc, ...rest } = props;
7-
return desc ? (
8-
// @ts-expect-error FIXME: "id cannot be a number"
9-
<line vectorEffect="non-scaling-stroke" {...rest}>
10-
<desc>{desc}</desc>
11-
</line>
12-
) : (
13-
// @ts-expect-error FIXME: "id cannot be a number"
14-
<line vectorEffect="non-scaling-stroke" {...rest} />
15-
);
16-
};
5+
export const Line = forwardRef<SVGLineElement, VictoryPrimitiveShapeProps>(
6+
(props, ref) => {
7+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
8+
* origin conflicts with the SVG element's origin attribute
9+
*/
10+
const { desc, id, tabIndex, origin, ...rest } = props;
11+
12+
const svgProps: React.SVGProps<SVGLineElement> = {
13+
vectorEffect: "non-scaling-stroke",
14+
id: evaluateProp(id, props)?.toString(),
15+
tabIndex: evaluateProp(tabIndex, props),
16+
...rest,
17+
};
18+
19+
return desc ? (
20+
<line {...svgProps} ref={ref}>
21+
<desc>{desc}</desc>
22+
</line>
23+
) : (
24+
<line {...svgProps} ref={ref} />
25+
);
26+
},
27+
);

‎packages/victory-core/src/victory-primitives/path.tsx

+23-17
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,26 @@
11
import React, { forwardRef } from "react";
2+
import { evaluateProp } from "../victory-util/helpers";
23
import { VictoryPrimitiveShapeProps } from "./types";
34

4-
// eslint-disable-next-line prefer-arrow-callback
5-
export const Path = forwardRef(function Path(
6-
props: VictoryPrimitiveShapeProps,
7-
ref,
8-
) {
9-
// eslint-disable-next-line react/prop-types
10-
const { desc, ...rest } = props;
11-
return desc ? (
12-
// @ts-expect-error FIXME: "id cannot be a number"
13-
<path {...rest} ref={ref}>
14-
<desc>{desc}</desc>
15-
</path>
16-
) : (
17-
// @ts-expect-error FIXME: "id cannot be a number"
18-
<path {...rest} ref={ref} />
19-
);
20-
});
5+
export const Path = forwardRef<SVGPathElement, VictoryPrimitiveShapeProps>(
6+
(props, ref) => {
7+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
8+
* origin conflicts with the SVG element's origin attribute
9+
*/
10+
const { desc, id, tabIndex, origin, ...rest } = props;
11+
12+
const svgProps: React.SVGProps<SVGPathElement> = {
13+
id: evaluateProp(id, props)?.toString(),
14+
tabIndex: evaluateProp(tabIndex, props),
15+
...rest,
16+
};
17+
18+
return desc ? (
19+
<path {...svgProps} ref={ref}>
20+
<desc>{desc}</desc>
21+
</path>
22+
) : (
23+
<path {...svgProps} ref={ref} />
24+
);
25+
},
26+
);

‎packages/victory-core/src/victory-primitives/rect.tsx

+25-14
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
1-
import React from "react";
1+
import React, { forwardRef } from "react";
2+
import { evaluateProp } from "../victory-util/helpers";
23
import { VictoryPrimitiveShapeProps } from "./types";
34

4-
export const Rect = (props: VictoryPrimitiveShapeProps) => {
5-
// eslint-disable-next-line react/prop-types
6-
const { desc, ...rest } = props;
7-
return desc ? (
8-
// @ts-expect-error FIXME: "id cannot be a number"
9-
<rect vectorEffect="non-scaling-stroke" {...rest}>
10-
<desc>{desc}</desc>
11-
</rect>
12-
) : (
13-
// @ts-expect-error FIXME: "id cannot be a number"
14-
<rect vectorEffect="non-scaling-stroke" {...rest} />
15-
);
16-
};
5+
export const Rect = forwardRef<SVGRectElement, VictoryPrimitiveShapeProps>(
6+
(props, ref) => {
7+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
8+
* origin conflicts with the SVG element's origin attribute
9+
*/
10+
const { desc, id, tabIndex, origin, ...rest } = props;
11+
12+
const svgProps: React.SVGProps<SVGRectElement> = {
13+
vectorEffect: "non-scaling-stroke",
14+
id: evaluateProp(id, props)?.toString(),
15+
tabIndex: evaluateProp(tabIndex, props),
16+
...rest,
17+
};
18+
19+
return desc ? (
20+
<rect {...svgProps} ref={ref}>
21+
<desc>{desc}</desc>
22+
</rect>
23+
) : (
24+
<rect {...svgProps} ref={ref} />
25+
);
26+
},
27+
);

‎packages/victory-core/src/victory-primitives/text.tsx

+13-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from "react";
2-
import PropTypes from "prop-types";
2+
import { evaluateProp } from "../victory-util/helpers";
33
import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
44

55
export interface TextProps extends VictoryCommonPrimitiveProps {
@@ -9,19 +9,22 @@ export interface TextProps extends VictoryCommonPrimitiveProps {
99
}
1010

1111
export const Text = (props: TextProps) => {
12-
const { children, title, desc, ...rest } = props;
12+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
13+
* origin conflicts with the SVG element's origin attribute
14+
*/
15+
const { children, desc, id, origin, tabIndex, title, ...rest } = props;
16+
17+
const svgProps: React.SVGProps<SVGTextElement> = {
18+
id: evaluateProp(id, props)?.toString(),
19+
tabIndex: evaluateProp(tabIndex, props),
20+
...rest,
21+
};
22+
1323
return (
14-
// @ts-expect-error FIXME: "id cannot be a number"
15-
<text {...rest}>
24+
<text {...svgProps}>
1625
{title && <title>{title}</title>}
1726
{desc && <desc>{desc}</desc>}
1827
{children}
1928
</text>
2029
);
2130
};
22-
23-
Text.propTypes = {
24-
children: PropTypes.node,
25-
desc: PropTypes.string,
26-
title: PropTypes.string,
27-
};

‎packages/victory-core/src/victory-primitives/tspan.tsx

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
import React from "react";
2+
import { evaluateProp } from "../victory-util/helpers";
23
import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
34

4-
export const TSpan = (props: VictoryCommonPrimitiveProps) => (
5-
// @ts-expect-error FIXME: "id cannot be a number"
6-
<tspan {...props} />
7-
);
5+
export const TSpan = (props: VictoryCommonPrimitiveProps) => {
6+
/* eslint-disable-next-line @typescript-eslint/no-unused-vars --
7+
* origin conflicts with the SVG element's origin attribute
8+
*/
9+
const { desc, id, tabIndex, origin, ...rest } = props;
10+
11+
const svgProps: React.SVGProps<SVGTSpanElement> = {
12+
id: evaluateProp(id, props)?.toString(),
13+
tabIndex: evaluateProp(tabIndex, props),
14+
...rest,
15+
};
16+
17+
return <tspan {...svgProps} />;
18+
};

‎packages/victory-core/src/victory-util/user-props.ts

+2-14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as React from "react";
2+
import { evaluateProp } from "./helpers";
23

34
/*
45
USER_PROPS_SAFELIST is to contain any string deemed safe for user props.
@@ -54,19 +55,6 @@ const testIfSafeProp = (key: string): key is SafeAttribute => {
5455
return false;
5556
};
5657

57-
/**
58-
* Gets the value from props if a function value is provided
59-
* @param {any} value: maybe function value
60-
* @param {Object} props: props object
61-
* @returns {any} newValue
62-
*/
63-
const getValue = (value, props) => {
64-
if (typeof value === "function") {
65-
return value(props);
66-
}
67-
return value;
68-
};
69-
7058
/**
7159
* getSafeUserProps - function that takes in a props object and removes any
7260
* key-value entries that do not match filter strings in the USER_PROPS_SAFELIST
@@ -83,7 +71,7 @@ export const getSafeUserProps = <T>(
8371
Object.entries(propsToFilter)
8472
.filter(([key]) => testIfSafeProp(key))
8573
.map(([key, value]) => {
86-
return [key, getValue(value, props)];
74+
return [key, evaluateProp(value, props)];
8775
}),
8876
);
8977
};

0 commit comments

Comments
 (0)