dynamicweb / swiffy-slider

Super fast carousel and slider with touch for optimized websites running in modern browsers.
MIT License
238 stars 29 forks source link

Typescript and framework integration #73

Open tleperou opened 1 year ago

tleperou commented 1 year ago

Is your feature request related to a problem? Please describe. For those working with typescript, integrating Swiffy-slider requires some work. In my case, the builder.io Qwik framework but it applies to any others like Remix.

Describe the solution you'd like Make Swiffy-slider Typescript friendly and/or submit a type in @types/* repo

Describe alternatives you've considered There isn't much alternative.

Additional context Using the reference of window shall eventually be reconsidered in order to make the integration within other frameworks easier.


I've already worked on it and got a simple Typescript support.

If interested, I would create a new branch using this issue's ID.

nicped commented 1 year ago

Thank you for supporting and taking Swiffy Slider for a spin.

I have little experience with TS so I cannot say much here.

Swiffy Slider comes in classic JS that can be included in script tags or you can import it as an ESM. The latter is supported in TS as far as I can tell: https://www.typescriptlang.org/docs/handbook/esm-node.html

I am a huge fan of simple things and following standards. It is fine to add TS support - but if it requires complexity and going away from standards, I might not be in favor. Feel free to create a pull so I can see the implications.

Thanks, Nicolai

chimok commented 4 months ago

@nicped You don't need to change your code. You (or someone else) could simply create a new package @types/swiffy-slider with the type definition only. The advantage with typescript is, if you transpile your code the type definition is generated automatically.

Anyway. I like simple things too. Instead Typescript i prefer for small scripts just jsdoc, that is sadly missing in your code and could be helpful. VSCode supports it and use it for linting too.

nicped commented 4 months ago

I can go with adding the JSDoc - as they are minified away and does not come with any requirements. Even though the comments pollute the src code :-)

I am either too smart or not smart enough to add TS types.... Your call!. But feel free to create a package,.

tleperou commented 4 months ago

:+1: on adding JS doc. I'll be available only in a couple of weeks before getting into it