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!: adds support for ESM and Deno #1708

Merged
merged 24 commits into from Aug 22, 2020
Merged

feat!: adds support for ESM and Deno #1708

merged 24 commits into from Aug 22, 2020

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Aug 6, 2020

Using the work in yargs-parser as a starting point, this PR begins refactoring yargs such that it can easily target ESM, Deno, and CJS.

TODO:

  • isolate modules pulled in via require.
  • isolate system modules (such that alternate modules can be used for Deno or browser).
  • get tests passing.
  • move y18n to ESM. I'm happy to land this as an improvement after this PR.
  • move cliui to ESM just need to publish.
  • module ends up being a fairly ridiculous 396.3k, is there anything we can do to reduce this size? with the decision not to yet ship typings, we get the unpacked size down to 327.7 kB and the packed size to 78.7 kB. It would be good to reduce this more, but it's a start:
    * I've started minifying the build/index.cjs output; this gets us down to 248kb, this is reasonable given the current published version is 230kb.
    * @mikeal convinced me that the annoyance of a minified build probably isn't worth the reduced package size, let's stick with the 327.7kb for now.
    * by removing comments from the build target, without minifying, the target size becomes unpacked size: 286.3 kB , this is very close to the current published size of 231kb.
  • ensure the API surface is close to identical for index.js/yargs.js in Node 10, 14.
  • expose argv helper for ESM.
  • get coverage back to 100%
  • add unit tests for platform shims.
  • update documentation.

Notable Breaking Changes:

  • In Node 12+ import behavior is stricter; for instance require('yargs/yargs') will work, whereas require('yargs/yargs.js') will not.
  • find-up has been swapped out for ESM escalade (logic may vary).

@bcoe
Copy link
Member Author

bcoe commented Aug 8, 2020

Refs: #1706

index.mjs Outdated
import { YargsFactory } from './build/lib/yargs-factory.js'

// TODO: stop using createRequire as soon as we've ported: cliui, y18n, etc.
import { createRequire } from 'module';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QmarkC if once we port y18n and cliui we'll be getting darn close to an ESM version of yargs.

Note that createRequire is Node.js specific, so won't port well to other runtimes.

yargs.js Outdated Show resolved Hide resolved
@bcoe bcoe mentioned this pull request Aug 9, 2020
8 tasks
@bcoe bcoe marked this pull request as ready for review August 22, 2020 16:57
@bcoe bcoe merged commit ac6d5d1 into master Aug 22, 2020
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.

None yet

1 participant