Wrapping logic seems to be weirdly different between ESM vs CJS versions #138

isaacs · 2023-04-29T19:15:19Z

Example:

const cliuiCJS = require('cliui')
import('cliui').then(({ default: cliuiESM }) => {
  const uiCJS = cliuiCJS({})
  const uiESM = cliuiESM({})

  const text = `usage: git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] [-e]
               <tagname> [<commit> | <object>]
   or: git tag -d <tagname>...
   or: git tag [-n[<num>]] -l [--contains <commit>] [--no-contains <commit>]
               [--points-at <object>] [--column[=<options>] | --no-column]
               [--create-reflog] [--sort=<key>] [--format=<format>]
               [--merged <commit>] [--no-merged <commit>] [<pattern>...]
   or: git tag -v [--format=<format>] <tagname>...`

  console.error('~ ~ ~ ~ ~ ~ ~ ~ ~ ~')
  console.error('cjs')
  uiCJS.div({
    padding: [0, 0, 0, 0],
    text,
  })
  console.log(uiCJS.toString())
  console.error('~ ~ ~ ~ ~ ~ ~ ~ ~ ~')
  console.error('esm')
  uiESM.div({
    padding: [0, 0, 0, 0],
    text,
  })
  console.log(uiESM.toString())
})

/* output:
~ ~ ~ ~ ~ ~ ~ ~ ~ ~
cjs
usage: git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] [-e]
<tagname> [<commit> | <object>]
or: git tag -d <tagname>...
or: git tag [-n[<num>]] -l [--contains <commit>] [--no-contains <commit>]
[--points-at <object>] [--column[=<options>] | --no-column]
[--create-reflog] [--sort=<key>] [--format=<format>]
[--merged <commit>] [--no-merged <commit>] [<pattern>...]
or: git tag -v [--format=<format>] <tagname>...
~ ~ ~ ~ ~ ~ ~ ~ ~ ~
esm
usage: git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] [-e]

       <tagname> [<commit> | <object>]
   or: git tag -d <tagname>...
   or: git
 tag [-n[<num>]] -l [--contains <commit>] [--no-contains <commit>]

  [--points-at <object>] [--column[=<options>] | --no-column]
               [--
create-reflog] [--sort=<key>] [--format=<format>]
               [--merged <comm
it>] [--no-merged <commit>] [<pattern>...]
   or: git tag -v [--format=<format>]
 <tagname>...
 */

I'm not sure why, but the esm results seem to wrap at very confusing places, differently from the cjs version.

isaacs · 2023-04-29T19:21:48Z

Ah, it looks like the cjs version is setting up the Mixin with different values:

const stringWidth = require('string-width');
const stripAnsi = require('strip-ansi');
const wrap = require('wrap-ansi');
function ui(opts) {
    return cliui(opts, {
        stringWidth,
        stripAnsi,
        wrap
    });
}

Whereas the mjs does this:

import { wrap, stripAnsi } from './build/lib/string-utils.js'

export default function ui (opts) {
  return cliui(opts, {
    stringWidth: (str) => {
      return [...str].length
    },
    stripAnsi,
    wrap
  })
}

So the difference appears to be that the functions in string-utils.ts are not equivalent.

isaacs · 2023-04-29T19:26:00Z

Confirmed, this fixes the issue:

diff --git a/index.mjs b/index.mjs
index bc7a022..ccb76e7 100644
--- a/index.mjs
+++ b/index.mjs
@@ -1,6 +1,8 @@
 // Bootstrap cliui with CommonJS dependencies:
 import { cliui } from './build/lib/index.js'
