Thoughts for yarn-based redesign

[alexeagle]

Opening this here as a better location to continue from bazelbuild/rules_closure#205 (comment)

yarn install is part of the repository rule where we install yarn. So after the WORKSPACE execution is finished, we have a node_modules directory up-to-date wrt. the yarn.lock file.
We only need to write a one-line BUILD file into node_modules with filegroup(glob(all), visibility=[public]).

I have a design doc out to @damienmg proposing this for the bazelbuild repo. Right now the doc is internal-only. I should have a prototype inside the typescript rules which I hope to open-source in a week or two.
Happy to discuss more!

[asf-stripe]

Hi, @alexeagle! I'm currently working on a similar thing on our internal bazel-ified and would love to trade notes (though I suspect you know way more about bazel than I do - I'm currently flailing just a little bit). At any rate - any insight into what you have would be very appreciated, and I'm happy to beta-test / ask questions / provide comments in return. (:

[alexeagle]

Hey, I finally got our implementation in good shape and open-sourced, as part of https://github.com/bazelbuild/rules_typescript (see eg. internal/node.bzl and internal/node_install.bzl)

Highlights:

1. there is only one download of each package from npm, and it lives in the users my_project/node_modules folder, not somewhere under $(bazel info output_base)/external. This avoids a long wait for duplicate package downloads, and ensures there's never a version skew between the libraries used for nodejs-based commands (eg. tslint on the command line) and the nodejs_binary rules running under bazel
2. To make the users my_project/node_modules folder visible to rules under some other repo, I create an external repo @npm and create a symlink to the users project at @npm//installed so any rule can depend on @npm//installed:node_modules. That requires a BUILD file in the project root that gives a filegroup which just globs the whole node_modules directory. Ideally we should move that to node_modules/BUILD and generate it during the repository_rule.
3. The nodejs_binary rule has the user's entire node_modules in its runfiles. Users dont have to declare individual dependencies on npm packages, and there is no need for a nodejs_module` rule. This is intended to match the existing semantics in the nodejs ecosystem, rather than be so opinionated about doing it the Bazel Way. There are tradeoffs here of course - eg. bazel doesn't like when some node_module contains a file with space or ampersand characters.
4. For hermeticity, the user is encouraged to run bazel build @yarn//:yarn rather than rely on whatever version of yarn they have installed on their machine. We should ideally run yarn check each time we execute nodejs_binary, depending on the yarn.lock file and the node_modules glob, so that we get approximately the same hermeticity guarantees as your implementation where Bazel acts as the package manager.

We're now discussing whether this should be extracted and placed in a first-party build_bazel_rules_node package and what that would mean for your rules. @pcj would love to discuss more with you. I'm on vacation the next two weeks but I'll try to swing by this issue.

[alexeagle]

Ping @pcj

[pcj]

Thanks @alexeagle and apologies for a very tardy reply as I have been doing some interesting traveling with the family over the summer and limited ability to deep dive on a number of areas, including this one.

I do like your design and fully support it being extracted into a standalone bazelbuild/rules_node project. I think the most important feature is probably keeping it as close to normal node_js project usage (for compatibility with other tools as your mention), and your approach via the @npm//installed:node_modules works well for that, even if not the most bazel'ish way.

These rules improved upon the state of nodejs support in the bazel ecosystem, but I make no claim they are the be-all-end-all. If you'd like help in setting up and.or maintaining that repo, I'd be game to assist.

[mattmoor]

FYI: https://github.com/dropbox/rules_node

[alexeagle]

Update on the design above: I learned of a special label syntax
@//:node_modules
which can be used from any repository to refer to the "main" repository.
That means I don't need the fake "@npm" repository that symlinks to the users project anymore, which cleans things up a bit.

[mattmoor]

FYI: https://github.com/redfin/npm-bazel (thanks, @dlorenc)

[samertm]

Nice! I'm interested in helping however I can :)

For context, I wrote the node rules that we use inside of Dropbox: https://github.com/dropbox/rules_node

At Dropbox, the first approach we tried was to have Bazel completely manage our npm dependencies. That broke down when we realized that modules with circular dependencies are everywhere in the npm ecosystem.

Then we went the complete opposite direction and said that each project should have a package.json and npm-shrinkwrap.json, and we'll use a Bazel rule to download everything from the shrinkwrap. That worked better and is fine for smaller projects, but it wasn't great at Dropbox because it meant that everyone was using a different version of their dependencies. When we wanted to upgrade something, like the version of typescript we used, we needed to make sure to upgrade it everywhere.

What we've landed on now is putting all of our direct dependencies in the folder //npm in our monorepo. We have a script that generates the npm_library rule (we have a internal tool that makes it easy to generate BUILD files) with an npm-shrinkwrap.json that only contains that package's modules. I think it strikes a good balance between the two extremes, and it's worked really well for us so far. Most of our node binaries and webpack builds don't even have a package.json anymore since using node with Bazel has been seamless.

[alexeagle]

Hey @samertm
Thanks for explaining your use case. At google we also pin versions for everyone, but we go (much) further and vendor everything into our monorepo, then can't use node module resolution at all (!).
That's just to explain why my proposal doesn't cover this use case: we're not going to use these rules internally.

But, since my rules take a label :package_json as an attribute, one could have a scheme for registering global package versions, and individual projects in the monorepo choose a subset of these, then generate the package.json file and pass to the nodejs rules. Then the package manager doesn't need to be aware of bazel.

Does dropbox use any TypeScript or Angular? Unless we converge on a single nodejs ruleset, I'm curious when the differences between our nodejs schemes will actually cause incompatibilities, and what remedies we have for that.

[alexeagle]

Update: I published https://github.com/bazelbuild/rules_nodejs

[samertm]

Nice!

Dropbox does use Typescript, but we have a custom set of typescript rules that we aren't planning on open sourcing because they integrate pretty tightly with how we do things at Dropbox.

[alexeagle]

I think we can close this issue for now. I'm always happy to discuss the different designs, and also we can open issues on bazelbuild/rules_nodejs for things specific to my re-design.

[TheLarkInn]

Hi @samertm!!! Sean, here from webpack!! I'm really interested to learn more about how you were leveraging the process flow with Bazel/Blaze and webpack. Any where we could sync up, (email, hangouts, etc.). I'm trying to get an official story between the tools and also make any changes needed in webpack (within reason) to acommidate a good parallelization story powered by Bazel/Blaze workers.

[samertm]

Hey!

It might make sense to open an issue on bazelbuild/rules_nodejs about this. Within Dropbox, we have a rule to specifically use Webpack (https://github.com/dropbox/rules_node#webpack_build), but bazelbuild/rules_nodejs is the open source set of nodejs rules that I think we should all start contributing to, and they don't currently have a webpack rule. So first, someone should implement it :)

I can offer some insight into making webpack compatible with Bazel once we've gotten that to work. @alexeagle can jump in if what I'm saying doesn't apply to bazelbuild/rules_nodejs (though I'm pretty sure it does), but when we run webpack in Bazel, it ends up being run in a tree of symlinks to other sources (called the "runfiles" for that target). Basically any time that webpack does a realpath on something (e.g. with loaders), that escapes the runfiles symlink tree and potentially breaks things. That was what caused extract-text-webpack-plugin to not work: webpack-contrib/extract-text-webpack-plugin#618.

So that's something to keep in mind, but we probably shouldn't jump the gun just yet. First someone needs to implement some node_build/webpack_build rules for bazelbuild/rules_nodejs.

Glad to hear that webpack is interested in supporting Bazel, though!

[alexeagle]

https://github.com/nrwl/nx/blob/master/packages/bazel/src/utils/webpack.bzl is already based on bazelbuild/rules_nodejs
but I think someone ought to own a proper design doc for how these things are meant to interact.

[mackross]

@mterring just published our modified rules https://github.com/happy-co/rules_node. Adding to the discussion as it supports sandboxing, yarn, bower and typescript.