Correctly type props in Victory Primitives (#2695)

.changeset/clean-needles-grow.md

@@ -0,0 +1,5 @@
+---
+"victory-core": patch
+---
+
+Correctly type props in Victory Primitives

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

@@ -1,16 +1,26 @@
-import React from "react";
+import React, { forwardRef } from "react";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryPrimitiveShapeProps } from "./types";
 
-export const Circle = (props: VictoryPrimitiveShapeProps) => {
-  // eslint-disable-next-line react/prop-types
-  const { desc, ...rest } = props;
-  return desc ? (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <circle vectorEffect="non-scaling-stroke" {...rest}>
-      <desc>{desc}</desc>
-    </circle>
-  ) : (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <circle vectorEffect="non-scaling-stroke" {...rest} />
-  );
-};
+export const Circle = forwardRef<SVGCircleElement, VictoryPrimitiveShapeProps>(
+  (props, ref) => {
+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+     * origin conflicts with the SVG element's origin attribute
+     */
+    const { desc, id, tabIndex, origin, ...rest } = props;
+
+    const svgProps: React.SVGProps<SVGCircleElement> = {
+      vectorEffect: "non-scaling-stroke",
+      id: evaluateProp(id, props)?.toString(),
+      tabIndex: evaluateProp(tabIndex, props),
+      ...rest,
+    };
+    return desc ? (
+      <circle {...svgProps} ref={ref}>
+        <desc>{desc}</desc>
+      </circle>
+    ) : (
+      <circle {...svgProps} ref={ref} />
+    );
+  },
+);

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

@@ -1,5 +1,4 @@
 import React from "react";
-import PropTypes from "prop-types";
 import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
 
 export interface ClipPathProps extends VictoryCommonPrimitiveProps {
@@ -9,17 +8,6 @@ export interface ClipPathProps extends VictoryCommonPrimitiveProps {
 
 export const ClipPath = (props: ClipPathProps) => (
   <defs>
-    {
-      // @ts-expect-error FIXME: "id cannot be a number"
-      <clipPath id={props.clipId}>{props.children}</clipPath>
-    }
+    {<clipPath id={props.clipId?.toString()}>{props.children}</clipPath>}
   </defs>
 );
-
-ClipPath.propTypes = {
-  children: PropTypes.oneOfType([
-    PropTypes.arrayOf(PropTypes.node),
-    PropTypes.node,
-  ]),
-  clipId: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
-};

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

@@ -1,16 +1,27 @@
-import React from "react";
+import React, { forwardRef } from "react";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryPrimitiveShapeProps } from "./types";
 
-export const Line = (props: VictoryPrimitiveShapeProps) => {
-  // eslint-disable-next-line react/prop-types
-  const { desc, ...rest } = props;
-  return desc ? (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <line vectorEffect="non-scaling-stroke" {...rest}>
-      <desc>{desc}</desc>
-    </line>
-  ) : (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <line vectorEffect="non-scaling-stroke" {...rest} />
-  );
-};
+export const Line = forwardRef<SVGLineElement, VictoryPrimitiveShapeProps>(
+  (props, ref) => {
+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+     * origin conflicts with the SVG element's origin attribute
+     */
+    const { desc, id, tabIndex, origin, ...rest } = props;
+
+    const svgProps: React.SVGProps<SVGLineElement> = {
+      vectorEffect: "non-scaling-stroke",
+      id: evaluateProp(id, props)?.toString(),
+      tabIndex: evaluateProp(tabIndex, props),
+      ...rest,
+    };
+
+    return desc ? (
+      <line {...svgProps} ref={ref}>
+        <desc>{desc}</desc>
+      </line>
+    ) : (
+      <line {...svgProps} ref={ref} />
+    );
+  },
+);

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

@@ -1,20 +1,26 @@
 import React, { forwardRef } from "react";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryPrimitiveShapeProps } from "./types";
 
-// eslint-disable-next-line prefer-arrow-callback
-export const Path = forwardRef(function Path(
-  props: VictoryPrimitiveShapeProps,
-  ref,
-) {
-  // eslint-disable-next-line react/prop-types
-  const { desc, ...rest } = props;
-  return desc ? (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <path {...rest} ref={ref}>
-      <desc>{desc}</desc>
-    </path>
-  ) : (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <path {...rest} ref={ref} />
-  );
-});
+export const Path = forwardRef<SVGPathElement, VictoryPrimitiveShapeProps>(
+  (props, ref) => {
+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+     * origin conflicts with the SVG element's origin attribute
+     */
+    const { desc, id, tabIndex, origin, ...rest } = props;
+
+    const svgProps: React.SVGProps<SVGPathElement> = {
+      id: evaluateProp(id, props)?.toString(),
+      tabIndex: evaluateProp(tabIndex, props),
+      ...rest,
+    };
+
+    return desc ? (
+      <path {...svgProps} ref={ref}>
+        <desc>{desc}</desc>
+      </path>
+    ) : (
+      <path {...svgProps} ref={ref} />
+    );
+  },
+);

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

