gulpjs / liftoff

Launch your command line tool with ease.
MIT License
843 stars 52 forks source link

support optional global require for non-js filetypes #28

Closed doublerebel closed 9 years ago

doublerebel commented 9 years ago

Hello,

Would you consider an option for Liftoff that allows global require of CoffeeScript, etc., rather than limiting to requireLocal?

In some cases I write a Hacker.coffee as a utility script for a package in another language, so having a locally installed interpreter seems like overkill. I think most users who would write a script in <js-variant> likely already rely on having a globally installed interpreter for utility purposes like REPL.

Thanks for your work!

Charles

EDIT: FWIW, npm recommends that modules called from the CLI prefer global installation, CoffeeScript has set preferGlobal: true as of 1.8.0.

tkellen commented 9 years ago

Hey Charles! Unfortunately (or, fortunately, depending on your perspective), node does not natively allow what you are describing. Please see the "global module semantics" section of this blog post: http://bocoup.com/weblog/building-command-line-tools-in-node-with-liftoff/.

That said, you could make this work by overriding requireLocal in your own code to manually look in the global node_modules directory for the requested module. I won't support that directly here though.

doublerebel commented 9 years ago

Well, I understand npm has a strong opinion on global require, but nevertheless it's still a feature available in node. I have my NODE_PATH set to take advantage of the feature. I haven't encountered another package that overrides native require to only requireLocal -- is there another reason for that?

tkellen commented 9 years ago

Fair point. While I would never personally use it, I would be down with a PR adding a configuration option to specify which folders should be used when looking for auto-loaded dependencies.

tkellen commented 9 years ago

Closing for inactivity. I would still accept a PR for this, though.

Meroje commented 9 years ago

I had a look at this, would the solution I just commited here be okay for a PR or is there some issues with require.paths (per the comment "Probably don't use this") ?

heikki commented 9 years ago

FYI that fix breaks if process.env.NODE_PATH is undefined.

Nvm has stopped setting it: https://github.com/creationix/nvm/issues/586

Meroje commented 9 years ago

Ah right, I was testing with gulp which wraps this in a try/catch. A simple condition will do for this. Any insight on this comment on node-resolve : "opts.paths: probably don't use this" ?

heikki commented 9 years ago

I've seen that too but don't know any additional details.

Here's related PR that was rejected: https://github.com/substack/node-resolve/pull/47

Meroje commented 9 years ago

So, as I understand it, use of NODE_PATH is discouraged just like having a very long PATH and putting a binary in the last folder will be slow. I will submit the PR later once I have a test written.