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 incorrect parameters in event dispatching in Svelte #5716

Merged
merged 1 commit into from
May 16, 2022

Conversation

jdoubleu
Copy link
Contributor

@jdoubleu jdoubleu commented May 12, 2022

The Svelte type definitions for custom events are declared as:

declare class Swiper extends SvelteComponentTyped<
  SwiperProps,
  {
    swiper: CustomEvent<void>;
    autoplayStart: CustomEvent<[swiper: SwiperClass]>;
    // ...
    lazyImageLoad: CustomEvent<[swiper: SwiperClass, slideEl: HTMLElement, imageEl: HTMLElement]>;
    // ...
}

I couldn't find those in the sources, so I assume they're generated during the build process.

As you can see, an event handler subscribing to one of these events, should receive an array with either a single element (i.e. the swiper instance) or more parameters.

Theres a bug in the event delegation from Swipers internal state to Svelte at swiper.svelte. Instead of passing the original event parameters along, they're instead wrapped in another array. This is due to the fact of incorrect usage of the ...args spread parameter. The actual type is something like CustomEvent<[[swiper: SwiperClass]]>, right now.

One problem arises, though, because this is sort of a breaking change. Project that currently rely on that misbehavior would break. If anyone used TypeScript and the definitions, they should've already noticed that.

I feel like this could also be checked in the CI. Unfortunately, you're not building svelte in the CI yet or at least on GitHub, as it seems. How do you feel about adding examples for all the integrations (e.g. Svelte, Vue and React) and then just build them as well to see if any warnings/errors are emitted. We could use the TypeScript compiler in "check" mode to spot any obvious type incompatibilities. The svelte template also ships with a handy check command, to verify all files. Only if swiper.svelte was strongly typed, this might be detectable by static checks.

@nolimits4web nolimits4web merged commit b7961a3 into nolimits4web:master May 16, 2022
@nolimits4web
Copy link
Owner

Merged, thanks! It is a breaking change, but I think it is the right way, more like a breaking fix :)

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.

2 participants