-
Notifications
You must be signed in to change notification settings - Fork 48
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
refactor!: typescript/ESM conversion #95
Conversation
@bcoe I pulled the typescript branch and I'm working on some changes/feedback for you. |
@QmarkC awesome, want to take this PR over and I'll concentrate on yargs itself this weekend? |
Sure thing. I've got most of it done but still need to fix a couple items. I'll try to wrap it up in the next few days but might not have much time till this weekend. |
@bcoe here's my WIP, master...QmarkC:85-typescript. |
@QmarkC awesome, I don't expect I'll get yargs too much farther along until the weekend either.
Awesome 😄 In yargs' I ended up landing on the term "platform shims" rather than "mixin", which seemed perhaps a better description of what was happening (see: here). I was thinking I'd then make an effort to write unit tests for these shims... what I care about most is that yargs itself has support for a variety of platforms, and is well tested. |
For "mixin", I was just using your terminology. We could polyfill/shim with rollup plugins such as rollup-plugin-node-builtins or rollup-plugin-node-polyfills. The local files methods could be moved to another module with conditional exports based on environment or using different entry points per environment. For y18n I was thinking we could add support for loading a locale as an object via an optional config prop and/or adding an isomorphic fetch like cross-fetch, which might be better for browser support to allow loading remote locale files. |
@QmarkC the challenge here, is that yargs is poorly designed for an asynchronous model ... as soon as we require something to be I've definitely of a mind that I'd like to get to a point where yargs expects you're using
I think there's value in us isolating the platform specific functionality ourselves, at the end of the day there's not a huge amount of it, and it suddenly puts us in great position to target a bunch of runtimes ... I was just making the point that maybe I could have chosen a better variable name, and that my code was getting slightly sloppy, so refactoring into a nicer abstraction might be a good idea (especially in yargs itself). |
@QmarkC landed a first pass and integrated it: I didn't ship the types though, in case you would like to improve them 👍 (figured this way we could add the types as |
convert module to using TypeScript.
@QmarkC I'm using the pattern that I developed in
yargs-parser
, and attempting to port this module to TypeScript in such a way that we can target both ESM and Deno.My end goal is to do this with the dependency graph of yargs, such that this module would be fully compatible with ESM, Deno, and CJS.
TODO: