bvaughn / react-resizable-panels

https://react-resizable-panels.vercel.app/
MIT License
3.99k stars 144 forks source link

Issue with DragEventHandler<keyof HTMLElementTagNameMap> Type #407

Closed in-ch closed 1 month ago

in-ch commented 2 months ago

Hello. 😀

When using the Panel component with event handlers such as onDragLeave, onDragEnter, onDrop, and onDragOver typed as DragEventHandler<keyof HTMLElementTagNameMap>, a type error occurs when trying to use the contains method, which is a method of HTMLElement.

Here's an example where the issue occurs:

const handleDragLeave: DragEventHandler<keyof HTMLElementTagNameMap> = (
  e: DragEvent<keyof HTMLElementTagNameMap>,
): void => {
  if (e.currentTarget.contains(e.relatedTarget)) return;
  setIsDragActive(false);
};

스크린샷 2024-09-12 12 20 00

The issue is that the keyof HTMLElementTagNameMap type represents string keys for tag names, so it cannot infer the types of methods (e.g., contains) that the event object has.

Therefore, the DragEventHandler<keyof HTMLElementTagNameMap> type is limited in using methods of HTMLElement.

How about using a union type or something similar to make it compatible with both keyof HTMLElementTagNameMap and HTMLElement types?

bvaughn commented 2 months ago

How about using a union type or something similar to make it compatible with both keyof HTMLElementTagNameMap and HTMLElement types?

I don't think this fixes the typing situation you describe (at least not when I just did a quick check locally). Want to share a TypeScript REPL or something?

in-ch commented 1 month ago

Thank you for your response. 😀

I am sharing the CodeSandbox link for part of the code used in the project.

In the useDrag hook, I defined the handleDragLeave function using the type React.DragEvent<HTMLDivElement> in order to use the currentTarget value from the event object.

However, when I wrote the code this way, I encountered the following error in the onDragLeave function of the component:

Type '(e: React.DragEvent<HTMLDivElement>) => void' is not assignable to type 'DragEventHandler<keyof HTMLElementTagNameMap>'.
  Types of parameters 'e' and 'event' are incompatible.
    Type 'DragEvent<keyof HTMLElementTagNameMap>' is not assignable to type 'DragEvent<HTMLDivElement>'.
      Type 'string' is not assignable to type 'HTMLDivElement'.
        Type 'string' is not assignable to type 'HTMLDivElement'.typescript(2322)
index.d.ts(1559, 9): The expected type comes from property 'onDragLeave' which is declared here on type 'IntrinsicAttributes & Omit<HTMLAttributes<keyof HTMLElementTagNameMap>, "id" | "onResize"> & { className?: string | undefined; ... 11 more ...; tagName?: keyof HTMLElementTagNameMap | undefined; } & { ...; } & RefAttributes<...>'
in-ch commented 1 month ago

Since the onDragLeave prop is defined as React.DragEventHandler<keyof HTMLElementTagNameMap>, TypeScript cannot correctly infer the currentTarget property in the event object. To avoid TypeScript compilation errors in the project code, I was able to resolve it by using a union type like this:

const handleDragLeave: DragEventHandler<keyof HTMLElementTagNameMap> = (
  e: DragEvent<keyof HTMLElementTagNameMap>,
): void => {
  if (e.currentTarget.contains(e.relatedTarget)) return;
  setIsDragActive(false);
};

In my opinion, for greater flexibility in types, it would be better to define the onDragEvent, onDragLeave, onDragOver, and onDrop props in the Panel component as follows to increase type flexibility and accuracy:

React.DragEventHandler<keyof HTMLElementTagNameMap | HTMLElementTagNameMap>

This will prevent type conflicts and ensure compatibility when handling drag events.🙏

bvaughn commented 1 month ago
React.DragEventHandler<keyof HTMLElementTagNameMap | HTMLElementTagNameMap>

I think you maybe meant...

React.DragEventHandler<keyof HTMLElementTagNameMap | HTMLElement >

But assuming I'm understanding you correctly...

import { createElement, HTMLAttributes } from "react";

type Props = HTMLAttributes<keyof HTMLElementTagNameMap | HTMLElement>;

function Component(props: Props) {
  return createElement("div");
}

const component = createElement(Component, {
  onDragLeave: event => {
    // @ts-expect-error This still errors
    event.currentTarget.contains()
  }
});

That doesn't work.

Property 'contains' does not exist on type 'EventTarget & (HTMLElement | keyof HTMLElementTagNameMap)'. Property 'contains' does not exist on type 'EventTarget & "object"'.(2339)

This works (at least for your "contains" example) but would otherwise be a breaking change for this library, I think:

type Props = HTMLAttributes<HTMLElement>

So can you share some code that shows how this would work?

in-ch commented 1 month ago

It definitely works well when done like this:

type Props = HTMLAttributes<HTMLElement>;

In the previous code, the type of currentTarget might have been a combination of string and HTMLElement. This means that currentTarget could have a type of string (which is a key of HTMLElement), causing the type check for the contains method to fail.

HTMLElement is generally supported for all tags included in keyof HTMLElementTagNameMap. In other words, HTMLElement represents typical DOM elements, so it can be used with various elements like <div>, <span>, etc. However, if there are compatibility issues with the library, I will look for compatible types.

Thank you for your response. 😀

bvaughn commented 1 month ago

Fixed via #414; fix has been released in react-resizable-panels@2.1.4


❤️ → ☕ givebrian.coffee