@@ -1,16 +1,27 @@
-import React from "react";
+import React, { forwardRef } from "react";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryPrimitiveShapeProps } from "./types";
 
-export const Rect = (props: VictoryPrimitiveShapeProps) => {
-  // eslint-disable-next-line react/prop-types
-  const { desc, ...rest } = props;
-  return desc ? (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <rect vectorEffect="non-scaling-stroke" {...rest}>
-      <desc>{desc}</desc>
-    </rect>
-  ) : (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <rect vectorEffect="non-scaling-stroke" {...rest} />
-  );
-};
+export const Rect = forwardRef<SVGRectElement, VictoryPrimitiveShapeProps>(
+  (props, ref) => {
+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+     * origin conflicts with the SVG element's origin attribute
+     */
+    const { desc, id, tabIndex, origin, ...rest } = props;
+
+    const svgProps: React.SVGProps<SVGRectElement> = {
+      vectorEffect: "non-scaling-stroke",
+      id: evaluateProp(id, props)?.toString(),
+      tabIndex: evaluateProp(tabIndex, props),
+      ...rest,
+    };
+
+    return desc ? (
+      <rect {...svgProps} ref={ref}>
+        <desc>{desc}</desc>
+      </rect>
+    ) : (
+      <rect {...svgProps} ref={ref} />
+    );
+  },
+);

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

@@ -1,5 +1,5 @@
 import React from "react";
-import PropTypes from "prop-types";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
 
 export interface TextProps extends VictoryCommonPrimitiveProps {
@@ -9,19 +9,22 @@ export interface TextProps extends VictoryCommonPrimitiveProps {
 }
 
 export const Text = (props: TextProps) => {
-  const { children, title, desc, ...rest } = props;
+  /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+   * origin conflicts with the SVG element's origin attribute
+   */
+  const { children, desc, id, origin, tabIndex, title, ...rest } = props;
+
+  const svgProps: React.SVGProps<SVGTextElement> = {
+    id: evaluateProp(id, props)?.toString(),
+    tabIndex: evaluateProp(tabIndex, props),
+    ...rest,
+  };
+
   return (
-    // @ts-expect-error FIXME: "id cannot be a number"
-    <text {...rest}>
+    <text {...svgProps}>
       {title && <title>{title}</title>}
       {desc && <desc>{desc}</desc>}
       {children}
     </text>
   );
 };
-
-Text.propTypes = {
-  children: PropTypes.node,
-  desc: PropTypes.string,
-  title: PropTypes.string,
-};

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

@@ -1,7 +1,18 @@
 import React from "react";
+import { evaluateProp } from "../victory-util/helpers";
 import { VictoryCommonPrimitiveProps } from "../victory-util/common-props";
 
-export const TSpan = (props: VictoryCommonPrimitiveProps) => (
-  // @ts-expect-error FIXME: "id cannot be a number"
-  <tspan {...props} />
-);
+export const TSpan = (props: VictoryCommonPrimitiveProps) => {
+  /* eslint-disable-next-line @typescript-eslint/no-unused-vars --
+   * origin conflicts with the SVG element's origin attribute
+   */
+  const { desc, id, tabIndex, origin, ...rest } = props;
+
+  const svgProps: React.SVGProps<SVGTSpanElement> = {
+    id: evaluateProp(id, props)?.toString(),
+    tabIndex: evaluateProp(tabIndex, props),
+    ...rest,
+  };
+
+  return <tspan {...svgProps} />;
+};

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

@@ -1,4 +1,5 @@
 import * as React from "react";
+import { evaluateProp } from "./helpers";
 
 /*
   USER_PROPS_SAFELIST is to contain any string deemed safe for user props.
@@ -54,19 +55,6 @@ const testIfSafeProp = (key: string): key is SafeAttribute => {
   return false;
 };
 
-/**
- * Gets the value from props if a function value is provided
- * @param {any} value: maybe function value
- * @param {Object} props: props object
- * @returns {any} newValue
- */
-const getValue = (value, props) => {
-  if (typeof value === "function") {
-    return value(props);
-  }
-  return value;
-};
-
 /**
  * getSafeUserProps - function that takes in a props object and removes any
  * key-value entries that do not match filter strings in the USER_PROPS_SAFELIST
@@ -83,7 +71,7 @@ export const getSafeUserProps = <T>(
     Object.entries(propsToFilter)
       .filter(([key]) => testIfSafeProp(key))
       .map(([key, value]) => {
-        return [key, getValue(value, props)];
+        return [key, evaluateProp(value, props)];
       }),
   );
 };