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 custom events on native DOM elements #431

Open
johanbissemattsson opened this issue Aug 5, 2020 · 16 comments
Open

Support custom events on native DOM elements #431

johanbissemattsson opened this issue Aug 5, 2020 · 16 comments
Labels
feature request New feature or request

Comments

@johanbissemattsson
Copy link

johanbissemattsson commented Aug 5, 2020

Is your feature request related to a problem? Please describe.
I get the following error when I'm trying to add custom events on native DOM elements:

Type '{ onswipestart: (event: CustomEvent<any>) => void; onswipemove: (event: CustomEvent<any>) => void; onswipeend: (event: CustomEvent<any>) => void; style: string; class: string; }' is not assignable to type 'HTMLProps<HTMLDivElement>'.
  Property 'onswipestart' does not exist on type 'HTMLProps<HTMLDivElement>'.ts(2322)

The custom events are dispatched to a div element using Sveltes actions/use directive https://svelte.dev/examples#actions).

Describe the solution you'd like
Be able to type check for custom events dispatched using Svelte actions on native DOM elements.

Describe alternatives you've considered
I could try to convert the div to an individual Svelte component but preferably it should work on native DOM elements as well.

Additional context

The error message

This is how I listen to the custom events:

<div
  class="bottomSheet"
  class:draggable
  bind:this={bottomSheet}
  use:swipeable
  on:swipestart={handleSwipeStart}
  on:swipemove={handleSwipeMove}
  on:swipeend={handleSwipeEnd}
  style="height:{height};bottom:{bottom};transform:translateY({$coords.ty}px);"
>

Related Pull Request for custom events on Svelte components: #303

@jasonlyu123
Copy link
Member

jasonlyu123 commented Aug 6, 2020

Don't know if there is a way to make it only applies to elements with the action. but you can make it globally available like this.

declare namespace svelte.JSX {
    interface HTMLAttributes<T> {
        onswipemove?: (event: CustomEvent<number> & { target: EventTarget & T }) => any;
    }
}

@dummdidumm
Copy link
Member

We could also enhance our typings with "fall back to CustomEvent<any> but I'd rather not do it because others may rely on the typings to throw a "does not exist" error if they mistype an event. I would be okay with something like that if it's only applied to elements with actions, but I'm not sure how we would implement that.
To infer the type from the action is an almost impossible task I think. Maybe we can find a way to let the user explicitly type the action and its possible events.

@johanbissemattsson
Copy link
Author

johanbissemattsson commented Aug 6, 2020

Thanks, letting the user explicitly type the action or explicitly choose when to fall back to CustomEvent<any> would probably be the best case scenario but the solution by @jasonlyu123 is good enough for me now. Thanks for the great work!

@dummdidumm dummdidumm added the feature request New feature or request label Aug 6, 2020
@dummdidumm
Copy link
Member

I had a look at this again and now I'm split on what's the best way forward.

Option 1: Silence error

We could silence the error if it's a on:XX event on a DOM element with a use:YY directive. The drawback is that the event is of type any implicitly. It also may not catch all cases, because Svelte is so dynamic in its nature, someone could do bubbling CustomEvents.

Option 2: Add CustomEvent<any> fallback

We would essentially add & {[key: string]: CustomEvent<any>} to all instrinsic element typings. This means any attribute that is not listed above will fall back to CustomEvent<any>. This would fix the "dynamic nature" problem and the "implicit any" problem of option 1. But it is also wrong in cases someone uses a custom attribute like myownAttribute.

Option 3: Add any fallback

Like option 2, only that we add a & {[key: string]: any} fallback. This means anything that does not fit falls back to any. This fits the dynamic nature of the DOM the best at the cost of not catching typos anymore.

@dummdidumm
Copy link
Member

dummdidumm commented May 16, 2021

My latest idea for this is to

  • generate a file with all event definitions from the jsx typings
  • if an action exists on a dom element, all events that are not found in the intrinsic events are treated as custom events by transforming the appropriately

Edit: I just realized this would break people who enhanced the definition with their own, because we don't know of those and would still transform to the CustomEvent<any> fallback. I don't think this is desireable.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue May 17, 2021
Svelte is very dynamic in its nature. One feature is to add actions to DOM elements which in turn means additional events could be dispatched on that element. For the default namespace, transform unknown events on an element with an action so that they are treated as custom events.
sveltejs#431
@aradalvand
Copy link

aradalvand commented Jun 24, 2021

Hey there, I was wondering if the solution suggested by @jasonlyu123 is still the only fix for this problem?
I, too, have actions that dispatch custom events and I would like to get rid of the type error.

@dummdidumm
Copy link
Member

It's still the solution, yes. I would appreciate some feedback on the three options listed in my comment above. I lean more and more towards option 3 since it matches the dynamic nature of Svelte the best, and typos are very rare I guess.

@aradalvand
Copy link

aradalvand commented Jun 24, 2021

@dummdidumm Personally, I'm an everything-should-be-as-type-safe-as-possible kinda guy, so I think rather than falling back to any or even CustomEvent<any>, it'd be nice if we could have a way to explicitly declare what custom events a given action will potentially dispatch, and also whether or not each event bubbles.
If an event bubbles, then it should be available on the element itself (the element that has the action applied to it) AND all of its ancestors. Everywhere else the event is used we should have a type error.

