search

joe223 / tiny-swiper

Ingenious JavaScript Carousel powered by wonderful plugins. Lightweight yet extensible. Import plugins as needed, No more, no less.
https://tiny-swiper.js.org
1.29k stars 59 forks source link

Add "destroy" method #3

Closed r0skar closed 4 years ago

r0skar commented 5 years ago

Is your feature request related to a problem? Please describe. Using the swiper in a SPA, the Swiper instances are never destroyed as there are no real page-reloads.

Describe the solution you'd like Implement the destroy method from SwiperJS.

Additional context See https://swiperjs.com/api/ for more details on the destroy API.

joe223 commented 5 years ago

The challenge for me is that I have to make a trade-off between package size and API richness. We will deviation from purpose if it is too large, it will be useless if lack of important APIs, either.

So, I've thinking of extracting destroy events-hub and life hooks (both of them are not completed) to components, just use them whenever you need. Otherwise, It will grow larger than 3kB someday and more.

@r0skar How do you think?

northkode commented 5 years ago

A destroy method is critical for memory leaks, we need a way to be able to remove all event listeners and cleanup after the component is no longer needed. Especially in SPA applications.

joe223 commented 5 years ago

@northkode You are right, I'll move destroy to core API.

joe223 commented 5 years ago

The draft https://github.com/joe223/tiny-swiper/blob/317b3c037b57c56383f0fe74267ea0073720c282/src/index.js#L462-L482

Pull Request https://github.com/joe223/tiny-swiper/pull/5

It's almost 3KB. 😂 a little bit embarrassing

joe223 commented 4 years ago

Resolved at https://github.com/joe223/tiny-swiper/pull/6