Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(es6) : switch a new batch of modules to ES6 #119

Closed
wants to merge 1 commit into from

Conversation

mmomtchev
Copy link

Refs: yargs/yargs#2112

string-width, wrap-ansi and strip-ansi now have ES6 versions

Also switch all unit tests that require them to ES6

@shadowspawn
Copy link
Member

The new versions of string-width, wrap-ansi and strip-ansi are not only esm, they are esm-only. So I think this PR will break the cjs build of cliui.

There are two recent PR taking different approaches to the problem: #139 #140

If I missed something cunning in this PR that means it does work for both the cjs and esm builds of cliui, let me know!

@mmomtchev
Copy link
Author

@shadowspawn this was precisely the point of this PR - use the ESM versions since these were the only ones available. It didn't break the CJS build, because the transpiler took care of it. But nevermind if you have another PR.

@shadowspawn
Copy link
Member

Thanks for info. I'll take another look.

@shadowspawn
Copy link
Member

Nice, the rollup configuration does work! This is a better direction than I went in #140 where I used the cjs libraries for the esm build to avoid breaking the cjs build.

There are a number of fixes required to the PR. The biggest one is that test:esm was not being included in npm run test and is not functional. I tried adding the import for chai to at least get should functional (as follows), and multiple tests still broken. The early failures are because the wrapping is now working, but there are a couple of later ones too that I have not looked into.

import chai from 'chai'
chai.should()

Are you interested in picking this up again @mmomtchev ? No worries if you are no longer interested, in which case I'll do some combo of this and my attempt and add you as co-author. (Thanks!)

@mmomtchev
Copy link
Author

@shadowspawn Don't you have another overlapping PR?

@shadowspawn
Copy link
Member

Yes, but your PR was first so I'll help you with yours if you like.

@mmomtchev
Copy link
Author

@shadowspawn Maybe I should have started by introducing myself. I have been unemployed for 3 years and I am living with $4 per day - every day I have to make a meal with $2.5. I don't do anything but to work on open-source projects, hoping that I will find a job. I have to do this, despite having 20 years of experience, despite having graduated top of my class and despite having worked incredibly hard every day during my whole life. I am in this situation because I refuse to accept that my supervisor posts dicks and tries to make it look like I was suffering from schizophrenia. My supervisors have been doing this for the last 5 years before my unemployment period. They have been doing this in order to cover up a false rape accusation organized with the help of the French police.

Since I have been unemployed, I had to take this same shit in every open-source project I tried to work in. It happened in the @OSGeo projects, then it happended in the @openjsf projects. Because simply posting dicks is not enough, they will usually simultaneously do something to impede my work - and in a way that clearly shows the criminal conspiracy.

I would just like to ask you what is your opinion on this?

@shadowspawn
Copy link
Member

It sounds like you have had a difficult time.

I don't think it is worth you working on this old PR. I'll rework my PR and your rollup config and do some new PRs. I want to look at the test setup anyway. I will add you as co-author on the PR with the dependency update and rollup changes.

Thanks for the contribution, and sorry it took a while to rediscover it.

@mmomtchev
Copy link
Author

Tell me @shadowspawn, because I am eating with $2.50 per meal and I wouldn't do what you are doing, tell me, please, what is the situation with you

@shadowspawn
Copy link
Member

See #143 for an updated PR following through with the approach of updating the dependencies to ESM and using rollup to transpile for the CJS implementation.

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