Now, I have no idea how difficult or tricky this would be to implement. It might actually be a terrible idea. Let me know what you think.

If this isn't feasible, I agree that the 3rd approach among the ones you proposed in this comment would be the sanest.

@dummdidumm
Copy link
Member

Yes what you describe would be ideal, but from a typing perspective I don't see a way to accomplish it, because one can only retrieve the return type of the function. One possibility would be to check if there's a property on the function which is only used for type checking, similar to how Angular does it. But even if we do that, then there's the bubble thing, which is just impossible in my mind to do, because we more or less have to statically analyze everything as a whole, and even that might not work for all cases.
So yeah, solution 3 is probably the best option, and people who want strict types for specific events can still type them given the instructions above. And we could look into that property-on-function-thing.

@dummdidumm
Copy link
Member

We could use the enhanced type signatures to tell svelte2tsx that every property starting with on and not part of the other defined ones is a Custom Event. Question is if that's also too narrow, since theoretically it could be something else. It also requires users of svelte-check to at least use TS 4.4

@TheFanatr
Copy link

TheFanatr commented Jul 20, 2021

what about returning some metadata from the action itself i.e.:

return {
    ... // destroy, update, etc.
    events: {
        allowAll: false,
        definitions: [
            { name: "panstart", maxRate: 3 },
            ...
        ]
    }
}

another option would be to include them as parameters:

type ClickEventDetails = HeldClickedEventDetails & { secondsSinceLast: number }
type ClickHeldEventDetails = { count: number }
type ClickHoldReleasedEventDetails = ClickEventDetails & { totalHoldSeconds: number }

function extraClickEvents(node: Node, countableClick: CustomEvent<ClickEventDetais>, clickHeld: CustomEvent<ClickHeldEventDetails>, clickHoldReleased: CustomEvent<ClickHoldReleasedDetails>) {
    ...
    node.dispatchEvent(nameof(countableClick).toLower(), countableClick.setDetails({ count: ..., secondsSinceLast: ... })) // toLower just represents the concept of lowercasing.
    ...
}

then:

<div use:extraclickevents on:countableclick={...} ...>

@dummdidumm
Copy link
Member

dummdidumm commented Sep 2, 2021

I like the "use return type to hint at this"-idea. How this could be implemented:

  1. assemble list of all known events inside the svelte-jsx-typings and make them available at runtime
  2. when transforming an event on a dom element, check the list of known events. If the event is not part of it and there's an action on the dom node, do a different transformation which also makes the extra events available

The limitation of this is that you can create actions which coordinate across multiple/parent nodes and therefore only works if the event is listened to on the same node the action was applied to.

@taylorhadden
Copy link

We could use the enhanced type signatures to tell svelte2tsx that every property starting with on and not part of the other defined ones is a Custom Event. Question is if that's also too narrow, since theoretically it could be something else. It also requires users of svelte-check to at least use TS 4.4

I do think it would be too narrow. What if I'm invoking some other event through the normal dom methods? What if the event originates from a child element? To handle this, I've resorted to manually adding & removing event listeners to element references. Not ideal.

@dummdidumm
Copy link
Member

Dumping this here for myself for later on: A way when using the new transformation on how to get the types of an action onto the element where the action is applied:

type SvelteAction<U extends any, El extends any> = (node: El, args:U) => {
	update?: (args:U) => void,
	destroy?: () => void,
    attributes?: Record<string, any>,
    events?: Record<string, any>
} | void

export type ValuesOf<T> = T[keyof T];
export type ObjectValuesOf<T extends Object> = Exclude<
  Exclude<Extract<ValuesOf<T>, object>, never>,
  Array<any>
>;
declare function createElement<Elements extends IntrinsicElements, Key extends keyof Elements, Actions extends Record<string, SvelteActionReturn>>(
    element: Key,
    actions: Actions,
    attrs: Elements[Key] & ObjectValuesOf<
        {[ActionKey in keyof Actions]:
            {[AttrKey in keyof Actions[ActionKey]['attributes']]: Actions[ActionKey]['attributes'][AttrKey] } &
            {[EvtKey in keyof Actions[ActionKey]['events'] as `on:${EvtKey & string}`]: Actions[ActionKey]['events'][EvtKey]
        }
    }>
): Key extends keyof ElementTagNameMap ? ElementTagNameMap[Key] : Key extends keyof SVGElementTagNameMap ? SVGElementTagNameMap[Key] : HTMLElement;

This monstrosity will allow events and attributes returned by the action type to be added as attributes in addition to the intrinsic attributes.

dummdidumm added a commit that referenced this issue Aug 26, 2022
#1243
#431

This assumes we also enhance the typing of the action type in Svelte core so they match
@jacwright
Copy link

Another place where this falls down is bubbled native events. In my app I am listening to the input event being bubbled up from form elements inside a form. Typescript doesn't appreciate <form on:input={...}> though it is perfectly valid. While I appreciate all the benefits of TypeScript, the nature of events and bubbling means that you can't account for every event that may come past an element by only looking at that element's events.

And in this case, the JSX method only works with Event and throws errors with InputEvent.

Screenshot 2022-11-17 at 11 52 34 AM

@jasonlyu123
Copy link
Member

The error is correct. The type of Input event is only InputEvent on some occasions as mentioned in MDN https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/input_event

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

No branches or pull requests

7 participants