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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support optional use of Yarn for packaging (instead of NPM) #286

Closed
RishitKedia opened this Issue Nov 22, 2017 · 22 comments

Comments

Projects
None yet
7 participants
@RishitKedia
Copy link

RishitKedia commented Nov 22, 2017

Hey! 馃憢

As discussed here with @HyperBrain, it'd be great if we could optionally use Yarn for packaging files, instead of NPM. 馃檪

In particular, since serverless-webpack completely relies on NPM and uses it for packaging files, I'm having issues using serverless-webpack with Yarn Workspaces.

Currently, the bundled file is very huge (4 MB when used with Yarn Workspaces) compared to the same project when not used with Yarn Workspaces (25 kB). serverless-webpack bundles all the dependencies in one file even when using webpack-node-externals and webpackIncludeModules.

Thanks! 馃槂

@Gerst20051

This comment has been minimized.

Copy link

Gerst20051 commented Feb 23, 2018

What's the status on this? Why doesn't this work while using yarn? @HyperBrain

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Feb 23, 2018

Because the plugin internally uses npm - and these are orthogonal in some regions (esp. workspaces and links - and finally the lock files).

Regarding the timeline: I'll try my best to phase this in asap.

In general, it would work like this:
You will have a new configuration variable custom: webpack: packager which can be npm (default) | yarn. Depending on this setting all calls currently done by using npm would be done by calling yarn with the similar commands`.

But first, from the architecture of the project, the packager itself (npm) should be abstracted into separate classes, so that yarn just can be added as additional packager. This approach will keep the way open for other packagers.

@Gerst20051

This comment has been minimized.

Copy link

Gerst20051 commented Feb 26, 2018

Thanks! Would webpack-node-externals also need to be updated to support yarn or is this the only package that needs to be updated?

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Feb 26, 2018

No, only serverless-webpack. As far as I know node-externals reports external modules independently from the actually used packager.

@o-alexandrov

This comment has been minimized.

Copy link

o-alexandrov commented Mar 4, 2018

@HyperBrain Thank you very much for the package.
Please consider also adding support for --flat:
$ yarn install --flat
Or, in other words, support for resolutions in package.json.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 5, 2018

@OlzhasAlexandrov Thanks for the hint :) I will consider it. As soon as I have something available in a PR, I'll write here, so that people can start testing and verifying.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 8, 2018

Update: Seems the first version of the Yarn packager works:

Serverless: Package lock found - Using locked versions
Serverless: Packing external modules: source-map-support@^0.5.3, flow-runtime@^0.16.0, bluebird@^3.5.1, lodash@^4.17.5, ...

This output is Yarn, not npm!

Serverless: Using configuration:
{
  "webpackConfig": "./webpack.config.js",
  "includeModules": true,
  "packager": "yarn",
  "packagerOptions": {},
  "packExternalModulesMaxBuffer": 204800
}

I will try to prepare a PR tomorrow, so that we have something that we can base further ideas (especially options and their defaults) onto. Stay tuned.

@HyperBrain HyperBrain added this to the 5.1.0 milestone Mar 8, 2018

@HyperBrain HyperBrain self-assigned this Mar 8, 2018

@HyperBrain HyperBrain referenced this issue Mar 8, 2018

Merged

Yarn support #340

7 of 7 tasks complete
@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 8, 2018

At everyone here: The initial PR is available and ready for testing. I'll continue with the unit tests and proper documentation there. Please use "serverless-webpack": "github:serverless-heaven/serverless-webpack#yarn-support".

BUT: Bear in mind, that the PR is based on the just released version 5 of the plugin and you need to switch to the new configuration layout in serverless.yml to make it work with yarn. (low risk and easy)

[UPDATE: use the --verbose switch with the serverless command line to check what it is actually doing and if it is doing things right]

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 11, 2018

I will merge the PR in the next few days. As mentioned in the PR, flat support is dropped for now. The feature in general should be functional.

If we encounter any issues, we should create separate issues for them and fix them or add missing stuff separately.

@bfsmith

This comment has been minimized.

Copy link

bfsmith commented Mar 11, 2018

Does this have any impact for those of us not using webpack? The "Excluding development dependencies..." step takes awhile for me and I figure yarn would be faster than npm.

Edit: Nevermind, just saw the repository is specifically webpack.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 12, 2018

@bfsmith serverless-webpack does the packaging on its own and bypasses Serverless' packaging, so this is specific to this plugin. ... and you're right, using Yarn for packaging is a lot faster than NPM on most systems. If you use Node/JS as language, it would be worth a try to use webpack just to get an improved packaging behavior for the projects 馃槃

@dashmug

This comment has been minimized.

Copy link

dashmug commented Mar 13, 2018

Any idea why I'm getting Could not find packager 'yarn' when doing an sls deploy -v?

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 14, 2018

@dashmug Then it does not find the yarn executable during packging (I assume you enabled it with `webpack: pacakger: yarn). Is it available in the serverless' command environment (path)?

