Layer: TypeScript error with latest `@types/react` version

[Lukas742]

Layer: TypeScript error with latest `@types/react` version

[ckifer]

Looks like the xyzCapture events were removed in react types in 18.x because they don't do anything.

Relevant react issue
facebook/react#17883

Our generated declaration files include these removed events which breaks you with skipLibCheck: true.

We'll upgrade react types in 3.x and this shouldn't be an issue anymore, but probably won't do it in 2.x for risk of breaking something else.

Thanks!

CC: @PavelVanecek

[Lukas742]

Thanks @ckifer!

I did a bit more digging and came to the following result, maybe it helps:

Typically, type discrepancies shouldn't be an issue, as long as only the imported types are used, allowing consumers to define the version of, e.g. @types/react on their end. In this case, it appears that the forwardRef types are transformed to leverage the SVGProps' keys for the Pick utility type. Consequently, these keys are bundled in with the @types/react version used in this project. If a type from SVGProps (or any interface it extends) is removed, the described error is thrown.

[HHogg]

Yeah that makes sense @ckifer, thanks for taking a look at this. I've had a quick look and here's my thinking. The SVGProps interface...

interface SVGProps<T> extends SVGAttributes<T>, ClassAttributes<T>

... is SVGAttributes anyway with ClassAttributes, which is

interface ClassAttributes<T> extends Attributes {
  ref?: LegacyRef<T> | undefined;
}

interface Attributes {
  key?: Key | null | undefined;
}

... so the ref and key which aren't part of SVGAttributes need to be added, and are added implicitly from createElement. and you can see that Attributes and ClassAttributes are added anyway, which is why key and ref are always available in your IDE even when you don't specify it.

const MyComponent = () => null;
const Foo = () => <MyComponent key="IT_WORKS" />

const MyComponentForwarded = forwardRef(() => null);
const Bar = () => <MyComponentForwarded key="IT_WORKS" ref={useRef()} />;

[ckifer]

