search

RyanMcG / lein-npm

Manage Node dependencies for CLJS projects
295 stars 43 forks source link

Create npm-root directory if it doesn't exist #34

Closed radhikalism closed 9 years ago

radhikalism commented 9 years ago

As reported in #26 an exception is thrown when lein-npm is invoked while a root directory does not exist.

As a convenience, this fix gets lein-npm to optimistically create the root directory, which covers the annoying common case where a user forgets to manually create a directory beforehand.

It doesn't deal with other edge cases such as incorrect permissions, pre-existing paths with the wrong type, etc, which will all still throw ugly exceptions. It might be nice to work out a more general error handling/reporting mechanism separately.

RyanMcG commented 9 years ago

Thanks for the PR. I'm not sure when this error occurs though. I should have asked for clarification in #26. The default, using :root seems like it will always work. What usecase did you have that you received this error? I'm trying to decide if "implicitly create the directory silently" is better for the user than asking them if they want to create it or giving a more descriptive error message. I'm worried that mis-configuration will result in lein-npm strolling along pleasantly while they user continues to not see the generated package.json.

After thinking about this some more I think that the result of mis-configuration will be obvious enough that simply generating the missing directories is probably ideal 90% of the time. Still, I'd appreciate a response to my question above.

radhikalism commented 9 years ago

Steps to reproduce:

For me it's an intuitive workflow and not too magical. I'm unable to see any harm in silently creating the directory as it's non-destructive even if a side-effect. Any pre-existing dir entry would get reused; if it's the wrong type, there's a safe exception as usual. What other misconfiguration risks do you have in mind?

It feels analogous to the target/ directory which Maven creates if absent, likewise npm's node_modules/ itself. My sense is that npm-root is also understood as a mechanically-controlled directory and not primarily for manual management.

(There might be interesting situations when/if lein-npm starts integrating with persistent package.json files or other tools, but that's a different issue I think.)

I'd avoid prompting for input if possible, since it tends to complicate automation and scripting. A better error message would be okay though.

RyanMcG commented 9 years ago

Steps to reproduce:

I was not intending to get steps to reproduce. I understood that. I have never used the custom root feature though so I was wondering what you were using it for specifically. I thought your use-case might be indicative of how others are using it.

Anyway, the rest of your reasoning makes a lot of sense. I wasn't trying to argue against creating the directory, but I wasn't sure it was ideal.

radhikalism commented 9 years ago

Ah, I see. The main use case I've had for :root is for large, mature, legacy projects that I'm gradually migrating to use Leiningen and lein-npm for the first time. There might be a legacy node_modules/ at the top-level already, managed by another toolchain. It's not an ideal or stable arrangement, but for the duration of transitioning the project's architecture, it's useful to run parallel systems within the same project. Otherwise the pre-existing codebase might need to be ported in bigger increments, possibly even a big bang approach, which is often a barrier to adopting Leiningen (depending on the economics of the enterprise).

RyanMcG commented 9 years ago

@arbscht Thanks for the reply, it is informative.

Can you think of any features that lein-npm could have that would make that work easier?

radhikalism commented 9 years ago

@RyanMcG Can't think of anything, it works pretty well from my perspective already.

Maybe supporting existing/persistent package.json data would be nice to have; it might permit a direct transition to lein-npm for any given legacy project, rather than running a system in parallel as I've done. But I don't have any ideas for how to arrange this or if it would really be worth the complexity.

I'm also vaguely considering the value of a lein plugin that would manage a project-local node.js instance that is only exposed to (or preferred by) lein and any interested plugins. This might be out-of-scope for lein-npm… or not. The general idea is to decouple the node.js versions used by lein-npm and any other parallel toolchain. (It can be done with environment variable hacking already, but what if it was easier, even to the extent of automatically fetching/installing/upgrading node.js? See also: https://github.com/skwakman/nodejs-maven-plugin)