From e903a8223b15967a34f7a70a74356fbcffbfccd7 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Tue, 28 May 2024 09:33:21 +0100 Subject: [PATCH] fix(xychart): strip legacy react events from exhaustive prop lists in type declarations The event names `onPointerEnterCapture` and `onPointerLeaveCapture` are erroneously included in the types for react 16 and 17 - these have never been valid event names, but the decision has been taken to leave them in place based on the definitely-typed backporting policy. However, these event names _have_ been removed from the types for React 18. In a few places in xycharts, properties are removed from components with DOM element props using the `Omit` utility type. When the declarations are generated, these `Omit`s are transformed into `Pick`s with the inverted set of keys. This results in all known properties for these components (including `onPointerEnterCapture` and `onPointerLeaveCapture`) being included. This causes compilation errors for consumers of this package that use React 18. This commit proposes a heavily targeted fix by including the removed event names in the string unions passed to `Omit`. This is somewhat distasteful, because it only resolves the issue for existing occurrences - any new places in the codebase with this pattern will need to be specifically handled. Possible future improvements: 1. Test the type declarations in this library against the React 18 types in CI to ensure we handle future occurrences of this pattern. 2. Find a general solution to this. The only possibility I can come up with is patching the react type declarations used in this library to remove these events - since they aren't valid anyway, it should be fine to remove them. --- .../components/series/private/BaseAreaSeries.tsx | 13 +++++++++---- .../components/series/private/BaseLineSeries.tsx | 11 ++++++++--- packages/visx-xychart/src/types/index.ts | 1 + packages/visx-xychart/src/types/util.ts | 12 ++++++++++++ 4 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 packages/visx-xychart/src/types/util.ts diff --git a/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx index dae620afbc..0dd47c2ff5 100644 --- a/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseAreaSeries.tsx @@ -3,7 +3,7 @@ import { AxisScale } from '@visx/axis'; import Area, { AreaProps } from '@visx/shape/lib/shapes/Area'; import LinePath, { LinePathProps } from '@visx/shape/lib/shapes/LinePath'; import DataContext from '../../../context/DataContext'; -import { GlyphsProps, SeriesProps } from '../../../types'; +import { GlyphsProps, SeriesProps, LegacyReactEvents } from '../../../types'; import withRegisteredData, { WithRegisteredDataProps } from '../../../enhancers/withRegisteredData'; import getScaledValueFactory from '../../../utils/getScaledValueFactory'; import getScaleBaseline from '../../../utils/getScaleBaseline'; @@ -29,11 +29,16 @@ export type BaseAreaSeriesProps< /** Props to be passed to the Line, if rendered. */ lineProps?: Omit< LinePathProps & React.SVGProps, - 'data' | 'x' | 'y' | 'children' | 'defined' + 'data' | 'x' | 'y' | 'children' | 'defined' | LegacyReactEvents >; /** Rendered component which is passed path props by BaseAreaSeries after processing. */ - PathComponent?: React.FC, 'ref'>> | 'path'; -} & Omit, 'x' | 'y' | 'x0' | 'x1' | 'y0' | 'y1' | 'ref'>; + PathComponent?: + | React.FC, 'ref' | LegacyReactEvents>> + | 'path'; +} & Omit< + React.SVGProps, + 'x' | 'y' | 'x0' | 'x1' | 'y0' | 'y1' | 'ref' | LegacyReactEvents + >; function BaseAreaSeries({ PathComponent = 'path', diff --git a/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx b/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx index 77d5ad60c7..a61ad9b113 100644 --- a/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx +++ b/packages/visx-xychart/src/components/series/private/BaseLineSeries.tsx @@ -2,7 +2,7 @@ import React, { useContext, useCallback, useMemo } from 'react'; import LinePath, { LinePathProps } from '@visx/shape/lib/shapes/LinePath'; import { AxisScale } from '@visx/axis'; import DataContext from '../../../context/DataContext'; -import { GlyphsProps, SeriesProps } from '../../../types'; +import { GlyphsProps, LegacyReactEvents, SeriesProps } from '../../../types'; import withRegisteredData, { WithRegisteredDataProps } from '../../../enhancers/withRegisteredData'; import getScaledValueFactory from '../../../utils/getScaledValueFactory'; import isValidNumber from '../../../typeguards/isValidNumber'; @@ -17,12 +17,17 @@ export type BaseLineSeriesProps< Datum extends object, > = SeriesProps & { /** Rendered component which is passed path props by BaseLineSeries after processing. */ - PathComponent?: React.FC, 'ref'>> | 'path'; + PathComponent?: + | React.FC, 'ref' | LegacyReactEvents>> + | 'path'; /** Sets the curve factory (from @visx/curve or d3-curve) for the line generator. Defaults to curveLinear. */ curve?: LinePathProps['curve']; /** Given a datakey, returns its color. Falls back to theme color if unspecified or if a null-ish value is returned. */ colorAccessor?: (dataKey: string) => string | undefined | null; -} & Omit, 'x' | 'y' | 'x0' | 'x1' | 'y0' | 'y1' | 'ref'>; +} & Omit< + React.SVGProps, + 'x' | 'y' | 'x0' | 'x1' | 'y0' | 'y1' | 'ref' | LegacyReactEvents + >; function BaseLineSeries({ colorAccessor, diff --git a/packages/visx-xychart/src/types/index.ts b/packages/visx-xychart/src/types/index.ts index c0260032ab..9ecbea04b6 100644 --- a/packages/visx-xychart/src/types/index.ts +++ b/packages/visx-xychart/src/types/index.ts @@ -4,3 +4,4 @@ export * from './event'; export * from './series'; export * from './theme'; export * from './tooltip'; +export * from './util'; diff --git a/packages/visx-xychart/src/types/util.ts b/packages/visx-xychart/src/types/util.ts new file mode 100644 index 0000000000..84e0463aeb --- /dev/null +++ b/packages/visx-xychart/src/types/util.ts @@ -0,0 +1,12 @@ +/** + * The typings for React 16 and 17 erroneously include events named + * `onPointerEnterCapture` and `onPointerLeaveCapture`. When type declarations + * are generated for this library, these events are included in + * exhaustive lists of properties through inlining of the `Omit` utility as an + * invested `Pick`. This causes type errors in React 18, where these event names + * are not valid. + * + * To work around this, wrap any such type in this helper, which will strip off + * the invalid event names. + */ +export type LegacyReactEvents = 'onPointerEnterCapture' | 'onPointerLeaveCapture';