Just a quick heads up. I investigated futher into this issue today and found out that we can use a solution which won't introduce an additional temp file.

It looks like using spawn could solve the buffer overflow problem since it returns a stream we can then react on.

I'll update this PR the upcoming days with a solution for this.

@ryanmurakami @cjelger @arabold @sbstjn @ajkerr I just pushed a fix for the issues with the current dev dependency exclusion.

The changes which were applied can be found in the PR description.

Would be nice if you could take this for a test drive and see if it resolves all the issues you brought up. This way we can get this PR in a mergeable state and publish it in a patch release the upcoming days.

Thanks in advance 👍

@pmuens The PR fixes the issues we were seeing with our project. Thanks!

@pmuens The PR fixes the issues we were seeing with our project. Thanks!

Great! 🎉 Thanks for confirming @ajkerr 👍

@pmuens Same result but worse. :-/ Since you changed to looking at dev dependencies, the output stops and one of the lines ends up excluding the entire node_modules folder. Smaller package size for sure. :-)

I'm going to mess around with using a temp file, see if I can make any progress on that.

@pmuens Didn't you have a temp file version before? Maybe reverting to that would fix @ryanmurakami 's issue.

@pmuens @HyperBrain So, it looks like using a tempfile fixes the issue.
I tried two methods. The first, hooking up a file write stream to the child process's stdout doesn't work because it still dies at 8kb.
The second is to just add > deps.txt to the end of the command. This works and sends everything to a file. Then I can just read from the file.

I can push that code, but there are a couple of concerns. The package command, with this approach, takes almost 2 minutes to complete, which is way too long. I switched the ls to look at production dependencies instead, and it cut the time in half. Oddly, the filesize was slightly larger.

Anyways, I pushed my changes to my repo in this commit: ryanmurakami@f137fbc

I can clean this up if you guys think it's a good approach.

🤔 thanks for taking another deep dive into this @ryanmurakami 👍

@HyperBrain yes, we had this file based approach (originally brought up by @arabold 🥇). It would be nice if we could replace the file creation and rely on streams (since IO could be unreliable and slow).

We switched from exec to spawn since spawn returns a stream you can tap into whereas exec has a buffer with a fixed size. Maybe I'm missing something here. (here's the post I'm referring to).

Isn't there any way to stream the data from stdout to avoid this buffer size limitation exec introduces?

I'll take a look into your commit @ryanmurakami tomorrow. Would be really nice if we could fix this issue w/o introducing an external file.

@pmuens I did a little more work and refactored the code so it's a little easier to riff on different approaches: https://github.com/ryanmurakami/serverless/tree/fix-dev-dependency-exclusion-bug

You can modify the getDependenciesStream and getDependenciesFile, then comment/uncomment lines 206 & 207 to switch each approach.

It appears this is some type of buffer paging that macOS is doing on the stdout stream. I don't see any differences when using spawn or exec in terms of the output changing. Both are cutting off at around 8kb whether you're piping stdout or just reading it after the fact.

I haven't been able to think of another approach except for a temp file. The only other thing is if there was some way we could emulate a file but actually write it directly in memory. I'm not even sure that's possible though.

Also, thanks so much for working hard on this @pmuens 🙏🙏🙏

The reason both exec and spawn fail to grab the whole output lies in npm itself. npm calls a process.exit() before it has flushed its stdout. That exit signal is catched by exec and spawn which stops reading the buffer immediately (which happens to be after 8k). I don't have the respective issue in npm at hand right now. It was linked in my original commit IIRC.

Long story short: From what I understand only a temp file will fully solve this problem for all platforms.

I believe that npm has purged most of their open GitHub bugs (for better or worse), and are only working on npm5+. Given that Node's current LTS is 6.x with npm 3.x, I don't think waiting for an npm fix is a great option.

Thanks for the in-depth discussion about this 👍

I found @arabold research comment and will link it here: #3737 (comment) (thanks for looking into this @arabold 👍).

Seems like there's no way around a file-based approach.

Next up I'll take a look @ryanmurakami implementation (thanks for the deep dive!) and update this PR so that we can hopefully fix this soon (and release a patch).

Again. Thanks for all the work and discussions on this here 💯!

