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

Road to ESM/Deno support #1706

Closed
8 tasks done
bcoe opened this issue Aug 5, 2020 · 15 comments
Closed
8 tasks done

Road to ESM/Deno support #1706

bcoe opened this issue Aug 5, 2020 · 15 comments

Comments

@bcoe
Copy link
Member

bcoe commented Aug 5, 2020

With ESM modules landing in Node 14, our ongoing effort to convert fully to TypeScript (which easily targets ESM), and Deno (which leverages ESM and TypeScript) reaching v1, I think it's worth the effort to try to build yargs such that it targets all these environments.

I did an initial spike on yargs-parser and was able to achieve this goal.

Known limitations/challenges:

  • shipping both CJS and ESM increases the size of the package (this limitation will go away in the future when ESM is more widely adopted and we can drop CJS).
  • TypeScript and Deno import files differently, which requires some fiddling during the build process.
  • Deno, Node.js, and the browser have different standard libraries, which requires some additional fiddling...

All this said, I think it's a worthwhile goal to target these other environments. The work to be done is as follows.

In Yargs Itself

In Dependencies

  • convert yargs-parser to TypeScript/ESM/Deno (thanks @QmarkC).
  • convert y18n to TypeScript/ESM/Deno (@bcoe/@QmarkC work in progress).
  • pull small helper libraries into yargs (which-module, set-blocking, require-main-filename, get-caller-file) (help wanted).
  • require-directory will not work well with ESM, come up with plan for replacement (help wanted). for the next major version of yargs, I would like to finally tackle async functionality in a more holistic way. If you had to await yargs.parse(), then we can use Dynamic imports for this functionality.
  • find-up switched to escalade which supports ESM.
  • convert cliui to TypeScript/ESM/Deno (help wanted). see.

CC: @QmarkC, @quilicicf

@bcoe
Copy link
Member Author

bcoe commented Aug 5, 2020

@QmarkC let me know if you're interested in taking on any of these tasks, and we can toss our names next to them as we do the work.

@quilicicf
Copy link

I'll look around and see if I'm up to any of the tasks above but that'll have to wait until September, holidays are coming :)
Glad to see this become a target for yargs though!

@bcoe
Copy link
Member Author

bcoe commented Aug 22, 2020

@quilicicf please give the following a try:

import { Yargs, YargsType, Arguments } from 'https://deno.land/x/yargs/deno.ts'

Yargs()
  .command('download <files...>', 'download a list of files', (yargs: YargsType) => {
    return yargs.positional('files', {
      describe: 'a list of files to do something with'
    })
  }, (argv: Arguments) => {
    console.info(argv)
  })
  .strictCommands()
  .demandCommand(1)
  .parse(Deno.args)

It's not 100% feature compatible with Node.js:

  • Deno doesn't yet have a non-experimental way to figure out the terminal width (I don't believe).
  • we haven't ported y18n to ESM/Deno yet; so it only supports English .
  • there's not a strategy for how we'd support command directories yet, I believe this will require yargs moving to an async interface, which I believe should be our plan for v17 now that top level await is a thing.

But any ways, I think this is a good start.

@quilicicf
Copy link

Awesome!
I'll give it a spin ASAP thanks!

@quilicicf
Copy link

As for the terminal width, there have been discussions about it and I've taken the time to follow the thread.

It's been implemented in Deno 1.3.1 but is still unstable.

You can currently get the terminal size with:

Deno.consoleSize(Deno.stdout.rid); // returns something like { columns: 93, rows: 50 }

But you'd have to run the script with:

deno run --unstable yourscript.ts

I have found no clue about whether/when this can land in the stable features but one can always ask :)

The PR that added the feature for reference: denoland/deno#6520

There are other solutions but this is the most elegant as it will eventually be portable (I think it currently does not work on windows). You can for example use Deno.run like:

const cmd = Deno.run({ cmd: [ 'tput', 'cols'], stdout:'piped' });
const columnsUintArray = await cmd.output();
const columnsString = new TextDecoder().decode(columnsUintArray);
cmd.close();

But you'll need to use something other than tput on systems that don't have it and it is hacky anyway :-(

@bcoe
Copy link
Member Author

bcoe commented Aug 27, 2020

@quilicicf I was thinking we probably just wait for consoleSize to be stable?

@quilicicf
Copy link

This would be the safest option IMO too yes 👍
Deno seems to progress pretty fast, hopefully this won't take long.
And it's fine for the port of yargs to Deno to follow Deno's own pace :-)

@tshinnic
Copy link

In beginning project with package.json "type": "module", I installed yargs@next "v16.0.0-deno.beta.1" (which I think is "modified-api-surface"?)

Tried out .command(module) api, as indeed require-directory completely dependent on module.require and .commandDir() is unavailable. The below is working for me:

main.js:

import futhark from './futhark.js'
...
  .command(futhark)


futhark.js:

const command = 'futhark [rune]' 

const describe = 'rune faster'

const builder = {
//  elder: {
//    default: true
//  },
}

const handler = function (argv) {
  // do something with argv
  console.log('  futhark():', argv)
}

const module = {
  command,
  describe,
  builder,
  handler,
}

export default module

Thank you for this timely (for me anyway) work on enabling Yargs for ESM.

Will this issue be the center for news on ESM work? I barely caught the transition to
let argv = yargs(hideBin(process.argv)) ...

@bcoe
Copy link
Member Author

bcoe commented Sep 1, 2020

Will this issue be the center for news on ESM work? I barely caught the transition to