The plugin spawns yarn install instead of npm install if configured for yarn, so the executable has to be found by the OS.

@dashmug

This comment has been minimized.

Copy link

dashmug commented Mar 14, 2018

Yes, yarn is available.

I think it might be just specific to my setup. I believe one of the following (or both) reasons may have caused it.

  1. I'm using nodenv to pin my node version (v6.10) for this project. Global npm modules are installed differently when inside a nodenv environment.
  2. yarn is installed using Homebrew so it uses my system's node version (v9.8.0). It was installed using brew install yarn not npm install -g yarn.
@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 14, 2018

(2) Might be the problem. As far as I remember the installation of global modules (like npm or yarn) is put into the specific node installation and tightly bound to the node version. I experienced that with nvm where every installed node version has its own globals.

So you should install yarn into the Node 6.10 context too, and it should work.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented Mar 14, 2018

Released with 5.1.0

@Gerst20051

This comment has been minimized.

Copy link

Gerst20051 commented Mar 14, 2018

Nice work @HyperBrain I'm impressed!

@dashmug

This comment has been minimized.

Copy link

dashmug commented Mar 14, 2018

@HyperBrain It turns out I got that error on v5.0.0 and I thought this PR was included in that release.

Updated to v5.1.0 now and it worked flawlessly despite nodenv and Homebrew yarn.

Good job!

@dekryptic

This comment has been minimized.

Copy link

dekryptic commented May 6, 2018

Is this meant to work with yarn workspaces? I'm getting error Your lockfile needs to be updated, but yarn was run with --frozen-lockfile., possibly because the yarn lockfile and the base package.json are different?

Also, for some reason I only have this issue when deploying to API Gateway and not when using serverless-offline.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented May 6, 2018

@dekryptic I did not try it with yarn workspaces, but the lockfile error normally means, that Yarn would change the lockfile during a Yarn install, i.e. the lockfile does not represent the contents of the package json. Do you have anything in your setup that changes only the package.json but does not update the lockfile? Can you try the very same with the latest serverless-webpack version (5.1.5)? I improved the error handling there and the plugin now checks that needed runtime dependencies are present in the dependencies section of the project.

I only have this issue when deploying to API Gateway and not when using serverless-offline.

That is expected. serverless-offline uses the locally available node_modules, but a deploy makes sure that only dependencies are packaged that are needed. You can do the packaging without an actual deployment for testing: serverless package.

@dekryptic

This comment has been minimized.

Copy link

dekryptic commented May 6, 2018

@HyperBrain Thank you for the quick response. I've upgraded to 5.1.5, but I think the problem is related to the nature of yarn workspaces. The basic premise is that you can have multiple package.json's in your subdirectories that specify your project's dependencies, so almost by definition the lockfile would have packages that are not present in the base package.json file. Well, at least this is what I understand.

@HyperBrain

This comment has been minimized.

Copy link
Member

HyperBrain commented May 6, 2018

@dekryptic Yeah. I think you're right. Maybe the best way would be that you create a separate feature request for support of Yarn workspaces. We could then do explicit experiments for that and document the behavior of Yarn, its lockfiles and the workspaces there to be able to find a proper solution.

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