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

Supporting @npm modules as deps for compatibility with rules_nodejs? #322

Open
LStAmour opened this issue Dec 17, 2018 · 0 comments
Open

Comments

@LStAmour
Copy link

LStAmour commented Dec 17, 2018

Hi,

I know this has been asked for in 2016 and 2017, but as rules_nodejs was recently updated this year after much experimentation, and as rules_typescript was rewritten to support rules_nodejs, I'd like to request (or work on) basic support for rules_nodejs in rules_closure. I recognize that I'm new to rules_closure, and that work might be underway internally to support rules_nodejs, so I'm opening this issue to hold a conversation on official support for rules_nodejs, as the only alternative (to me) would be to create a very basic closure binary output rule in rules_nodejs and that would duplicate efforts here.

To start with, here's what was said in 2017 when support was first proposed:

We unfortunately provide very little currently in terms of Node integration. Integrating Node with Bazel is extraordinarily difficult, because there's so many tiny packages. pubref/rules_node tried to make it simple, but traded hermeticity, determinism, linear complexity, sandbox support, and sha256 checksums in the process. For these reasons, interop is most likely not a feature we'd consider.

Another reason why Node has been a low priority for Google, to cite another issue:

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 (!).

There's been recent work to improve rules_nodejs however. Here's a doc that covers some of it:

JavaScript/TypeScript Bazel rules refactor (proposal, Google Doc)

And here's a video from BazelCon that goes into a bit more practical detail (for end users) on what's been implemented:

BazelCon 2018 Day 1: Building Large Angular Apps With Bazel (YouTube)

The end result may not reach rules_closure's standards sited above, however as it's possible to have Bazel manage each of the npm modules under this new system, I believe at least some of the concerns from 2017 have now been addressed.

To that end, if looser filegroup support (as deps, data or srcs) is not possible for rules_closure (or some variant of existing rules), I'd like to still propose interop between rules_nodejs and rules_closure so that either you can simply specify a nodejs-based library and get all the deps inferred for free, or so that you can pass in nodejs-managed @npm repo deps and have rules_closure use them.

I'm open to other suggestions, however, as I'm just guessing based on how rules_typescript currently builds on top of rules_nodejs.

Why do we need any changes at all?

Well, I'd prefer using Closure Compiler's native support for commonjs modules (in loose mode if necessary) and nodejs package.json files, if I'm reading this source code correctly: Compiler.java#L1989

But rules_closure is too restrictive about both srcs and deps, not even allowing filegroups, and requiring that all srcs files be *.js (so no binary pre-built files and no .json files). That would be fine if it supported data or deps that are filegroups or folders, but it does not appear to support anything other than closure_js_library files and of that, only js files are allowed as srcs.

I've an issue over in the rules_typescript project where I've been discussing interop between rules_typescript and rules_closure, but I've hit a bit of a wall as rules_closure is the least flexible ruleset I've encountered in Bazel so far, and is even less flexible than Closure Compiler itself. So the only other option I can think of is to mimic rollup support in rules_nodejs and create a thin wrapper around closure compiler for rules_nodejs interop.

Related:
bazel-contrib/rules_nodejs#57 (comment)

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

No branches or pull requests

1 participant