Edit: This issue might be especially interesting: nodejs/node#6456

I just looked through @ryanmurakami branch (thanks for putting this together!) and pushed an update and switched back to the tmp file approach. I kept all the operations async.

Works on my machine. Would be nice if you could try it and see if it fixes the problem.

Another question: Is the exec command (>> foo.file) OS independent and works on all Windows machines? 🤔

/cc @ryanmurakami @arabold

Here's a test service which can be used to verify that the package exclusion works correctly.

It consists of 3 different directory levels where each level has a respective package.json file. Just run npm install in the different directories to install the dependencies and dev dependencies.

You can also e.g. only run npm install in the root level to force an exit code 1 (since npm ls in a directory with no installed modules will fail).

/cc @eahefnawy @ryanmurakami @HyperBrain @arabold

dev-dependency-exclusion.zip

@pmuens Thanks again for your continued work on this!
Testing it on my problem directory and this seems to have solved the issue.

I'm encountering something different, but I think it has to do with how I'm testing and not the actual code. I'm testing the PR by doing an npm link serverless and it appears that the zip process is zipping up the entire serverless directory with the package. Any idea why this is happening or a better way to test the new code? Regardless, I think you've got it with the most recent changes and it should be good to go.

Okay, I used npx and was able to validate this fix working good.

On a side note, it takes almost 3x longer to package my large project with this fix. I noticed the time is mostly spent after the dependencies are all resolved, most likely when archiver is zipping directories. Not a huge deal considering I think my project is an outlier, but just something I wanted to mention.

Okay, I used npx and was able to validate this fix working good.

Nice! Thanks for testing it out @ryanmurakami 👍 Great to hear that it finally works on your machine!

On a side note, it takes almost 3x longer to package my large project with this fix. I noticed the time is mostly spent after the dependencies are all resolved, most likely when archiver is zipping directories. Not a huge deal considering I think my project is an outlier, but just something I wanted to mention.

🤔 That's definitely smth. we should look into next up. Thanks for bringing this to our attention! 👍

I pushed some minor changes and fixed / add a couple of tests. This PR is GTM from my side.

/cc @eahefnawy

@pmuens following your instructions, I was able to validate that exclusion works as expected! 👍

G2M! 😊

I deployed https://github.com/react-boilerplate/react-boilerplate.git over lambda in AWS by cloudformation and I wanted to test this app now by lambda function , any ideas on how to test its localhost

@sasidharkanumuri thanks for commenting 👍

I think the best place to get help for such problems would be the Serverless Gitter of our Forum.

Let us know if you need any help with that 👍

Thanks @pmuens I just posted my question in the Serverless Gitter forum

Thanks @pmuens I just posted my question in the Serverless Gitter forum

Great! Thanks for that 👍 Hopefully your problem can be resolved soon!

I'm still getting the dev dependencies, or at least most of them using serverless v1.23.0 and serverless-plugin-typescript v1.1.3

Do I need a later version to get this branch?

Or it seems packages are being linked / reused or peer dependency more now than nesting under the same package in npm 5 and yarn, could this be part of the issue?

Sorry i'm kind of a noob.

I'm facing a few issues related to this. I've created a sample repo here:

https://github.com/tommedema/serverless-mono-example

Steps to reproduce

• yarn install
• cd packages/serverless-random
• yarn package

Note that the package now has a few dev dependencies which shouldn't be there:

• ts-node
• make-error
• arrify
• yn
• .bin/ts-node

I think this may be because serverless is hardcoding calls to npm, even though I don't use npm (I use yarn for its workspaces feature).

On a new project using the latest NPM vs. yarn and node 8 vs. typescript I didn't see this issue like I was with typescript/yarn/node 6. Not sure if any of those factors made the difference.

What I ended up doing was using gulp to copy my package.json and lock file over to the output directory then running yarn with the --prod flag so it wouldn't install any of the dev dependencies

@andythecoderman I upgraded yarn, npm etc. but still face the same issue.

Re your workaround: when I copy package.json to an output directory, yarn will fail to install packages from the workspace: error An unexpected error occurred: "https://registry.yarnpkg.com/@org%2frandom: Not found"