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

fix(xychart): strip legacy react events from exhaustive prop lists in type declarations #1841

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mtlewis
Copy link

@mtlewis mtlewis commented May 28, 2024

🐛 Bug Fix

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 Omits are transformed into Picks 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.

… 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.
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.

1 participant