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

Harmony flags #1846

Closed
danielstjules opened this issue Aug 26, 2015 · 20 comments
Closed

Harmony flags #1846

danielstjules opened this issue Aug 26, 2015 · 20 comments

Comments

@danielstjules
Copy link
Contributor

Rather than exposing individual node/iojs harmony flags, what if we were to expose only a single --harmony flag that enabled all harmony features for that environment?

@jkresner
Copy link

This is a bad idea. The harmony flag has different behaviors across node versions.

@jkresner
Copy link

What about a feature that folks can do --node-anything and mocha attempts to append anything when invoking node?

@danielstjules
Copy link
Contributor Author

At the very least, we could be smart about harmony and es_staging. Either way, all these options shouldn't have to be hardcoded in mocha anymore

@jkresner
Copy link

Should have some sort of interim solution soon though. Gonna have a swarm of people moving off traceur and babel and their code won't run on mocha.

@danielstjules
Copy link
Contributor Author

Should have some sort of interim solution soon though.

Agreed. I don't think we'll be releasing a patch within the next day though, so happy to discuss an alternative :)

@danielstjules
Copy link
Contributor Author

Automatically passing along any arguments that start with --harmony would be a good start I think. I just want something rather future proof so we don't have to be reactive as new flags are released.

@jkresner
Copy link

Since mocha runs on more than just node, I think it's not its responsibility to intercept and interpret. My gut says, let people just pass node args in some format and their own environment do the work.

@danielstjules
Copy link
Contributor Author

As long as we're handling unrecognized flags, I'd be happy with just passing things along. Looks like it could just piggyback off node's own handling for versions > 0.10 :) But with node 0.10 and 0.8, it'll still run and exit with a 0 error code. Though it wouldn't break anything.

$ cat test.js
console.log('testing');

$ nvm use 0.12
Now using node v0.12.0

# Positive error code, perfect
$ node --invalid test.js
node: bad option: --invalid

$ nvm use 0.10
Now using node v0.10.20

# 0 error code, still runs :(
$ node --invalid test.js
Error: unrecognized flag --invalid
Try --help for options
testing

@danielstjules
Copy link
Contributor Author

A more complex solution would be to parse node --help and node --v8-options to ensure that it's a valid flag. That would work for node 0.10 as well. But maybe we're fine with the behavior I highlighted in #1846 (comment)

@jkresner
Copy link

I just don't think you want to be responsible for maintaining logic outside the scope of what a library's real purpose is.

I wouldn't let people pass anything to mocha, but something like

mocha index.js --node-harmony_destructuring

Then attempt to invoke node in some catch and if it fails, you can output from mocha that the node option --harmony_destructuring doesn't existing for their environment. Then you no longer need to concern yourself with maintaining around framework upgrades of if people do weird custom things to their environment.

@boneskull
Copy link
Member

I'm of the opinion we should do none of it, and if people need flags, invoke mocha with the node executable

@danielstjules
Copy link
Contributor Author

Ah, sorry - I misunderstood your original suggestion. So you're thinking just prefixing all node flags? So --node-harmony_destructuring would pass --harmony_destructuring? I feel like people would get tripped up by that pretty frequently. I'd rather be able to pass --harmony_destructuring to mocha, just as I would with node.

@jkresner
Copy link

@boneskull might it alienate / make is hard for some people to get going? Many are going to want to write in ES6 shortly.

@jkresner
Copy link

@danielstjules I suggest that because

  1. (I think) You want to differentiate between mocha (behavior) and node (settings) options
  2. What happens if you run mocha from the browser with the --harmony_destructuring flag?
  3. Is it possible to have a conflict with naming of mocha options and node options?

@danielstjules
Copy link
Contributor Author

I'm of the opinion we should do none of it, and if people need flags, invoke mocha with the node executable

@boneskull That doesn't currently work though?

$ cat test.js
(() => console.log('test'))();

$ node --version
v0.12.0

$ node --harmony_arrow_functions test.js
test

$ node --harmony_arrow_functions bin/mocha test.js
/Users/danielstjules/git/mocha/test.js:1
(function (exports, require, module, __filename, __dirname) { (() => console.l
                                                                ^
SyntaxError: Unexpected token )
...

$ bin/mocha --harmony_arrow_functions test.js
test



  0 passing (1ms)

You want to differentiate between mocha and node options

I suppose, though there's no collisions between any of the harmony-related flags between node/iojs and mocha. So I'm not sure that I agree. I personally see it as an issue of "one more detail I gotta remember". Was it --harmony_destructuring? Nope, mocha requires a prefix, so it's --node-harmony_destructuring. I feel like that would annoy me as a developer trying to setup a project.

What happens if you run mocha from the browser with that flag?

How are you passing these flags to the browser build of mocha?

Is it possible to have a conflict with naming of mocha options and node options?

Not for any harmony-related flags. And considering it'd only be for long flags and not short flags, I don't think there's any existing conflicts other than --version and --help.

@boneskull
Copy link
Member

@boneskull That doesn't currently work though?

@danielstjules Correct, this doesn't work.

@boneskull might it alienate / make is hard for some people to get going? Many are going to want to write in ES6 shortly.

@jkresner I'm not sure what's so difficult about it? Furthermore--for the task-runner-phobic out there--you can easily put this in the scripts prop of your package.json and invoke it with npm run. I think it even supports arguments now.

I'm not sure I'd use the word "alienate". Any change of this magnitude would happen in a major release. Before we break changes for v3.0.0, I'd like to deprecate--with warnings--as much as we can.

At any rate, it's a maintenance headache, and I'm open to other solutions. I'm not super hot on the prefixing idea. Maybe something like what the -o option of the mount command does:

$ mount -t ext3 -o ro,user,noauto,unhide

Perhaps:

$ mocha --node harmony-proxies,harmony_atomics test/

Some of the .deb build scripts also do this weird double-colon stuff. I'd prefer even that over "subargs".

Ideally, I would like to avoid hardcoding flags and having two executables, which is why I suggest dropping it altogether.

I understand that test frameworks are a special case in the grand scheme of CLI-things. That being said, I can't think of another module off the top of my head having two executables with the sole purpose of passing NodeJS flags. There must be a better way...

@danielstjules
Copy link
Contributor Author

@boneskull For a non-breaking change, would you be opposed to #1846 (comment) ? It would eliminate the need to update mocha and publish patch releases each time a flag is introduced. And if you don't see this as something you'd like to support going forward, it would only be necessary until 3.0.0 was released.

@danielstjules
Copy link
Contributor Author

Pushed a simplified PR with #1885

@jamestalmage
Copy link

On a (kinda) related note, I just wrote node-version-cli.

Example: use babel only if node <4.0.0

$ npm install -g node-version-cli
$ mocha $(node-version --lt-4.0.0 --compilers js:babel/register)

I use it so I get nice clean stack-traces on Node 4+, but can still run my CI server against older versions of Node.

@drazisil
Copy link

This has a merged PR and no activity in 2.5 years. Closing.

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

5 participants