Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SVGElement for useEventListener #547

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

LumaKernel
Copy link
Contributor

Resolves #546

Copy link

changeset-bot bot commented Mar 18, 2024

🦋 Changeset detected

Latest commit: 9e8eba4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
usehooks-ts Patch
www Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@LumaKernel
Copy link
Contributor Author

usehooks-ts:lint
cache bypass, force executing 608f84a12c3816f9

> usehooks-ts@3.0.1 lint /home/runner/work/usehooks-ts/usehooks-ts/packages/usehooks-ts
> eslint './src/**/*.{ts,tsx}' && tsc --noEmit


/home/runner/work/usehooks-ts/usehooks-ts/packages/usehooks-ts/src/useEventListener/useEventListener.ts
Error:   40:3  error  This overload and the one on line [29](https://github.com/juliencrn/usehooks-ts/actions/runs/8325984947/job/22929381312#step:7:31) can be combined into one signature taking `(event: HTMLElementEventMap[K]) => void | (event: SVGElementEventMap[K]) => void`  @typescript-eslint/unified-signatures

✖ 1 problem (1 error, 0 warnings)

 ELIFECYCLE  Command failed with exit code 1.
Error:  command finished with error: command (/home/runner/work/usehooks-ts/usehooks-ts/packages/usehooks-ts) /home/runner/setup-pnpm/node_modules/.bin/pnpm run lint exited (1)
Error: usehooks-ts#lint: command (/home/runner/work/usehooks-ts/usehooks-ts/packages/usehooks-ts) /home/runner/setup-pnpm/node_modules/.bin/pnpm run lint exited (1)
 ERROR  run failed: command  exited (1)

 Tasks:    1 successful, 2 total
Cached:    0 cached, 2 total
  Time:    11.07s 
Failed:    usehooks-ts#lint

 ELIFECYCLE  Command failed with exit code 1.
Error: Process completed with exit code 1.

hmm, with quick look, I believe it's false-positive case of @typescript-eslint/unified-signatures, the case has type parameter with extends to support meaningful inference.

function useEventListener<
-  K extends keyof SVGElementEventMap,
+  K extends keyof HTMLElementEventMap | keyof SVGElementEventMap,
  T extends SVGElement,
>(

Something like this unification may not resolve the problem caused even if adopting the suggestion made by this rule.

I'll dive into it afterward, and if it would be found to be false-positive, I'll open the issue on typescript-eslint.

@juliencrn
Copy link
Owner

juliencrn commented Mar 21, 2024

Hi @LumaKernel,
First, thank you for the PR 👍

I fixed it that way, merging the HTMLElement and SVGElement overloads into the Element Event based interface one:

// Element Event based useEventListener interface
function useEventListener<
  K extends keyof HTMLElementEventMap & keyof SVGElementEventMap,
  T extends Element = K extends keyof HTMLElementEventMap
    ? HTMLDivElement
    : SVGElement,
>(
  eventName: K,
  handler:
    | ((event: HTMLElementEventMap[K]) => void)
    | ((event: SVGElementEventMap[K]) => void),
  element: RefObject<T>,
  options?: boolean | AddEventListenerOptions,
): void

@juliencrn juliencrn merged commit 59c0b93 into juliencrn:master Mar 21, 2024
1 of 2 checks passed
@LumaKernel LumaKernel deleted the luma/issue-546 branch March 22, 2024 01:14
@LumaKernel
Copy link
Contributor Author

Great! Thank you 🐈‍⬛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] useEventListener can be used with SVGElement, but rejected by type-check
2 participants