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

pass argv: string[] to main operation #214

Closed
cowboyd opened this issue Nov 10, 2020 · 4 comments
Closed

pass argv: string[] to main operation #214

cowboyd opened this issue Nov 10, 2020 · 4 comments

Comments

@cowboyd
Copy link
Member

cowboyd commented Nov 10, 2020

@effection/node main() is designed to implement the main method of a node process and yet we don't actually do the work of coupling to process.argv which has to be done manually, but in fact this is something that you invariably need to do.

Proposal, instead of

function main(operation: Operation<T>): Context<T>;

should be:

function main((argv: string[]) => Operation<T>): Context<T>;

That way

import { main } from '@effection/node';

main(function*() {
  yargs(process.argv);
});

becomes

import { main } from '@effection/node';

main(function*(argv) {
  yargs(argv);
});
@jnicklas
Copy link
Collaborator

Not sure I agree with this. E.g. if you're using yargs it will look at process.argv without having to pass it in.

We could totally do this, but it feels a bit unnecessary to me. Using process.argv is fine, imo.

@cowboyd
Copy link
Member Author

cowboyd commented Nov 12, 2020

There are two isues: convenience and isolation. Right now, we're forcing a program to couple to a global context for argument processing. That'll always be an option, but also passing in the arguments as an argument in this way let's your operation be used in any context, for example a unit-testing context.

@jnicklas
Copy link
Collaborator

@cowboyd given that main now also works in the browser, do you still feel we should do this, since there isn't really an equivalent to argv in the browser?

@cowboyd
Copy link
Member Author

cowboyd commented Jun 22, 2021

@cowboyd given that main now also works in the browser, do you still feel we should do this, since there isn't really an equivalent to argv in the browser?

We could say [] in the browser, but I don't feel strongly about this. I say let's not do it now since it can be a forward compatible change if we decide to do it later, but it will not be a backwards compatible change if we decide we don't want it in the future

@cowboyd cowboyd closed this as completed Jun 22, 2021
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

2 participants