Add npm management support

[javierbertoli]

Should fix #36

[noelmcloughlin]

Hi @javierbertoli This is impressive work. This formula has come a long way from your "First Commit".

Thanks for the PR. I really think npm support is useful and the review is straightforward.

• SLS follows same proven pattern as pips, pkgs, snaps, etc.
• The README update is very clean.
• Three travis CI jobs completed successfully - all three using npms state.

The npms states did not run on Ubuntu. Does pillar.example need update too?

[noelmcloughlin]

LGTM.

[javierbertoli]

The npms states did not run on Ubuntu. Does pillar.example need update too?

As I added here, because I'm depending on an external formula, which has a different configuration/behaviour for different os (`nodejs.ppa, binary, source, etc), I just decided to go and 'test it in a single os'.

If it works, I consider THIS formula works, as I can assume npm's behaviour will be consistent across OSes

The other choice would be to use OS-provided npm, but again, that's not consistent at all, which was the reason that pushed me to add an external formula dependency.

[myii]

@javierbertoli I'm a bit caught up at the moment but I'll try to have a look at this fairly soon, even if only an eyeball review.

[myii]

https://docs.saltstack.com/en/latest/ref/states/all/salt.states.npm.html#salt.states.npm.installed also has a pkgs option. Can we avoid this loop and just use that instead? Otherwise lots of extra, unnecessary noise if using this for numerous packages.

[myii]

From the same reference as above, we've got dir, which controls whether the installation is global or not. Can this be incorporated here? If so, will probably require user as well. That may be beyond the scope of this PR. However, it would be far easier to structure the pillar and states at this stage, than making (breaking) changes later on to accommodate for non-global installations.

[myii]

It would be great to have an example here of how to supply the package version as well. Again, using the example from the upstream docs, something like coffee-script@1.0.1.

[myii]

formual => formula.

[myii]

formual => formula.

[myii]

@javierbertoli @noelmcloughlin So this was a fairly breezy eyeball review. I love the idea of this introducing this feature. I think it is better to arrange the structure now, rather than try to accommodate for non-global installations later. But you may consider that beyond the scope of this PR or even the formula itself.

[myii]

@javierbertoli Actually, now I'm going to throw a bit of a curveball... when I install npm packages, I always use nvm, rather than clutter the system. There is this: https://github.com/adcade/nvm-formula. Could we do something similar here or is that really beyond the scope of things?

[myii]

@javierbertoli Looking at the license and the README of https://github.com/adcade/nvm-formula, it looks like it was a formula that was intended to join our collection here at SaltStack Formulas, but never made it for some reason... but that suggests it would be a very useful basis for introducing nvm here instead, with attribution, of course.

[myii]

@javierbertoli This is looking really good now, thanks for the effort you have put in. I was concerned that I may have caused you too much trouble.

Let me try running this at my end and see how things fare. I'll get back to you fairly soon.

[myii]

@javierbertoli This appears to be working great. I'm running into one problem, though -- it appears to be an upstream bug in salt.modules.npm.install. I can't install any package that begins with an at sign (@), such as @davidodio/hello. I've tried all sorts of invocations, even with straight with the npm.install module but I keep getting errors. Do you know of the special way to pass this through to get these packages installed? If so, it would be very helpful to put one example in pillar.example, to save others the trouble of hunting the right way to do this.

---

Edit: I tried on a minion with a newer version of npm and the following works fine:

$ salt -Cv 'ABC' npm.install @davidodio/hello@2.3.0

But still having trouble via. the pillar.

[myii]

@javierbertoli OK, it's not just me! I managed to break the build, simply by adding a package starting with @:

       [CRITICAL] Rendering SLS 'packages' failed, render error:
       found character that cannot start any token
       Traceback (most recent call last):
         File "/usr/lib/python2.7/dist-packages/salt/pillar/__init__.py", line 736, in render_pstate
           **defaults)
         File "/usr/lib/python2.7/dist-packages/salt/template.py", line 101, in compile_template
           ret = render(input_data, saltenv, sls, **render_kwargs)
         File "/usr/lib/python2.7/dist-packages/salt/renderers/yaml.py", line 57, in render
           raise SaltRenderError(err_type, line_num, exc.problem_mark.buffer)
       SaltRenderError: found character that cannot start any token
       [CRITICAL] Pillar render error: Rendering SLS 'packages' failed. Please see master log for details.

See:

• https://travis-ci.org/myii/packages-formula/jobs/504557818 -- this was the first one, only after changing pillar.example.
• https://travis-ci.org/myii/packages-formula/builds/504561178 -- this is all of them failing, after changing all of the testing pillar.example files as well.

---

Upstream code where the error is occurring:

• salt/renderers/yaml.py

[noelmcloughlin]

@myii I can fork nvm-formula into saltstack-formulas organisation. Regarding this formula we could add nvm support in future PR.

[myii]

@javierbertoli Excellent, thanks for going the extra mile. As a note, I had some strange issue where enclosing in quotation marks wasn't working -- however, the tests have confirmed it and I was able to restart things in my environment to confirm that all working OK. Thanks to @noelmcloughlin for the review and the offer of reviving the nvm-formula.

[noelmcloughlin]

Thanks @javierbertoli for adding this feature. Nice.

@myii Here is the formula here now: https://github.com/saltstack-formulas/nvm-formula

[myii]

@noelmcloughlin Thanks for rolling with that. I hope it gets some traction because it is really useful when working with npm.

[noelmcloughlin]

I'm working with node at the moment so will make try it out. thanks guys.

[javierbertoli]

@noelmcloughlin I understand you install npm packages with nvm pretty much the same way as you do with npm, right? Ie, nvm install some_package?

If so, we can add an example in the README/pillar to show how to add a dep to the nvm formula (as done with the node-formula) and we can add a cmd.run resource in npms.sls to loop and install using nvm instead of npm.*? wdyt? (I don't using neither of them atm, so guessing here, 😄 )

[noelmcloughlin]

@javierbertoli let's raise an issue for this because that sounds a good idea. I have not use nvm but I really like node so I probably will, and could test the idea works.

[noelmcloughlin]

New feature suggestion: We could also add yarn state. It works same as npm but syntax differs... Instead of npm install it's yarn add. For every npm action there seems to be a equivalent yarn command.