Implement liftoff into grunt-cli #117
Conversation
|
|
|
FYI, I've been using this branch as my |
bin/grunt
Outdated
| var options = nopt(known, aliases, process.argv, 2); | ||
|
|
||
| if (options.version) { | ||
| console.log('grunt-cli v' + pkg.version); |
There was a problem hiding this comment.
I think you need to exit after this line as it will cause an exception in the line 53. Here is what I got when I run grunt --version:
$ grunt --version
grunt-cli v1.2.0
assert.js:84
throw new assert.AssertionError({
^
AssertionError: missing path
at Module.require (module.js:502:3)
at require (internal/module.js:20:19)
at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/bin/grunt:53:17)
at Liftoff.execute (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:203:12)
at module.exports (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/flagged-respawn/index.js:51:3)
at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:195:5)
at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:170:7)
at _combinedTickCallback (internal/process/next_tick.js:73:7)
at process._tickCallback (internal/process/next_tick.js:104:9)
There was a problem hiding this comment.
Note that I runned that command in an arbitrary folder which doesn't have Grunt installed to its node_modules.
There was a problem hiding this comment.
I think that would be fixed by doing the same as the original version of the CLI: exit if the modulePath is not defined and the user requested the version of the CLI.
There was a problem hiding this comment.
Sweet, fixed. Thanks for the review!
bin/grunt
Outdated
|
|
||
| // Do stuff based on CLI options. | ||
| if ('completion' in options) { | ||
| completion.print(options.completion); |
There was a problem hiding this comment.
Are we going to drop the completion command line flag? If someone was configuring the auto-completion as pointed in the README, they will only see Grunt trying to run their tasks directly after opening a new terminal session, which will result in an exception simular to the one in my other comment.
package.json
Outdated
| "interpret": "^1.1.0", | ||
| "liftoff": "^2.5.0", | ||
| "nopt": "^4.0.1", | ||
| "v8flags": "^3.0.1" |
There was a problem hiding this comment.
are you sure you want to change from ~ to ^?
|
@shama, @vladikoff, @Arkni, ya'll are saints. Thank you for working on this! |
|
One part I'm having second thoughts about is whether to support all the v8 flags. At the moment, v8flags needs to shell out The alternative is someone could do |
If it's a really tiny hit that's fine. |
|
@shama That perf hit is only incurred once, we cache the output and read it from a file after subsequent runs. |
|
Is anything holding this issue back? It sounds great! |
|
Soon. |
* grunt-cli updated to nopt 4.x in gruntjs/grunt-cli#117 * grunt-cli updated to nopt 5.0 in gruntjs/grunt-cli#156 * changelog: https://github.com/npm/nopt/blob/v5.0.0/CHANGELOG.md The simple usage of `nopt(known, alias, argv, 2)` has remained unchanged through both changes, same as in this repo. Installation of latest grunt- produces a warning: > npm WARN deprecated [email protected]: This package is no longer supported. This package was used by nopt to call `os.homedir()` which has been built-in since Node.js 2. The dependency was removed in nopt 5.0.0 with npm/nopt@5c0e45b. Newer versions of nopt are available, but those raise the required Node.js engine level. In order to make this safe to land, and easy to release, resolve the warning first by moving to nopt 5.0.0, matching grunt-cli. I'm loosening grunt-cli to allow for grunt-cli 1.5.0, which was just released with a similar fix. This way it will update by default, but not cause duplicate installations for downstream projects that depend on grunt-cli 1.4 directly.
Ref: https://github.com/gruntjs/rfcs/blob/master/text/0001-liftoff.md
This is a conservative implementation of Liftoff into grunt-cli. It should be fully backwards compatible with existing Grunt versions. It needs a bit more vetting though to be sure.
This adds the ability to write your Gruntfile in any language:
Gruntfile.ts,Gruntfile.coffee,Gruntfile.babel.jsand many others. Or if the language isnt natively supported, you can run your own:grunt --require custom-lang/register.You can also now specify v8 flags, so
grunt --harmonygets passed to the node process (maybe not as useful these days but at least it can now).Down the road, this change will allow us to remove the
coffeescriptdependency andlib/grunt/cli.js(among other related deps). When we do so, anyone who previously had aGruntfile.coffee, would need to separatelynpm install coffeescriptbut the good news is they'll no longer be stuck to our old version. But all that can wait until we do a[email protected]release.