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

Handling node cli options. #4

Closed
jaridmargolin opened this issue Nov 4, 2016 · 7 comments
Closed

Handling node cli options. #4

jaridmargolin opened this issue Nov 4, 2016 · 7 comments

Comments

@jaridmargolin
Copy link

So the examples clearly show (with the exception of displaying an outdated interface) how to account for v8flags. I may be overlooking something, but wouldn't it be important to handle node options as well?

Looking at the reorder function it looks like the reordering only takes into account ARG=VAL format, however node options can behave differently. For example: https://nodejs.org/api/cli.html#cli_r_require_module.

Are these intentional decisions? Perhaps I am misunderstanding or overlooking something?

@tkellen
Copy link

tkellen commented Nov 5, 2016

I believe you may be correct! That being said, we have not seen this problem in the wild and flagged-respawn is used quite extensively in the node ecosystem. If you'd be willing to write one, I would be happy to accept a PR with tests that ensures this use-case is supported.

@jaridmargolin
Copy link
Author

I would absolutely write a PR, I however don't believe the solution is trivial. The format in which the arguments are passed does not shed enough light on how they are to be parsed. For example, imagine the following args:

[ '/usr/local/bin/node',
  '/usr/local/bin/inspect.js',
  '--harmony',
  '--require',
  './test/fixtures/required',
  'success.js'
]

There is no way to determine if ./test/fixtures/required is an argument for the spawned child or an option value for the node process. The only way the reorder function would be capable of parsing this correctly, is if it knew that --require was expecting to receive a value.

I looked at how v8flags operates under the hood (haha I know you are not a huge fan of this lib), and it parses the --help command. I was thinking this could perhaps take an option to provide more "details", so rather than returning just a string of the flag name it returns detailed object that holds more information regarding how the flag behaves. If this were the case, a similar library could be written js-nodeflags. But looking at how node documents the --require option (-r, --require module to preload (option can be repeated)), it does not provide enough insight to even automate.

I have a few ideas regarding how to proceed but nothing I would consider a "good" solution.


I do have one additional question as I explore how to proceed. Why was the decision made to require the user to specify v8flags? Why not hide this complexity from the user? Including v8flags, I am assuming, is the desired behavior of 99% of users who consume this module.

@tkellen
Copy link

tkellen commented Nov 6, 2016

I hear you loud and clear RE: the complexity of dealing with all the possible permutations of arguments to the node binary. I don't envy you the task of figuring out how to deal with this!

I left v8flags out because I thought it would be useful in other contexts. I don't think it has been (yet), though.

@jaridmargolin
Copy link
Author

So I have been battling with ideas of how to best solve this for a little while now. I've done a few things:

  1. I created nodeflags which is a wrapper around v8flags that also adds node specific options (they are unfortunately hardcoded as I did not find a means to auto generating them). Additionally nodeflags alters the format to return a yargs compatible options object.
  2. In a library I authored, inspect-process, I use nodeflags in combination with yargs to pluck out the node specific arguments. Because inspect-process functions by inspecting and controlling a child_process I do not make use of js-flagged-respawn.

I am curious if the general strategy applied to inspect-process, of using yargs to pluck the properties is a valid strategy that could be used in js-flagged-respawn. It would obviously add quite a large dependency to the project.

I am at a point where I am trying to determine if the functionality I have built so far should move into js-flagged-respawn (pending your approval), or if I should continue this work in a library that sits somewhat parallel in functionality.

@tkellen
Copy link

tkellen commented Jan 4, 2017

Hey @jaridmargolin--this is really great work!

I would be interested in deprecating v8flags and replacing it with nodeflags if you can find a way to auto-generate the node options too. I think the yargs parsing belongs in whatever consumes nodeflags, though.

I also think we could offer a new minor version of flagged-respawn that uses nodeflags by default if the function arity is 2 instead of 3.

Assuming we did all of that, would it be possible for inspect-process to consume flagged-respawn?

@jaridmargolin
Copy link
Author

jaridmargolin commented Jan 4, 2017

@tkellen - There is to my knowledge not a clear path forward on autogenerating all of the flags. For example --debug-brk is not documented in node --help or in the cli options on the website. In fact the only reference I found was in the documentation under debugger. There are additional flags that exist in node --help but not the online documentation. Compiling the flags was a fairly manual process.

I attempt to fix what gets included by utilizing the version in which the flag was added. So if you are using node 5.X, nodeflags won't return flags added in node 7.X.

Additionally, the api of nodeflags manually separates "commands" from "options". In all of the use cases I have, I do not need/want "commands" returned.

This is all just a long way of saying, I think it needs to be manual unless work is done in nodejs to provide a better interface for exposing them.


As for inspect-process, It will not ever use flagged-respawn. It is doing more than just forwarding stdio and returning the status code. It works by listening to the stdout/stderrr of the child_process and acting accordingly.

My interest in flagged-respawn stems from a different project I am working on. Additionally, I just think it should be a solved problem. Mocha for example implements this on there own. And then something like this happens.

Having one relied upon solution would be better for everyone.

@phated
Copy link
Member

phated commented Nov 21, 2021

I believe this was solved by gulpjs/v8flags#55 which landed in v8flags 3.2.0

@phated phated closed this as completed Nov 21, 2021
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

No branches or pull requests

3 participants