-import { wrap, stripAnsi } from './build/lib/string-utils.js'
+import { stripAnsi } from './build/lib/string-utils.js'
+import { createRequire } from 'module'
+const wrap = createRequire(import.meta.url)('wrap-ansi')
 
 export default function ui (opts) {
   return cliui(opts, {

I'm guessing the reason for not using wrap-ansi in this way is to support Deno and browsers, which do not have createRequire. But the wrap method in string-utils is not working properly, so there's a bug there.

It'd probably be best to ditch wrap-ansi entirely (including in the cjs version) and fix the wrap method in string-utils.ts, yes?

isaacs · 2023-04-29T19:36:15Z

Ugh, and it'd be trivial to update strip-ansi, wrap-ansi, and string-width to ESM, but that would drop support for cjs, because those modules are ESM-only.

Can do it with a npm alias, idk if that's too weird, but it would at least fix the issue.

This also ports some of the `// istanbul ignore` comments to their associated `/* c8 ignore start/stop */` equivalents, and coverage-ignores some value fallbacks that are there for safety but can never be hit in normal usage. Fix: yargs#138

shadowspawn · 2023-04-29T21:30:41Z

See also: #89

Related: yargs/yargs#2112 (comment)

isaacs · 2023-04-30T01:28:58Z

Fixed by #139

This also ports some of the `// istanbul ignore` comments to their associated `/* c8 ignore start/stop */` equivalents, and coverage-ignores some value fallbacks that are there for safety but can never be hit in normal usage. Fix: yargs#138

Waiting on yargs/cliui#139 Re: yargs/cliui#138

Netail · 2024-03-06T13:16:59Z

Any update on this? @shadowspawn (PR #143 ) Experiencing the following issue with jest;

Error [ERR_REQUIRE_ESM]: require() of ES Module /.../node_modules/string-width/index.js from /.../node_modules/cliui/build/index.cjs not supported.

shadowspawn · 2024-03-06T19:58:42Z

No update

Interested in knowing more about your context to understand what the error message might be about, and whether the proposed PR will help. This issue is about cliui not wrapping when used from esm which is due to complications around string-width and cjs/esm but does not (normally) cause runtime import errors. Just lack of wrapping.

Is your project pulling in cliui (e.g. v8.0.1) or @isaacs/cliui (v8.0.2)? If your project is pulling in @isaacs/cliui, are you using Yarn v1? Do you have a public repo where I can try and reproduce?

Netail · 2024-03-06T20:19:07Z

So our project uses jest for unit testing, where a lower level dependency uses string-width.

Error [ERR_REQUIRE_ESM]: require() of ES Module <dir>/node_modules/string-width/index.js from <dir>/node_modules/cliui/build/index.cjs not supported.
Instead change the require of index.js in <dir>/node_modules/cliui/build/index.cjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (<dir>/node_modules/cliui/build/index.cjs:291:21)
    at Object.<anonymous> (<dir>/node_modules/yargs/build/index.cjs:1:60678)
    at Object.<anonymous> (<dir>/node_modules/yargs/index.cjs:5:30)
    at _yargs (<dir>/node_modules/jest-cli/build/run.js:30:39)
    at buildArgv (<dir>/node_modules/jest-cli/build/run.js:149:26)
    at Object.run (<dir>/node_modules/jest-cli/build/run.js:124:24)
    at Object.<anonymous> (<dir>/node_modules/jest-cli/bin/jest.js:16:17)
    at Object.<anonymous> (<dir>/node_modules/jest/bin/jest.js:12:3)

I can create a small test environment

Netail · 2024-03-06T20:46:35Z

I've created a test environment; https://github.com/Netail/jest-string-width
yarn install -> yarn test

shadowspawn · 2024-03-06T21:07:10Z

Great, thanks @Netail , I'll give that a try in next couple of days.

(I can see in the yarn.lock both cliui and @isaacs/cliui, so first question answered already.)

shadowspawn · 2024-03-07T22:31:57Z

Reproduced with Yarn 1. The Jest problem is triggered by the use of aliases in #139 which is published in @isaacs/cliui. Yarn is getting confused with the aliased (string-width-cjs) and unaliased (string-width) packages.

Details

I was able to get a working install by deleting the node_modules file and creating an empty yarn.lock file before installing with yarn 1.22.21, but I suspect it is fragile and not a reliable fix.

This output is from the example repo and an install with the original yarn.lock.

The only installed package that uses alias string-width-cjs is @isaacs/cliu. Yarn thinks multiple packages are using string-width-cjs, but of course they don't and in their code they require string-width and break when they get a newer esm-only version.

% yarn why string-width
yarn why v1.22.21
[1/4] 🤔  Why do we have the module "string-width"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "string-width@5.1.2"
info Reasons this module exists
   - "eslint-config-next#@next#eslint-plugin-next#glob#jackspeak#@isaacs#cliui" depends on it
   - Hoisted from "eslint-config-next#@next#eslint-plugin-next#glob#jackspeak#@isaacs#cliui#string-width"
   - Hoisted from "eslint-config-next#@next#eslint-plugin-next#glob#jackspeak#@isaacs#cliui#wrap-ansi#string-width"
info Disk size without dependencies: "220KB"
info Disk size with unique dependencies: "328KB"
info Disk size with transitive dependencies: "348KB"
info Number of shared dependencies: 4
✨  Done in 0.15s.
% yarn why string-width-cjs
yarn why v1.22.21
[1/4] 🤔  Why do we have the module "string-width-cjs"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "string-width-cjs@4.2.3"
info Reasons this module exists
   - "jest#jest-cli#yargs" depends on it
   - Hoisted from "jest#jest-cli#yargs#string-width-cjs"
   - Hoisted from "jest#jest-cli#yargs#cliui#string-width-cjs"
   - Hoisted from "eslint-config-next#@next#eslint-plugin-next#glob#jackspeak#@isaacs#cliui#string-width-cjs"
   - Hoisted from "jest#jest-cli#yargs#cliui#wrap-ansi-cjs#string-width-cjs"
info Disk size without dependencies: "20KB"
info Disk size with unique dependencies: "128KB"
info Disk size with transitive dependencies: "148KB"
info Number of shared dependencies: 4

The suspect entry in the yarn.lock file is:

"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
  name string-width-cjs
  version "4.2.3"
  resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
  integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
  dependencies:
    emoji-regex "^8.0.0"
    is-fullwidth-code-point "^3.0.0"
    strip-ansi "^6.0.1"

isaacs · 2024-03-08T22:15:51Z

@arcanis This is another example of the "yarn 1 deduplicating aliased packages incorrectly" issue.

@Netail I have not checked, but I'm willing to be that this would not be a problem with any other package manager. Try using npm, pnpm, or latest yarn (v4). Yarn 1 should not be used, as long as this bug prevents it from correctly resolving dependency graphs.

Netail · 2024-03-09T20:34:01Z

When installing yarn (npm i -g yarn) it uses v1 by default. (Meaning a lot of people still use v1 and will potentially run into the same issue) Wouldn't it make more sense to update the string-width package, so it would work with v1?

arcanis · 2024-03-09T21:38:48Z

I tried to release a 1.22.22 release which should fix this issue. Running the reproduction in eslint/eslint#17215 (reply in thread) with yarn@1.22.22 instead of yarn@1 seems to work. Can you try to upgrade and let me know?

shadowspawn · 2024-03-09T21:58:32Z

I retested reproduction from @Netail . Looks good with yarn@1.22.22.

I needed to delete the yarn.lock (which I expected) and then install worked as expected. Checking yarn why string-width looks tidy.

For reference, the yarn fix is: yarnpkg/yarn#9023

shadowspawn · 2024-03-09T22:05:54Z

(I'll try a couple of different reproductions from #139 as well.)

Netail · 2024-03-09T22:19:06Z

I retested reproduction from @Netail . Looks good with yarn@1.22.22.

I needed to delete the yarn.lock (which I expected) and then install worked as expected. Checking yarn why string-width looks tidy.

For reference, the yarn fix is: yarnpkg/yarn#9023

Yes seems like my issue has been resolved with v1.22.22, that's great! Thank you guys very much :)

shadowspawn · 2024-03-09T22:29:02Z

I reproduced problems with packages used in #139 (comment) with yarn@1.22.21, and fixed with yarn@1.22.22. (rimraf and web-ext and yargs.)

callmeteus · 2024-04-18T20:18:33Z

Just for a feedback, I've had this problem and upgrading to yarn 1.22.22 as cited worked perfectly.

isaacs linked a pull request Apr 29, 2023 that will close this issue

fix: handle strings the same in cjs, esm, and deno #139

Open

isaacs added a commit to isaacs/jackspeak that referenced this issue May 1, 2023

add examples, temporarily use git fork of cliui

3f72027

Waiting on yargs/cliui#139 Re: yargs/cliui#138

This was referenced May 13, 2023

feat!: implement smart wrapping for all contexts #140

Closed

cliui remove kleur colors (ESM project) #113

Open

shadowspawn mentioned this issue May 24, 2023

feat: add proper wrapping for esm and Deno by updating dependencies #143

Open

nijinekoyo mentioned this issue Jan 16, 2024

cliui dependency error alex8088/electron-vite#383

Closed

4 tasks

Netail mentioned this issue Mar 6, 2024

[Bug]: CommonJS / ESM modules issue jestjs/jest#14881

Closed

gustavohenke mentioned this issue Mar 10, 2024

Build failed when using concurently open-cli-tools/concurrently#432

Closed

Netail mentioned this issue Mar 26, 2024

bump: yarn to 1.22.22 dependabot/yarn-lib#56

Merged

TooTallNate mentioned this issue Apr 5, 2024

Error [ERR_REQUIRE_ESM]: require() of ES Module from vercel dependency vercel/vercel#11102

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrapping logic seems to be weirdly different between ESM vs CJS versions #138

Wrapping logic seems to be weirdly different between ESM vs CJS versions #138

isaacs commented Apr 29, 2023

isaacs commented Apr 29, 2023 •

edited

isaacs commented Apr 29, 2023

isaacs commented Apr 29, 2023

shadowspawn commented Apr 29, 2023 •

edited

isaacs commented Apr 30, 2023

Netail commented Mar 6, 2024 •

edited

shadowspawn commented Mar 6, 2024

Netail commented Mar 6, 2024

Netail commented Mar 6, 2024

shadowspawn commented Mar 6, 2024 •

edited

shadowspawn commented Mar 7, 2024

isaacs commented Mar 8, 2024

Netail commented Mar 9, 2024 •

edited

arcanis commented Mar 9, 2024

shadowspawn commented Mar 9, 2024 •

edited

shadowspawn commented Mar 9, 2024

Netail commented Mar 9, 2024 •

edited

shadowspawn commented Mar 9, 2024

callmeteus commented Apr 18, 2024

Wrapping logic seems to be weirdly different between ESM vs CJS versions #138

Wrapping logic seems to be weirdly different between ESM vs CJS versions #138

Comments

isaacs commented Apr 29, 2023

isaacs commented Apr 29, 2023 • edited

isaacs commented Apr 29, 2023

isaacs commented Apr 29, 2023

shadowspawn commented Apr 29, 2023 • edited

isaacs commented Apr 30, 2023

Netail commented Mar 6, 2024 • edited

shadowspawn commented Mar 6, 2024

Netail commented Mar 6, 2024

Netail commented Mar 6, 2024

shadowspawn commented Mar 6, 2024 • edited

shadowspawn commented Mar 7, 2024

isaacs commented Mar 8, 2024

Netail commented Mar 9, 2024 • edited

arcanis commented Mar 9, 2024

shadowspawn commented Mar 9, 2024 • edited

shadowspawn commented Mar 9, 2024

Netail commented Mar 9, 2024 • edited

shadowspawn commented Mar 9, 2024

callmeteus commented Apr 18, 2024

isaacs commented Apr 29, 2023 •

edited

shadowspawn commented Apr 29, 2023 •

edited

Netail commented Mar 6, 2024 •

edited

shadowspawn commented Mar 6, 2024 •

edited

Netail commented Mar 9, 2024 •

edited

shadowspawn commented Mar 9, 2024 •

edited

Netail commented Mar 9, 2024 •

edited