Skip to content

.build/node_modules symlink vs. npm #45

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

Open
stesie opened this issue Oct 29, 2017 · 10 comments
Open

.build/node_modules symlink vs. npm #45

stesie opened this issue Oct 29, 2017 · 10 comments

Comments

@stesie
Copy link

stesie commented Oct 29, 2017

Hey,

I've just recently started trying out serverless framework and this plugin just today. So sorry in case things are a bit inaccurate :)

My problem is that, together with the typescript-plugin, the core's package plugin fails to properly exclude development dependencies from the build/package. This way even a simple "hello world" function has a footprint of roughly 15MB, where it should be just about 2 kB.

Problem seems to be, that package plugin just calls npm ls --dev=true ... and npm ls --prod=true ... and diffs the output of those two. With this plugin these shell calls are done from the .build folder, yet npm ls --dev=true ... fails to properly list all the dependencies as it stumbles over the symlinking.
Seems like it doesn't expect deduped/flattened dir structure then.

Replacing both symlinkSync calls by copySync immediately fixes the problem so :)
... yet I'm unsure if there's a better way to do that ...

regards

@schickling
Copy link
Contributor

schickling commented Oct 29, 2017

Could you elaborate a bit more on the implications of replacing symlinkSync with copySync? If there are no downsides, we'd be happy to merge a PR for this.

@stesie
Copy link
Author

stesie commented Oct 29, 2017

So my current playground setup looks like this ...

excerpt from package.json:

{
  ...
  "devDependencies": {
    "@types/aws-lambda": "0.0.18",
    "@types/node": "^8.0.47",
    "aws-sdk": "^2.141.0",
    "serverless": "^1.23.0",
    "serverless-plugin-typescript": "^1.1.3"
  },
  "dependencies": {}
}

(so only dev dependencies, serverless installed locally)

With the replacement mentioned above in place, .build folder has a node_modules which is a real directory + package.json a copy of the file from the main folder.

Within .build folder npm ls just works as it should:

[email protected] /home/stesie/Projekte/budgetier/.build
├── @types/[email protected]
├── @types/[email protected]
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ ├── [email protected]
│ │ ├── [email protected]
│ │ └── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├─┬ [email protected]
│ │ ├── [email protected]
│ │ └── [email protected] deduped
│ ├── [email protected]
│ ├─┬ [email protected]
│ │ ├── [email protected] deduped
│ │ └── [email protected] deduped
│ └─┬ [email protected]
│   └── [email protected] deduped
├─┬ [email protected]
│ ├─┬ @serverless/[email protected]
│ │ ├─┬ [email protected]
│ │ │ ├── [email protected] deduped
│ │ │ └── [email protected] deduped
│ │ ├── [email protected]
│ │ └─┬ [email protected]
│ │   ├── [email protected]
│ │   └── [email protected]
│ ├─┬ [email protected]
│ │ ├── @types/[email protected]
│ │ ├─┬ [email protected]
│ │ │ ├── [email protected] deduped
... and so on ...

yet without the change/fix in place both node_modules and package.json are symlinks to the parent's folders files. In this case npm ls fails like this:

[email protected] /home/stesie/Projekte/budgetier/.build
├── @serverless/[email protected] extraneous
├── @types/[email protected]
├── @types/[email protected] extraneous
├── @types/[email protected]
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├── [email protected] extraneous
├─┬ [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ ├── UNMET DEPENDENCY events@^1.1.1
│ ├── UNMET DEPENDENCY [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ ├── [email protected]
│ ├── UNMET DEPENDENCY [email protected]
│ └── UNMET DEPENDENCY [email protected]
├── [email protected] extraneous
... again many more ...

Notice how it reports unmet dependencies with the actual dependencies + extraneous directories in node_modules folder. This seems to be due to some special handling in npm for symlinks...

Regarding the downsides of this change I think it's just about copying around lots of data instead of creating a two symlinks. Even in my simple case it's copying around 100 MBs on each build, be it invoke local or deploy. Yet with SSDs and the OS's file system cache it shouldn't hurt much. At least on my laptop it's negligible

In a way this would also "solve" issue #23 :-)

@schickling
Copy link
Contributor

@pmuens could you share your view on this?

@pmuens
Copy link

pmuens commented Oct 30, 2017

Thanks for pinging @schickling 👍

Sounds like a feasible solution! Wondering if this is an edge case or smth. common 🤔.

@schickling
Copy link
Contributor

Maybe a good solution is to use copy when deploying and linking while building for development?

@brettneese
Copy link

Also having issues with this.

@dls314
Copy link

dls314 commented Jun 4, 2018

Is there any update on this, or related issues (like #23, #89, #90) around using serverless-plugin-typescript on windows?

For whatever reason, the workaround that previously worked (being an administrator) isn't working for me on Windows 10 build 17134.

@dls314 dls314 mentioned this issue Jun 4, 2018
@markwainwright
Copy link

I'm experiencing the exact same issue described by @stesie.

I came across a workaround in a different issue that fixes this for me (#89 (comment)) – set NODE_PRESERVE_SYMLINKS=1 in your env.

I'd still love to see this get fixed though!

@RichardBradley
Copy link

I have the same issue: the "excludeDevDependencies" feature in Serverless is not working, because npm ls cannot properly read the node_modules from the symlinked dir in the .build dir.

This is not a Windows issue; npm ls will give incorrect results on a unix box when run inside the .build dir as currently created by serverless-plugin-typescript.

This largely breaks excludeDevDependencies. I am getting most of my dev dependencies included in my final zip output. It seems that NODE_PRESERVE_SYMLINKS=1 does fix this though.

I'm not sure what the best fix is from serverless-plugin-typescript's point of view is. It seems to me that the way excludeDevDependencies is coded up in zipService.js in base serverless is a bit fragile. And fixing npm itself seems a bit out of scope for serverless-plugin-typescript. Some ideas:

  • Maybe serverless-plugin-typescript should just do a cp -r rather than a symlink for now? That would be a more expensive build than the current symlink approach
  • Maybe serverless-plugin-typescript should explicitly copy only prod dependencies? That would complicate things here and might lead to different problems later.
  • Maybe serverless-plugin-typescript can convince the dependency examination code in zipService.js to run in the "real" node_modules dir, then use the outputted exclusion paths against the .build node_modules dir later?

@RichardBradley
Copy link

I think the title of this issue should be changed to be more like "excludeDevDependencies broken by .build/node_modules symlink vs. npm". This issue is hard to find at the moment.

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

7 participants