Apexcharts.js: Issue with code quality and build

[cstlaurent]

Hi,
I was working on investigating the issue #260 with @mamasselin but a couple of issues quickly blocked me after cloning the repo and looking at the code.

Here is a list of issues I found:

Code quality issues

• Some tests do not pass on the latest master branch, some e2e tests look flakey and break sometimes, 2 unit tests fail consistently.
• There is an eslint file, but it is not enforced. It looks like there are more than a thousand issues regarding that.
• There is a couple of hardcoded/copy-pasted dependencies in the src folder.

Build problems

• There is a mixed usage of require and import which causes issues with Webpack.
• There are Gulp, Webpack, and Rollup used for builds, probably causes a lot of unnecessary maintenance burden (was there a reason for that?)
• I think the ESM/module build is misconfigured. If ApexChart is imported through NPM, the Webpack bundler always take the dist/apexcharts.min.js, making it very hard to debug in dev mode and optimize correctly for production mode.
• Since some dependencies are hardcoded in the project (svg.js, some polyfills) and not correctly imported through NPM, Webpack is not able to optimize them and deduplicate/tree-shake dependencies.
• Dev dependencies like rollup and Webpack are quite old and should probably be updated if kept at all.
• Tests and Lint are not run in the CI, making it very hard to do proper gate-keeping in PRs.
• Changelog or release definitions are not properly kept up to date, making it hard for an external contributor and users to see what was changed or fixed.

This may seem like much negativity, but even with those issues, I still think your library is one of the best out there, and I thank you for creating it.

I think my colleague @mamasselin and I can help out on most of those issues if you are open to tackling them.

I can open separate issues for every point and elaborate a plan to fix them or propose fixes if you are ok with that.

Thanks!

[junedchhipa]

@cstlaurent
Those are very good points, and I agree on many of them. I will try to answer each of them.

1. Mixed usage of import and require is due to the SVG.js not allowing import in their previous versions. They released v3.0 recently, and I haven't upgraded to it.
2. Why Webpack, gulp, and rollup?
   Initially, Webpack was used primarily for the build. Later on, a feature request to expose ApexCharts as ES module was made. Webpack didn't help much in those regards (maybe because I am using older versions), so I used rollup.
   Gulp, on the other hand, does some minor tasks of copying files and cleaning some logs. If you have alternatives for it, I am open to implementing.
3. Hardcoded dependencies.
   SVG.js had some issues in their core library which were not fixed in their old versions. So, I manually applied some patches and had to hardcode their library into src.
4. Yes, dev-dependencies should be updated regularly, agreed.
5. I agree about your points of tests not in CI. Also, the test coverage is very low at the moment. There are several issues with SVG.js not being passed in unit tests, hence I move more towards e2e tests.
6. Agreed on the point of Changelog as well as release definitions.

The codebase got huge very quickly and along with maintaining the library, website maintenance takes a considerable amount of efforts, hence it takes a toll on the code quality and tests.

If you would like to contribute in any of these areas, you are more than welcome.

I also setup code-climate for ApexCharts - https://codeclimate.com/github/apexcharts/apexcharts.js
Maybe it will help to focus on duplications/code-quality from there.

[cstlaurent]

I will take a look into the build configuration tomorrow as a first step.

[junedchhipa]

Closing this.
If any points from the list are left out, please open a new issue for each point to allow better tracking of the issues.