search

ashtuchkin / iconv-lite

Convert character encodings in pure javascript.
MIT License
3.04k stars 282 forks source link

Upgrade: using ES6 Module instead of CommonJS #279

Closed yosion-p closed 2 years ago

yosion-p commented 2 years ago

Hi buddy, I upgraded the whole project from CommonJS to ES6 Module(#273 ). Although there are still some issues which are mainly generation, test/performance and test/webpack, but it passed all other UT. I submitted the PR. Do you think it can work well in this way? If yes, I can fix the rest asap.

ashtuchkin commented 2 years ago

Hey thank you for your effort here. I think it would work in general, but I can't accept it, at least for now, because iconv-lite supports pretty old Node versions and I don't want to lose this support. Not sure why Travis CI didn't provide test coverage here, otherwise it'd show exactly what versions are affected.

yosion-p commented 2 years ago

Hi buddy, I built GitHub Action CI in my branch, and the PR was passed. However, the CI tool has version restrictions on nodejs. The official declaration link: (P.S. Lline:38),My test link is here. For what you said that "Travis CI didn't provide test coverage", yes, i noticed it as well. And i see actually there is official declaration that since June 15th, 2021, the building on travis-ci.org is ceased, official declaration link: .

Due to these, maybe we need to upgrade the related Travis CI configuration, or to use Action CI(while it can't be compatible with older versions nodejs, but we can add it manually). What do you think?

ashtuchkin commented 2 years ago

Hey, yep I believe you have the tests passing on latest node. What I wanted to see is them passing on older node versions, which I don't think can happen as the older versions don't support ES Modules.

Travis-ci.org is dead, but travis-ci.com continues supporting public projects for free, so I just updated the config on its website and it should start checking all PRs and commits automatically.

ashtuchkin commented 2 years ago

See #280 for an example.

yosion-p commented 2 years ago

OK Unfortunately, I didn't notice that CI needs a node version compatible with 0.1, Perhaps, based on the development route of iconv Lite, stability and compatibility are necessary? I just ran Travis Ci, and it really didn't pass; But my other #275 can pass all. Maybe you can take a look at it.