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

Bundling of node modules works oddly #117

Closed
JakubMatejka opened this Issue Mar 27, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@JakubMatejka
Copy link

JakubMatejka commented Mar 27, 2017

I use webpack-node-externals for bundling mode modules like this:

externals: [nodeExternals({
    modulesFromFile: true,
  })],

(whole config is here: https://github.com/keboola/developer-portal/blob/master/webpack.config.js)

And it worked quite well until recently when I decided to move the deployment to Docker and noticed that the code stopped working because some modules are missing. I have found out that the nodeExternals() method returns list of 47 modules however when I run serverless deploy, it bundles 137 modules. Although quite more then I want but the code still works. But the weird thing is that when I run the same deployment from a docker container (using this Dockerfile: https://github.com/keboola/developer-portal/blob/docker-travis/Dockerfile) it bundles only 18 modules, so the code fails on missing modules. Would you have any idea why is this going on? Thanks.

@JakubMatejka

This comment has been minimized.

Copy link
Author

JakubMatejka commented Mar 27, 2017

Oh interesting is that deployment in Docker worked earlier but the container was based on node:alpine image (https://github.com/keboola/developer-portal/blob/76292f987ec09ec165e28466c978c3c8bd10f700/Dockerfile). It does not work when we switched to image amazonlinux (https://github.com/keboola/developer-portal/blob/docker-travis/Dockerfile).

@JakubMatejka

This comment has been minimized.

Copy link
Author

JakubMatejka commented Mar 28, 2017

I have found a missing requirement (which was in devDependencies instead of dependencies). So the question is why did it work earlier, i.e. why it in some cases bundles more modules then it is supposed to.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Jun 27, 2017

We discovered an issue with the update to Serverless 1.16 today (in combination with the plugin), that the packaged dependencies were screwed up completely - lots of missing node modules that lead to module required exceptions when deployed to AWS. Switching back to 1.15.x bundles the correct dependencies again.

Maybe it is related to serverless/serverless#3737.

Can anyone confirm these problems with Serverless 1.16? If yes, I will open a separate bug there after a deeper analysis what exactly happens.

Just another issue with 1.16 module packagin appeared: serverless/serverless#3869

/cc @StevenACoffman @ricardolpd @franciscocpg

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Jul 31, 2017

I implemented the support of the service wide package: individually: true. It is available in the v3.0.0-individual-packaging branch. Can you test, if the branch works, if you set the flag at service level?
The implementation should also stabilize the module dependency installation and should work with SLS up to 1.18.
Please comment the results in #177 .

@keplersj

This comment has been minimized.

Copy link

keplersj commented Jul 31, 2017

@HyperBrain Would love to test it! Could you publish the branch with an npm dist-tag so I can use it without referencing a specific git commit?

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Aug 1, 2017

@keplersj Will do later today. Hmmm... the individual packaging is still in a PR, i.e. it is not yet merged into the v3.0.0 branch. I'd rather like to do a prerelease of the v3.0.0 branch as soon as the feature is merged.
So it would be great if an initial test is done by referencing the git version. I just need some feedback before I merge it.
Afterwards the prerelease will be publicly available as dist-tag

@keplersj

This comment has been minimized.

Copy link

keplersj commented Aug 2, 2017

@HyperBrain No worries! I just read up the npm documentation and didn't realize I could reference the branch over the commit hash. Will test it real soon.

@HyperBrain HyperBrain added this to the 3.0.0 milestone Aug 4, 2017

@HyperBrain HyperBrain added the bug label Aug 4, 2017

@HyperBrain HyperBrain self-assigned this Aug 4, 2017

This was referenced Aug 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment