dom-event icon indicating copy to clipboard operation
dom-event copied to clipboard

feat: narrow down type of currentTarget

Open TylorS opened this issue 6 years ago • 3 comments

This PR tries to narrow down the type of currentTarget when the node type is statically known. I think that line 33 and down may be a breaking change because it makes the type of node no longer support all EventTargets but looking on MDN these are the places that implement these events. If it's preferred I can put that change into a separate PR to just get the currentTarget changes merged before.

TylorS avatar May 28 '19 23:05 TylorS

I think we could also do this same thing for Flow right?

TylorS avatar May 28 '19 23:05 TylorS

This looks really nice, @TylorS. I see what you're saying about lines 33 and below, but I have no idea how to gauge how painful it might be. I guess someone could be using a custom event or custom EventTarget.

I certainly don't mind doing a major version bump. One strategy would be to include the potentially breaking changes, bump major, and then if it turns out to be prohibitive, loosen the types in a subsequent patch or minor release. Thoughts? Other options?

As for Flow, yeah, I think the same approach should work. 👍.

briancavalier avatar May 29 '19 00:05 briancavalier

Yeah, moving back to EventTarget afterwards shouldn't even a breaking change after the initial major bump. I think that's a pretty solid plan.

I can try to add the corresponding flow types soon 👍

TylorS avatar May 29 '19 14:05 TylorS