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

Incompatible with npm 3 #28

Open
timdp opened this issue Jul 27, 2015 · 6 comments
Open

Incompatible with npm 3 #28

timdp opened this issue Jul 27, 2015 · 6 comments

Comments

@timdp
Copy link
Contributor

timdp commented Jul 27, 2015

With npm 3 on the way, I tried to get grunt-aws-lambda to support it. There are two main issues:

  1. The call to npm.commands.install in lambda_package.js should be updated to pass its second argument, the package to install, as an array rather than a string. Seems easy enough, and it worked in my tests.

  2. Unfortunately, because npm 3 flattens dependency structures rather than creating nested node_modules folders, you end up with something like

    temp
    |_  node_modules
        |_  myLambda
        |_  dependency1
        |_  dependency1.1
        |_  dependency1.2
        |_  dependency2
    

    IIRC (I didn't check), with npm 2, you would get the desired effect:

    temp
    |_  node_modules
        |_  myLambda
        |_  node_modules
            |_  dependency1
                |_  node_modules
                    |_  dependency1.1
                    |_  dependency1.2
            |_  dependency2
    

    I assume that what we're looking for with npm 3 is:

    temp
    |_  node_modules
        |_  myLambda
        |_  node_modules
            |_  dependency1
            |_  dependency1.1
            |_  dependency1.2
            |_  dependency2
    

    I'm not sure if that can be achieved by passing the appropriate arguments. Alternatively, I guess you could just move all the dependencies into that extra node_modules manually, but that seems messy.

    You can just depend on npm 2 for the time being, but I already wanted to document this issue for when npm 3 becomes mainstream.

@Tim-B
Copy link
Owner

Tim-B commented Aug 1, 2015

Thanks for your research into this.

I think at some point I'll just take npm's advice re. using npm programmatically:

If you want to use npm to reliably perform some task, the safest thing to do is to invoke the desired npm command with appropriate arguments.

That should resolve the first issue and also avoid having to install a separate copy of npm as a dependency.

I'll have to look into the second issue more deeply when I have time. For the moment making npm 2 as a dependency seems to be a pragmatic solution.

@timdp
Copy link
Contributor Author

timdp commented Aug 1, 2015

Then you won't know the version though (at least not without invoking npm --version first). It's weird that they don't recommend using the API, given that it's there. But yeah, the duplicate install is somewhat annoying.

@Tim-B
Copy link
Owner

Tim-B commented Aug 1, 2015

I figure that although you won't know the version, the CLI API should remain relatively constant - at least for most common commands. ie. npm install will probably always be npm install, just they might add a few extra flags over time. Any way, I'll have to see what commands I need to use to make it work first.

@timdp
Copy link
Contributor Author

timdp commented Aug 2, 2015

If you just copy the lambda's package.json to a temp folder, running npm install in that folder should give you what you need.

@timdp
Copy link
Contributor Author

timdp commented Nov 27, 2015

I had a stab at this on my npm3-native branch because I wanted to see if using npm 3 resulted in smaller packages (it didn't, but hey). The patch seems to work with simple Lambda tasks.

As discussed above, I'm running the globally available npm directly rather than installing it as a dependency. I currently have a check for npm >= 3 in there but it's probably unnecessary.

At first, I tried copying over package.json, but then, I figured it would actually be cleaner to copy over the whole package folder and install into the clone. That meant breaking relative paths in package.json, so there's some quick-and-dirty file: dependency handling in there. Not sure about that yet.

Anyway, it's just an attempt. I can turn it into a PR if you like.

@piercus
Copy link

piercus commented Mar 30, 2017

Hello @timdp,

I have made same kind of experiments today (see my npm3-experimental branch ).

It seems to work by using npm.load({ 'global-style': true },function(err,npm){}).

I agree with Tim-B that it would be better to use npm-cli but i wanted to have a working version with minimum impacts.

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

3 participants