Lmk if this looks okay #4413
(fixing some related types while I'm at it, not sure why Pie was an HTMLElement...)

[ckifer]

Thanks for the sandbox, I can reproduce.

Generally skipLibCheck is recommended so that things like this do not break you - otherwise I think everything works fine, this type and file has not changed in a long time.

Either way, we'll use this to track the issue and hopefully be able to resolve it

[dkilgore-eightfold]

I get the same linter error for placeholder and onPointerLeaveCapture (in addition to the onPointerEnterCapture error) on a component deriving props from HTMLDivElement. Good to know this is a typing error and nothing to do with my component. A fix would be much appreciated.

[HHogg]

👋 @ckifer I see there is a project for v3, but is there any information anywhere on the timeline for when it's going to be released and if it's going to be including many other breaking changes?

Just wondering if it could be worth doing a small fix for this on the v2 release to help folks resolve this without needing to do any migration to v3?

[PavelVanecek]

@HHogg it will still be couple of months until 3.x is ready. Yeah we can do 2.x release as well.

What would the fix look like?

[ckifer]

What Pavel said - I'm not sure if there is any fix except upgrading React types? I'm also unsure if doing so breaks anyone on 16.8 or 17

[HHogg]

👋 Similar to what Lukas742 above said. Taking a look at what we have in node_modules, we see that the keys are all being embedded and used in the Pick...

// types/container/Layer.d.ts
import React, { ReactNode, SVGProps } from 'react';
interface LayerProps {
    className?: string;
    children?: ReactNode;
}
export declare type Props = SVGProps<SVGGElement> & LayerProps;
export declare const Layer: React.ForwardRefExoticComponent<Pick<React.SVGProps<SVGGElement> & LayerProps, "string" | "children" | "dangerouslySetInnerHTML" | "onCopy" | "onCopyCapture" | "onCut" | "onCutCapture" | "onPaste" | "onPasteCapture" | "onCompositionEnd" | "onCompositionEndCapture" | "onCompositionStart" | "onCompositionStartCapture" | "onCompositionUpdate" | "onCompositionUpdateCapture" | "onFocus" | "onFocusCapture" | "onBlur" | "onBlurCapture" | "onChange" | "onChangeCapture" | "onBeforeInput" | "onBeforeInputCapture" | "onInput" | "onInputCapture" | "onReset" | "onResetCapture" | "onSubmit" | "onSubmitCapture" | "onInvalid" | "onInvalidCapture" | "onLoad" | "onLoadCapture" | "onError" | "onErrorCapture" | "onKeyDown" | "onKeyDownCapture" | "onKeyPress" | "onKeyPressCapture" | "onKeyUp" | "onKeyUpCapture" | "onAbort" | "onAbortCapture" | "onCanPlay" | "onCanPlayCapture" | "onCanPlayThrough" | "onCanPlayThroughCapture" | "onDurationChange" | "onDurationChangeCapture" | "onEmptied" | "onEmptiedCapture" | "onEncrypted" | "onEncryptedCapture" | "onEnded" | "onEndedCapture" | "onLoadedData" | "onLoadedDataCapture" | "onLoadedMetadata" | "onLoadedMetadataCapture" | "onLoadStart" | "onLoadStartCapture" | "onPause" | "onPauseCapture" | "onPlay" | "onPlayCapture" | "onPlaying" | "onPlayingCapture" | "onProgress" | "onProgressCapture" | "onRateChange" | "onRateChangeCapture" | "onSeeked" | "onSeekedCapture" | "onSeeking" | "onSeekingCapture" | "onStalled" | "onStalledCapture" | "onSuspend" | "onSuspendCapture" | "onTimeUpdate" | "onTimeUpdateCapture" | "onVolumeChange" | "onVolumeChangeCapture" | "onWaiting" | "onWaitingCapture" | "onAuxClick" | "onAuxClickCapture" | "onClick" | "onClickCapture" | "onContextMenu" | "onContextMenuCapture" | "onDoubleClick" | "onDoubleClickCapture" | "onDrag" | "onDragCapture" | "onDragEnd" | "onDragEndCapture" | "onDragEnter" | "onDragEnterCapture" | "onDragExit" | "onDragExitCapture" | "onDragLeave" | "onDragLeaveCapture" | "onDragOver" | "onDragOverCapture" | "onDragStart" | "onDragStartCapture" | "onDrop" | "onDropCapture" | "onMouseDown" | "onMouseDownCapture" | "onMouseEnter" | "onMouseLeave" | "onMouseMove" | "onMouseMoveCapture" | "onMouseOut" | "onMouseOutCapture" | "onMouseOver" | "onMouseOverCapture" | "onMouseUp" | "onMouseUpCapture" | "onSelect" | "onSelectCapture" | "onTouchCancel" | "onTouchCancelCapture" | "onTouchEnd" | "onTouchEndCapture" | "onTouchMove" | "onTouchMoveCapture" | "onTouchStart" | "onTouchStartCapture" | "onPointerDown" | "onPointerDownCapture" | "onPointerMove" | "onPointerMoveCapture" | "onPointerUp" | "onPointerUpCapture" | "onPointerCancel" | "onPointerCancelCapture" | "onPointerEnter" | "onPointerEnterCapture" | "onPointerLeave" | "onPointerLeaveCapture" | "onPointerOver" | "onPointerOverCapture" | "onPointerOut" | "onPointerOutCapture" | "onGotPointerCapture" | "onGotPointerCaptureCapture" | "onLostPointerCapture" | "onLostPointerCaptureCapture" | "onScroll" | "onScrollCapture" | "onWheel" | "onWheelCapture" | "onAnimationStart" | "onAnimationStartCapture" | "onAnimationEnd" | "onAnimationEndCapture" | "onAnimationIteration" | "onAnimationIterationCapture" | "onTransitionEnd" | "onTransitionEndCapture" | "className" | "color" | "height" | "id" | "lang" | "max" | "media" | "method" | "min" | "name" | "style" | "target" | "type" | "width" | "role" | "tabIndex" | "crossOrigin" | "accentHeight" | "accumulate" | "additive" | "alignmentBaseline" | "allowReorder" | "alphabetic" | "amplitude" | "arabicForm" | "ascent" | "attributeName" | "attributeType" | "autoReverse" | "azimuth" | "baseFrequency" | "baselineShift" | "baseProfile" | "bbox" | "begin" | "bias" | "by" | "calcMode" | "capHeight" | "clip" | "clipPath" | "clipPathUnits" | "clipRule" | "colorInterpolation" | "colorInterpolationFilters" | "colorProfile" | "colorRendering" | "contentScriptType" | "contentStyleType" | "cursor" | "cx" | "cy" | "d" | "decelerate" | "descent" | "diffuseConstant" | "direction" | "display" | "divisor" | "dominantBaseline" | "dur" | "dx" | "dy" | "edgeMode" | "elevation" | "enableBackground" | "end" | "exponent" | "externalResourcesRequired" | "fill" | "fillOpacity" | "fillRule" | "filter" | "filterRes" | "filterUnits" | "floodColor" | "floodOpacity" | "focusable" | "fontFamily" | "fontSize" | "fontSizeAdjust" | "fontStretch" | "fontStyle" | "fontVariant" | "fontWeight" | "format" | "fr" | "from" | "fx" | "fy" | "g1" | "g2" | "glyphName" | "glyphOrientationHorizontal" | "glyphOrientationVertical" | "glyphRef" | "gradientTransform" | "gradientUnits" | "hanging" | "horizAdvX" | "horizOriginX" | "href" | "ideographic" | "imageRendering" | "in2" | "in" | "intercept" | "k1" | "k2" | "k3" | "k4" | "k" | "kernelMatrix" | "kernelUnitLength" | "kerning" | "keyPoints" | "keySplines" | "keyTimes" | "lengthAdjust" | "letterSpacing" | "lightingColor" | "limitingConeAngle" | "local" | "markerEnd" | "markerHeight" | "markerMid" | "markerStart" | "markerUnits" | "markerWidth" | "mask" | "maskContentUnits" | "maskUnits" | "mathematical" | "mode" | "numOctaves" | "offset" | "opacity" | "operator" | "order" | "orient" | "orientation" | "origin" | "overflow" | "overlinePosition" | "overlineThickness" | "paintOrder" | "panose1" | "path" | "pathLength" | "patternContentUnits" | "patternTransform" | "patternUnits" | "pointerEvents" | "points" | "pointsAtX" | "pointsAtY" | "pointsAtZ" | "preserveAlpha" | "preserveAspectRatio" | "primitiveUnits" | "r" | "radius" | "refX" | "refY" | "renderingIntent" | "repeatCount" | "repeatDur" | "requiredExtensions" | "requiredFeatures" | "restart" | "result" | "rotate" | "rx" | "ry" | "scale" | "seed" | "shapeRendering" | "slope" | "spacing" | "specularConstant" | "specularExponent" | "speed" | "spreadMethod" | "startOffset" | "stdDeviation" | "stemh" | "stemv" | "stitchTiles" | "stopColor" | "stopOpacity" | "strikethroughPosition" | "strikethroughThickness" | "stroke" | "strokeDasharray" | "strokeDashoffset" | "strokeLinecap" | "strokeLinejoin" | "strokeMiterlimit" | "strokeOpacity" | "strokeWidth" | "surfaceScale" | "systemLanguage" | "tableValues" | "targetX" | "targetY" | "textAnchor" | "textDecoration" | "textLength" | "textRendering" | "to" | "transform" | "u1" | "u2" | "underlinePosition" | "underlineThickness" | "unicode" | "unicodeBidi" | "unicodeRange" | "unitsPerEm" | "vAlphabetic" | "values" | "vectorEffect" | "version" | "vertAdvY" | "vertOriginX" | "vertOriginY" | "vHanging" | "vIdeographic" | "viewBox" | "viewTarget" | "visibility" | "vMathematical" | "widths" | "wordSpacing" | "writingMode" | "x1" | "x2" | "x" | "xChannelSelector" | "xHeight" | "xlinkActuate" | "xlinkArcrole" | "xlinkHref" | "xlinkRole" | "xlinkShow" | "xlinkTitle" | "xlinkType" | "xmlBase" | "xmlLang" | "xmlns" | "xmlnsXlink" | "xmlSpace" | "y1" | "y2" | "y" | "yChannelSelector" | "z" | "zoomAndPan" | "aria-activedescendant" | "aria-atomic" | "aria-autocomplete" | "aria-busy" | "aria-checked" | "aria-colcount" | "aria-colindex" | "aria-colspan" | "aria-controls" | "aria-current" | "aria-describedby" | "aria-details" | "aria-disabled" | "aria-dropeffect" | "aria-errormessage" | "aria-expanded" | "aria-flowto" | "aria-grabbed" | "aria-haspopup" | "aria-hidden" | "aria-invalid" | "aria-keyshortcuts" | "aria-label" | "aria-labelledby" | "aria-level" | "aria-live" | "aria-modal" | "aria-multiline" | "aria-multiselectable" | "aria-orientation" | "aria-owns" | "aria-placeholder" | "aria-posinset" | "aria-pressed" | "aria-readonly" | "aria-relevant" | "aria-required" | "aria-roledescription" | "aria-rowcount" | "aria-rowindex" | "aria-rowspan" | "aria-selected" | "aria-setsize" | "aria-sort" | "aria-valuemax" | "aria-valuemin" | "aria-valuenow" | "aria-valuetext" | "key"> & React.RefAttributes<unknown>>;
export {};

I think it's due to using SVGProps rather than SVGAttributes. I stumbled upon AntD, making a similar change. Which after making and running another build, I get something more like I would expect to see.

// types/container/Layer.d.ts
import React, { ReactNode, SVGAttributes } from 'react';
interface LayerProps {
  className?: string;
  children?: ReactNode;
}
export type Props = SVGAttributes<SVGGElement> & LayerProps;
export declare const Layer: React.ForwardRefExoticComponent<
  React.SVGAttributes<SVGGElement> & LayerProps & React.RefAttributes<unknown>
>;
export {};

[HHogg]

Also worth noting, it's only Layer that has this problem, looking at all the other generated type definitions they seem to be fine.

[ckifer]

Pretty much all components extend SVG props one way or another so I'm not sure why that's the case. If we change it one place we should change it everywhere

[ckifer]

I'll take a look at things at some point today but I want some sort of guarantee that only this component is affected and a simple change such as the one proposed will fix it so we can avoid multiple patches (and avoid this in the future)

Type changes have a way of making things break. Just want to sanity check that we won't break more than this already does with a change

[ckifer]

Thanks @HHogg that helps.

I'm still confused why this is only an issue for Layer

Most components props extend this which contains SVGProps. Most other components have onPointerEnterCapture, etc. as acceptable props because of that. Why do things not fail there as well? (saying this out loud before investigating, I'll start investigating when I get time)

[HHogg]

That I'm not really sure about, other than assuming there's some tooling that is being "helpful" with optimizing it at build time for some reason.

[ckifer]

Ok so double checking everything everything:

• This only happens for components that use forwardRef (noted above)
• we only do this twice
  • Layer -> this issue
  • ResponsiveContainer -> The same problem does not seem to happen with HTMLDivElement, only SVGProps causes the issue. If I do something silly like make ResponsiveContainerProps extend SVG props, we have the same result. It is unclear why, but 🤷🏼‍♂️
• Removing SVGProps isn't an issue due to what was mentioned above - ref and key are added implicitly in React (forwardRef also adds ref for us)
• Autocomplete still works for all applicable SVG types (still need to update @types/react in 3.x of course)
• No urgent need to replace the other usage of SVGProps in 2.x, but we should use SVGAttributes over SVGProps moving forwards

Confirmed the SVGAttributes fix works with a local publish using yalc

[HHogg]

Hey @ckifer 👋 Would it be ok to pop out a release for this change?

[ckifer]

I can yeah, just need to find some free time

[ckifer]

Fixed in https://github.com/recharts/recharts/releases/tag/v2.12.5