Skip to content

Upgrade: using ES6 Module instead of CommonJS#279

Closed
yosion-p wants to merge 6 commits intopillarjs:masterfrom
yosion-p:esm
Closed

Upgrade: using ES6 Module instead of CommonJS#279
yosion-p wants to merge 6 commits intopillarjs:masterfrom
yosion-p:esm

Conversation

@yosion-p
Copy link
Contributor

@yosion-p yosion-p commented Sep 7, 2021

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
Copy link
Contributor

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
Copy link
Contributor Author

yosion-p commented Sep 7, 2021

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
Copy link
Contributor

ashtuchkin commented Sep 8, 2021

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
Copy link
Contributor

See #280 for an example.

@yosion-p
Copy link
Contributor Author

yosion-p commented Sep 9, 2021

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.

@yosion-p yosion-p closed this Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants