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

Don't copy devDependencies #90

Closed
crysislinux opened this issue Feb 23, 2018 · 16 comments · Fixed by #159
Closed

Don't copy devDependencies #90

crysislinux opened this issue Feb 23, 2018 · 16 comments · Fixed by #159
Labels

Comments

@crysislinux
Copy link

there is an issue about this problem but it's closed.
#13

Some other ppl and I still have the problem in that issue. So just to reopen it here.

package.json

{
  "name": "aws-credentials-exp",
  "version": "1.0.0",
  "description": "",
  "main": "handler.js",
  "dependencies": {
    "aws-sdk": "^2.201.0"
  },
  "devDependencies": {
    "serverless-plugin-typescript": "^1.1.5"
  },
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}

serverless-plugin-typescript is not in the final release, but typescript and the dependencies of typescript do.

I tried not to include serverless-plugin-typescript plugin in serverless.yml (it's still installed and exist in package.json), the node_modules in zip file will only have aws-sdk and its dependencies.

@crysislinux
Copy link
Author

It seems that only the packages declared explicitly in devDependencies are excluded. But the dependencies of those packages are not.

Since typescript package is a really large one, as a workaround, I add typescript into devDependencies, then the zip size only added 0.1 M with a very simple handler.js

But that's not a good solution because once I added more dev packages, their dependencies will go out of control.

@cspotcode
Copy link

This breaks due to the symlinking, at least on Windows.

Serverless runs this command to discover dependencies.

npm ls --dev=true --parseable=true --long=false --silent

Try running that command in the project's root, then cd .build and do it again. All transitive dependencies are missing from the latter invocation. (You need to do this when the .build/node_modules symlink exists)

@crysislinux
Copy link
Author

Sorry I should include my env.

serverless: 1.26.0
os: macOs High Sierra
npm: 5.6.0

@ajmath
Copy link
Contributor

ajmath commented May 16, 2018

I'm also seeing this issue on on both my dev box (macOS High Sierra) and in the build container (node:carbon-alpine). Both are using:
npm: 5.6.0
sls: 1.27.2
serverless-plugin-typescript: 1.1.5
yarn: 1.6.0

I'm seeing the same symptoms where npm ls --dev=true --parseable=true --long=false --silent returns the full list when run in the service directory and returns only the top-level dev dependencies in .build

@mlev
Copy link

mlev commented Jun 25, 2018

I'm also seeing this on macOS High Sierra and npm 5.6.0. I've tried npm 6 and yarn and they also behave the same.

Are there any plans to review the PR? I'd be happy with the proposed fix to copy instead of symlink. I doubt it would cause any significant performance issues - and regardless would be better to have a fully working build.

@mlev
Copy link

mlev commented Jun 28, 2018

I can get dependencies excluding correctly by downgrading to npm 5.0.4. Anything higher and only top level is excluded.

npm install npm@5.0.4 -g

@ValiDraganescu
Copy link

I really need this issue fixed. I see that there are several pull requests to fix this. May we have them merged?

Thank you!

@mlev
Copy link

mlev commented Aug 16, 2018

FYI - I've switched to use serverless-webpack with ts-loader and works well

@Junkern
Copy link

Junkern commented Apr 30, 2019

This issue still persists.
I am using serverless in version 1.41.0, node 8.7.0 and serverless-plugin-typescript: ^1.1.7

Just curious: Is there a reason this does not get worked on?

@JackCuthbert
Copy link
Contributor

JackCuthbert commented May 1, 2019

Hi @Junkern, it's just me maintaining this in my spare time and I'm looking into another issue right now. If this is continuing to be a problem for many I'll prioritise this when I can. Am I correct in assuming that this issue does not block deployments/packaging?

@ajmath
Copy link
Contributor

ajmath commented May 1, 2019

@JackCuthbert That's for maintaining this. To answer your question about blocking deployments, depending on the size of the project, it can be a blocker. For our project, we ended up having to write some custom code to trim dev dependencies to allow the project bundle to stay within the 50mb lambda limit.

@Junkern
Copy link

Junkern commented May 2, 2019

Am I correct in assuming that this issue does not block deployments/packaging?

For me, it does not block. But as @ajmath explained it can be a blocker. Also having a larger bundle increases deploymen times, startup times and so on...

Would you prefer me doing a PR and fixing this? (e.g. https://github.com/prisma/serverless-plugin-typescript/pull/101/files)

@JackCuthbert
Copy link
Contributor

For our project, we ended up having to write some custom code to trim dev dependencies to allow the project bundle to stay within the 50mb lambda limit.

Big oof, can you share that source, @ajmath? 😛

@Junkern totally happy for you to submit (or build upon existing) PRs for this issue!

@ajmath
Copy link
Contributor

ajmath commented May 3, 2019

@JackCuthbert Here is what I did for the workaround https://gist.github.com/ajmath/db7a1210cf9e01403e3dfb6004c1b89a

@thomasruiz
Copy link

@ajmath that works perfectly, thanks!

JackCuthbert added a commit that referenced this issue Jul 12, 2019
Includes a refactor of the hooks property and class structure to
"modularise" everything for better readability/lifecycle configuration
as we're now adding flags to individual steps.

Closes #157, resolves #90 and #154
JackCuthbert added a commit that referenced this issue Jul 12, 2019
Includes a refactor of the hooks property and class structure to
"modularise" everything for better readability/lifecycle configuration
as we're now adding flags to individual steps.

Closes #157, resolves #90 and #154
JackCuthbert added a commit that referenced this issue Jul 13, 2019
Includes a refactor of the hooks property and class structure to
"modularise" everything for better readability/lifecycle configuration
as we're now adding flags to individual steps.

Closes #157, resolves #90 and #154
JackCuthbert added a commit that referenced this issue Jul 13, 2019
Includes a refactor of the hooks property and class structure to
"modularise" everything for better readability/lifecycle configuration
as we're now adding flags to individual steps.

Closes #157, resolves #90 and #154
JackCuthbert added a commit that referenced this issue Jul 13, 2019
Includes a refactor of the hooks property and class structure to
"modularise" everything for better readability/lifecycle configuration
as we're now adding flags to individual steps.

Closes #157 and #101, resolves #90 and #154
@divyenduz
Copy link
Contributor

🎉 This issue has been resolved in version 1.1.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants