galkatz373 / svelte-apexcharts

Svelte wrapper for ApexCharts
68 stars 7 forks source link

Component #162

Open edde746 opened 2 years ago

edde746 commented 2 years ago

Made this a component & made it use svelte-kit package for TypeScript support, these changes are breaking.

samal-rasmussen commented 1 year ago

This is unnecessary. Just use bind:this to get an html element reference to pass to the vanilla js ApexCharts constructor.

Example here: https://github.com/galkatz373/svelte-apexcharts/issues/160#issuecomment-1759940967

edde746 commented 1 year ago

This is unnecessary. Just use bind:this to get an html element reference to pass to the vanilla js ApexCharts constructor.

Thank you for your opinion, I disagree. Having it as a component just increases readability and is the more "proper way" to do it in my opinion. It doesn't matter anyways as the owner of the repo seems to be inactive, so this will never get merged anyways.

samal-rasmussen commented 1 year ago

Which you like more is of course going to be a personal opinion. I'm pointing out that the component is not necessary. Svelte can use vanillja js libraries just as is. In React and other frameworks you often have to create wrappers in order to get third party libraries to work properly (in React because of integration with hooks). Not so in Svelte. You can usually use the basic original example from the docs of the third party library directly in Svelte. In the case of ApexCharts we need to use bind:this to get a reference to a node to pass to it, and we need to dynamically import ApexCharts in the onMount cb, because of ssr in SvelteKit. Otherwise the example I wrote in the issue comment I linked to, is just the very first example from the ApexChart docs.

This whole "just use third party libraries as is" thing got a lot of traction three months ago: https://hackmd.io/@roguegpu/r1RKQMdt3 https://www.reddit.com/r/sveltejs/comments/14uzavm/sveltejs_my_ecosystem_is_bigger_than_yours/ https://www.youtube.com/watch?v=bh-e700IlmQ

I'm not cop telling you what the "proper way" is, but this is certainly the most direct way :)

hyunbinseo commented 10 months ago

Can you use the vanilla JavaScript version of the ApexCharts in Svelte(Kit)? - Yes

Does that mean component⋅helper⋅wrapper libraries should be criticized? - No

Simple, no-dependency helper libraries all great if they are well maintained.

I even created a HTML \<dialog> wrapper component library which encapsulates the official Svelte modal example.


Regarding the PR, I believe this can be a non-breaking change, since the component does not rely on SvelteKit APIs. Non top-level import works in plain Svelte.

Wouldn't additional export be enough?

// Existing Export
import { chart } from "svelte-apexcharts";

// Newly Added Export
import Chart from 'svelte-apexcharts/Chart.svelte';

Notes: