Proposal: Rules refactor

[pauldraper]

There's too much for one issue, but wanted to present everything context, and get a take from rules_nodejs owners.

I believe there are a number of needed improvements, as well as lessons to be learned from the structure other rules with similar situations, e.g. rules_java, rules_kotlin, rules_jvm_external.

rules_javascript

• js_library - JS files and their dependencies (no js_import needed, since there's no compile step)
• nodejs_binary - Node.js executable
• rollup_bundle - Rollup bundle
• webpack_bundle - Webpack bundle
• amd_devserver - ibazel-aware web server using AMD modules
• karma_web_test - Karma test
• jest_node_test - Jest test
• jasmine_node_test - Jasmine Node.js test

Currently: js_library is missing. amd_devserver is called ts_devserver. jest_node_test is called jest_test.

rules_npm

• npm_install - Executable producing package-log.json/npm.bzl
• yarn_install - Executable producting yarn.lock/npm.bzl
• npm_export - npm package
• yarn_export - yarn package

Controversial take: npm and yarn are convenient tools to resolve dependencies, but it is annoying having to download 1GB of dependencies if I ever need anything from them. As with many other rule sets, Bazel should download and unpack these archives and use *_import to make them available.

load(":npm.bzl", "npm_packages")
npm_repositories(npm_packages())
# npm_repositories() is some default or user-defined function
# for adding js_library, ts_import, css_library, sass_library, less_library, etc...
# whatever rules users need to consume npm packages.

There's nothing "special" about npm dependencies, except the way they are fetched. As much as possible they should use the same machinery available to all other code regardless of delivery.

rules_typescript

• ts_compiler - TypeScript compiler (typescript), runtime library (tslib), and perhaps options
• ts_library - TS sources and their dependencies
• ts_import - TS declarations, JS files, and their dependencies

Currently: ts_compiler stuff is specified for every library rule.

The ts_compiler rule could also add plugins like tsickle, tslint, tsestse, etc. Though tsickle might make more sense as an aspect.

Controversial take: Using the tsconfig.json format for Bazel produces of a ton of exceptions and confusing cruft (files, paths, includes, excludes, module, declaration, composite, etc.). Instead, use Skylark as the source of truth, and generate the tsconfig files for IDEs. (This also permits a number of possible customizations...like pointing to declarations instead of sources in very large repos.) This Skylark -> IDE file approach is used elsewhere, e.g. bazelbuild/intellij.

rules_angular

• ng_compiler - Angular compiler (@angular/compiler, @angular/compiler-cli) and runtime library (tslib, @angular/core), and perhaps options.
• ng_library - Angular library

Currently: ng_compiler stuff is specified for every library rule.

[alexeagle]

Thanks Paul! Lots of useful stuff in here, I'll start with a few high-level observations but like you say it needs to be unpacked into a set of specific refactorings/migrations.

Is there a specific reason you think more rules_* is better than one? Like they would have separate governance? Or do you just use those headers as conceptual grouping?

I think your suggestions on the whole do make for a more "Bazel-idiomatic" rule set compared with those Java-ecosystem ones. In a lot of ways "Bazel-idiomatic" also means similar-to-Google-internal (google3). We have been taking the opposite bias in the last year or so: be more JS-ecosystem-idiomatic. The latter approach has some big advantages: more adoptable by a wide range of users, less disruptive to their mental model and therefore less aversion to change. Everything they do today can be modeled and most bugs are reproducible outside of Bazel. An important part for me is that there's a much wider audience of JS tooling ecosystem adopters looking for a better build tool than Bazel enthusiasts who want to add node/browser runtimes to the mix.

One important bit of feedback:

nothing "special" about npm dependencies, except the way they are fetched

I disagree about this one - in order to have existing code work, you need to lay out those dependencies into a node_modules tree that carefully follows existing semantics to allow multiple versions of a library. This is part of the reason we've never tried to fetch dependency tarballs and then deal with them ourselves.
Of course if you follow the Google-internal model you can just declare that your rules aren't meant to be compatible, and some code has to be changed or some third-party libraries may no longer be used.

There is also the impracticality of it. No maintainer here has more than a couple hours a week, and the existing rules have a bunch of adoption, so making any of these breaking changes will take a long time and we'd only have a budget to do a couple of them. Would be interesting to stack-rank and size them. Or maybe you think there's a community who would engage to build some of that out?

I could also imagine someone wanting to start over from scratch and go for the Bazel idiom. But that seems like it should be a Google project and Google has little appetite for owning anything outside of Bazel core and has in fact been giving away ruleset ownerships to the community (eg. rules_python)

Note that we do plan to introduce js_library soon which will be a rename of https://github.com/bazelbuild/rules_nodejs/blob/1.x/internal/npm_install/node_module_library.bzl - it's basically a filegroup of .js/.d.ts with providers that let it be "linked" for first-party dependencies (the module_name feature of ts_library)

[pauldraper]

Is there a specific reason you think more rules_* is better than one? Like they would have separate governance?

It's really more about scope. You can put them all in one repo...it's really just about having clear divisions. It's helpful to know "this is JavaScript," "this is TypeScript," "this is Angular," "this is SASS," "this is Kotlin." When my basic ts_library rule starts including code for some MVC framework...(a) it's confusing and (b) it's strong evidence of abstraction violations and non-extensibility.

This is very shaped to Google's concerns, e.g. rules_typescript and rules_nodejs which no non-Googler quite knows what goes where. Given it's contributors, that's only natural...but I'll also argue for reshaping/generalizing it.

Note that we do plan to introduce js_library soon which will be a rename of node_module_library

Great, good to know.

I disagree about this one - in order to have existing code work, you need to lay out those dependencies into a node_modules tree that carefully follows existing semantics to allow multiple versions of a library.

No argument from me on multiple version of a library.

I'll disagree about node_modules. Consider PnP which has quickly reached adoption or compatibility with most module-aware tools. That said, I'm not opposed to using node_modules linker...I just think that we can and should be using the same dependency stuff whether internal or external. (And yes I agree node_modules linkers can be complex...thus the preference to have another tool do that.)

There is also the impracticality of it. No maintainer here has more than a couple hours a week, and the existing rules have a bunch of adoption, so making any of these breaking changes will take a long time and we'd only have a budget to do a couple of them.

Agreed. I'm considering working on some of these these, whether part of this project or not. I first wanted to understand yes/no/neutral on even the desirability of these changes, realizing that they are breaking (and the in the case of rules_typescript, used by Google internally).

It's a question of (a) direction and (b) appetite for breaking changes.

I think your suggestions on the whole do make for a more "Bazel-idiomatic" rule set...We have been taking the opposite bias in the last year or so: be more JS-ecosystem-idiomatic. An important part for me is that there's a much wider audience of JS tooling ecosystem adopters looking for a better build tool than Bazel enthusiasts who want to add node/browser runtimes to the mix.

Interesting.

Frankly, npm and Webpack are pretty good for many projects. I think the recent reversal on the approach to Angular CLI/Bazel integration shows how difficult/compromising it can be get Bazel to be a drop-in replacement.

It's large and polyglot repos that don't have good solutions. I assume that's who uses it now. (For example, AFAIK there's no npm_export publisher...it would seem few are publishing to npm.)

[alexeagle]

pkg_npm is exactly the npm_export publisher. I would guess this is pretty commonly used (including private npm registry in BigCorp) but we don't have telemetry.

js_library feels like clearly the next step to me. We've been discussing design recently, it cleans up several things, and you're interested as well. Would you be willing to participate in the design there? I reached out on Bazel slack.

Note that the layering in ts_library was very consciously violated, not just by Angular but Polymer as well. https://www.npmjs.com/package/ts-lit-plugin works in the Language Service but TS team still hasn't extended this to tsc. As you know, building the whole compiler is a giant task when all you needed was a bit of extension to the emit phase in tsc. We hope that will eventually be upstreamed (Ron Buckton had a proposal for it) and then the layering is correct again because our fiction of use_angular_plugin becomes reality in tsc.

The whole rules_typescript thing bothers us very much, eventually we need to get attention from Google decision makers and decide between:

• rules_nodejs could drop ts_library, ts_devserver and friends and claim "Google should support these" - in practice we'd need a Googler reviewer to move these fully into rules_typescript. I'd love the reduction in our scope. But once there, you can bet they'll just be abandonware, and some of these things don't have a ready replacement (ts_library worker mode for example)
• We could fork the code, drop dependency on rules_typescript, tell Googlers they can stop syncing it to GitHub. Then we own the full story and can make whatever breaking changes regardless of how hard google3 is to change. Similar maintenance burden for us as what we have now. Google stays on their tech island and has even less chance of using Bazel and Blaze together for code they open-source
• We could get a Google team that cares more about owning this so that changes in rules_typescript land and google3 adopts them.

I'm working on rules_python now too, where there's a similar governance problem, so maybe we can lump these together when I get someone's attention.

[pauldraper]

pkg_npm is exactly the npm_export publisher.

Ah, great. The documentation misses a lot of stuff.

We should probably add Stardoc.

js_library feels like clearly the next step to me. I reached out on Bazel slack.

Thank you.

We hope that will eventually be upstreamed (Ron Buckton had a proposal for it) and then the layering is correct again because our fiction of use_angular_plugin becomes reality in tsc.

Good to hear.

Regardless of the success of that, however, I think it makes sense to have ts_compiler/ng_compiler rule as I suggested. It allows separate configuration/fiddling with the compiler and associated runtime libs.

ts_compiler(
  name = "tsc",
  compiler = "@npm//typescript",
  runtime_deps = ["@npm//tslib"],
  options = { "strict": True } # defaults
)

ts_library(
  name = "example_ts"
  compiler = ":tsc", # default to some bind()
  srcs = glob(["**/*.ts"])
)

ng_compiler(
  name = "ngc",
  compiler = "@npm//angular-compiler-cli",
  runtime_deps = ["@npm//angular-core", "@npm//tslib"],
  options = { "strict": True } # defaults
)

ng_library(
  name = "example_ng"
  compiler = ":ngc", # default to some bind()
  srcs = glob(["**/*.ts"])
  assets = glob(["**/*.html", "**/*.css"])
)

Also, there ought to be a separate ng_library because it accepts CSS and HTML files as inputs which is pretty different than tsc normally.

The whole rules_typescript thing bothers us very much, eventually we need to get attention from Google decision makers and decide between

I'm not in a good position to judge, but offhand I'd say that whatever is done for Bazel core or any other open-source Google project. (I assume that the last option?) Certainly the "can't make changes because there is a hidden of sets that need to pass" stops contribution pretty hard.

I'm working on rules_python now too, where there's a similar governance problem

Yeah, exactly. I know that Google struggles with the open-source/monorepo integration question generally, Bazel or otherwise.

As far as repo structure I would prefer:

• npm_nodejs (rules_javascript if possible)
• rules_typescript
• rules_angular (or just angular repo itself)

Just like there is separate rules_closure and rules_sass repos.

[mattem]

Ah, great. The documentation misses a lot of stuff.
We should probably add Stardoc.

This is the docs for pkg_npm:
https://bazelbuild.github.io/rules_nodejs/Built-ins.html#pkg_npm

There was some work I did a few days back where the generated docs were missing the docs from aspects and providers, this is addressed at HEAD. Once 2.0.0 becomes final I think the docs should be updated.

I did think about reworking the layout of some of the docs making it easier to find sections etc, perhaps something to discuss further.

[alexeagle]

Hey @pauldraper could we figure out the remaining actions from this, file independent issues for discussion, and close?

I created

• Separate compiler rules from library rules #2119
• [governance] resolve ownership of rules_typescript #2120

also js_library is green and LGTM and probably included in release this week #2109

To address a couple of the remaining points:

Just like there is separate rules_closure and rules_sass repos

I think these were not well thought-out. The introduction of https://github.com/bazelbuild/rules_postcss makes it more clear. All other ones are rules_[language] - but closure is not a language (at least so the authors claim) and postcss isn't either. If they want to make a ruleset per tool on npm, good luck to them :) I think it's a sensible scope for rules_nodejs to include minimal rules/macros that call out to npm tool CLIs so users don't need to genrule everything.

Also, there ought to be a separate ng_library because it accepts CSS and HTML files as inputs which is pretty different than tsc normally

tsc does accept other files. It used to only read .ts/.tsx but they added .json so you can typecheck import thing.json - I suspect they could allow plugins to read css or svg imports as well. Again our theory here is that ts_library anticipates future TS ecosystem, which in fact it has always done (we added support for path mapping and other things before TS got to it)

I'll close this unless you want to keep more discussion on this thread? Or open issues for other bits that aren't covered above?

[alexeagle]

Oh, one more thing you brought up is to just install packages in the deps of requested targets, #2121

[kylecordes]

(Arriving late to this party…)

The proposed rename of ts_devserver to amd_devserver would be helpful to explaining its purpose and value to folks outside to the Bazel ecosystem. ts_devserver is a clever idea, and there is renewed interest in the JS world for faster dev (and production) bundling. (i.e. esbuild and multiple tools growing atop it).

[alexeagle]

@pauldraper didn't hear back from you so I think we're good with the issues I peeled out?