Yes, and I expect there will be a couple more beta releases before we publish for real.

@quilicicf
Copy link

quilicicf commented Sep 2, 2020

@bcoe I tried taking Yargs for a spin with Deno and I'm facing some weird issue.

My code runs OK when run via deno run but deno install fails miserably.

The trace:

deno install --allow-run --allow-read="$FORGE" index.ts
Download https://deno.land/std/fmt/colors.ts
...
Check file:///home/cyp/talend/forge/github/quilicicf/Gut/index.ts
error: TS2614 [ERROR]: Module '"https://deno.land/x/yargs@v16.0.0-deno.beta.1/deno"' has no exported member 'YargsType'. Did you mean to use 'import YargsType from "https://deno.land/x/yargs@v16.0.0-deno.beta.1/deno"' instead?
import { YargsType } from 'https://deno.land/x/yargs/deno.ts';
         ~~~~~~~~~
    at file:///home/cyp/talend/forge/github/quilicicf/Gut/src/commands/simple/pile.ts:1:10
...
+ Three more errors like the one above but in index.ts

My code is here: https://github.com/quilicicf/Gut/tree/master_deno

I've had a look at https://deno.land/x/yargs@v16.0.0-deno.beta.1/deno and I don't have a clue what's going on ATM. Any idea? Or should I open an issue on Deno's repo?

@bcoe
Copy link
Member Author

bcoe commented Sep 2, 2020

@quilicicf types are in their own file:

import yargs from 'https://deno.land/x/yargs/deno.ts'
import { Arguments, YargsType } from 'https://deno.land/x/yargs/types.ts'

yargs()
  .command('download <files...>', 'download a list of files', (yargs: YargsType) => {
    return yargs.positional('files', {
      describe: 'a list of files to do something with'
    })
  }, (argv: Arguments) => {
    console.info(argv)
  })
  .strictCommands()
  .demandCommand(1)
  .parse(Deno.args)

@quilicicf
Copy link

Ooooh, it's changed a bit since your first code snippet apparently.
The Deno integration in IntelliJ is misleading here, I ctrl-clicked the import URL (https://deno.land/x/yargs/deno.ts) and it pointed me to what seems to be an old cached file that was exposing the types.
Thanks for the answer. I'll continue testing Yargs. Looks fine to me up to now :-)

@bcoe
Copy link
Member Author

bcoe commented Sep 5, 2020

@tshinnic I would just install 16.0.0-beta.1, the Deno version is just a tag that includes the build directory.

@bcoe
Copy link
Member Author

bcoe commented Sep 11, 2020

this work is shipped in v16.

@bcoe bcoe closed this as completed Sep 11, 2020
@felipecrs felipecrs mentioned this issue Sep 17, 2020
2 tasks
@ffMathy
Copy link

ffMathy commented Nov 2, 2020

Doesn't work anymore. See #1794

rivy added a commit to rivy/js.os-paths that referenced this issue Dec 25, 2020
- ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
- ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)

## discussion

As suggested in "Node Modules at War", an ESM wrapper is used to provide basic ESM
support. However, `deno` is unable to use that ESM module, failing to load the
transitive CJS module with `'../index.js' does not provide an export named 'default'`.
So, `deno` is instead referred to a full ESM transpile.

As `deno` is it's own animal, there is no issue with CJS and ESM referring to differing
modules, either for installation or creating subtle code issues.
rivy added a commit to rivy/js.os-paths that referenced this issue Dec 26, 2020
- ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
- ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)

## discussion

As suggested in "Node Modules at War", an ESM wrapper is used to provide basic ESM
support. However, `deno` is unable to use that ESM module, failing to load the
transitive CJS module with `'../index.js' does not provide an export named 'default'`.
So, `deno` is instead referred to a full ESM transpile.

As `deno` is it's own animal, there is no issue with CJS and ESM referring to differing
modules, either for installation or creating subtle code issues.
rivy added a commit to rivy/js.os-paths that referenced this issue Dec 28, 2020
- adds an ESM wrapper to provide simple ESM support (as suggested in "Node Modules at War"

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)
rivy added a commit to rivy/js.os-paths that referenced this issue Dec 28, 2020
- adds an ESM wrapper to provide simple ESM support (as suggested in "Node Modules at War"

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)
rivy added a commit to rivy/js.os-paths that referenced this issue Jan 1, 2021
- adds an ESM wrapper to provide simple ESM support (as suggested in "Node Modules at War"

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)
rivy added a commit to rivy/js.os-paths that referenced this issue Jan 1, 2021
- adds an ESM wrapper to provide simple ESM support (as suggested in "Node Modules at War"

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)
rivy added a commit to rivy/js.os-paths that referenced this issue Jan 2, 2021
- adds an ESM wrapper to provide simple ESM support (as suggested in "Node Modules at War"

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706)
rivy added a commit to rivy/js.xdg-portable that referenced this issue Jan 31, 2021
- reveals ESM wrapper providing ESM support (as suggested in "Node Modules at War")

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706).
rivy added a commit to rivy/js.xdg-portable that referenced this issue Feb 10, 2021
- reveals ESM wrapper providing ESM support (as suggested in "Node Modules at War")

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706).
rivy added a commit to rivy/js.xdg-app-paths that referenced this issue Feb 16, 2021
- reveals ESM wrapper providing ESM support (as suggested in "Node Modules at War")

* ref: [Node Modules at War](https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1) @@ <https://archive.is/zl0qg>
* ref: [yargs ~ 'Road to ESM/Deno'](yargs/yargs#1706).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants