babel_library for babel support and compatibility with ts_devserver

[Globegitter]

This is a first proof of concept that picks up on the ideas talked about in #149, #137 and #200 to have full js (using es6 modules) support with ts_devserver as well with the possibility of some incremental transpilation before passing to a rollup_bundle or equivalent.

I ended up using babel for the js_library rules as it seems to be the easiest way to downlevel es6 modules to amd, this also opens up the nice possibility to reuse this for further transpilation in the future. This does not support node_module dependencies yet and probably has some other ways how it can be improved (e.g. there is the issue with #211) but I thought I would push this out before I go on holiday to have something to get the conversation started.

[alexeagle]

/cc @mprobst who is working on a similar transpilation for es6 sources and ts_devserver in Google-internal. We use the TypeScript compiler to do the downleveling in this case with --allowJs.

[alexeagle]

Hey @Globegitter would you like some help with this? I think Babel rules are an important missing area, and would help to get a chunk of the community interested in Bazel (I talked to James Kyle a bit this week)

[Globegitter]

@alexeagle Yeah would be very open to help. I am back from holiday on Friday. Then I can look more at #211. A specific open question I had was how to handle node modules. To me it seems the more bazel way would be to specify each direct dependency directly on the js_library but it seems the direction for the js rules seems to just specify the whole node modules. And I was wondering if it even makes sense to specify that for each js_library target, or if that is just expected to be handled fully by the top level rules, such as ts_devserver and rollup_bundle?

[alexeagle]

Okay let's try to schedule some time on Friday to work together on it?

…

On Mon, Jun 4, 2018 at 2:30 AM Markus Padourek ***@***.***> wrote: @alexeagle <https://github.com/alexeagle> Yeah would be very open to help. I am back from holiday on Friday. Then I can look more at #211 <#211>. A specific open question I had was how to handle node modules. To me it seems the more bazel way would be to specify each direct dependency directly on the js_library but it seems the direction for the js rules seems to just specify the whole node modules. And I was wondering if it even makes sense to specify that for each js_library target, or if that is just expected to be handled fully by the top level rules, such as ts_devserver and rollup_bundle? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5I6jSHMh332ghkmIhecwXanHZjnx0ks5t5P4jgaJpZM4UQa0d> .

[Globegitter]

I just sent you an email.

[nimerritt]

@Globegitter @alexeagle I'm looking forward to this! We want to incrementally convert our legacy Babel built projects to TypeScript with Bazel but without a js_library rule it's pretty awkward!

[alexeagle]

@Globegitter how is this going? LMK when you're ready for another review pass.

[Globegitter]

@alexeagle yeah sorry for the long delay on this, it has not been on my immediate priority list for a while as we found other solutions but it is starting to become more relevant again. I am very close for it to be ready for another review, next week I should have time for the last touches. The benefit of it taking that time is I have gotten a much better sense of how things work with bazel internally :)

[an-kumar]

I'm up for it. Babel support is a huge part of the javascript ecosystem and if we want to get Bazel actual adoption in js we really need to do it right. Happy to help if I can.

[Globegitter]

@alexeagle Yep that sounds good, I could do both days but Friday would work a bit better. How do we best schedule this?

[pward123]

fyi, I was getting the following error while trying to create a babel_library.

INFO: From Compiling Javascript (Babel) //my-es6-lib:my-es6-lib:
{ Error: EEXIST: file already exists, mkdir 'bazel-out/k8-fastbuild/bin/my-es6-lib/src'
  errno: -17,
  code: 'EEXIST',
  syscall: 'mkdir',
  path: 'bazel-out/k8-fastbuild/bin/my-es6-lib/src' }

I resolved it by lifting the mkdirp code from npm_package/packager.js

[pward123]

Should the output folder retain transpiled files that I've removed from the source package?

[Globegitter]

@pward123 which version of node are you using? You need to be using at least 10.12, though I would recommend 10.13 as it is the latest LTS.

[Globegitter]

And not sure I understand your second question - you mean if the transpiled files will stay in bazel-bin. As far as I am aware yes they would stay in bazel-bin as it does not do any kind of automatic cleanup etc. If you want to get rid of it (and all of your local cache in fact) then I think you would have to run bazel clean

[pward123]

Ah, I've been sticking with 8.11 since I have a few legacy apps that can't move to 10 yet.

[Globegitter]

If necessary I can also make the code backwards-compatible, it just seemed too nice to now have it available as native node api.

[Globegitter]

But how is it going, are you getting these rules to work for you?

[pward123]

I'm very green to Bazel so it wasn't totally pain free. However, I was able to get babel_library working on a very simple package. Putting together a solution for transpiling without all the rollup overhead was on my short list. I've also learned a fair amount about Bazel by looking through what you've done. Thanks!

On the clean issue. Let's say I run ibazel and remove a file from my source. The babel_library would still retain the transpiled version of the deleted file in the bazel-out folder. I'll probably just end up copying internal/babel_library into my monorepo and modifying it for 8.x and also globby the js files from the out-dir then delete the ones that weren't transpiled this pass.

[Globegitter]

Ahh, you are talking particularly about ibazel, I am not sure, it might just do the right thing if you are using glob. Definitely if you are using a nodejs_binary that depends on the babel_library and run that binary with ibazel. Then I am pretty sure that file would be removed for the binary, as it would just recreate the runfiles directory.

But yeah copying this into you repo manually sounds like a good solution for now. Happy to hear any further feedback in how this can be improved or if it is working well for you.

[pward123]

fyi, I've finished my first pass at moving the babel_library code into my own repo. One thing you might be interested in is the minor cleanup and removal of sync operations in babel.js

[alexeagle]

Sorry I missed the thread about scheduling a VC.
Could we try again for end of next week? Email me alexeagle@google.com and I'll include @mgechev also

[wtgtybhertgeghgtwtg]

I think it should be "handles".

[vmax]

@Globegitter thank you for the PR! there are some things I'd kindly ask you to address:

1. babel_library now cannot be used as deps in npm_package due to this error:

ERROR: /Users/vmax/work/GraknLabs/grakn/client-nodejs/BUILD:52:1: in npm_package rule //client-nodejs:client-nodejs: 
Traceback (most recent call last):
	File "/Users/vmax/work/GraknLabs/grakn/client-nodejs/BUILD", line 52
		npm_package(name = 'client-nodejs')
	File "/private/var/tmp/_bazel_vmax/a61cd25b9823cba82c925ec919cf6ad0/external/build_bazel_rules_nodejs/internal/npm_package/npm_package.bzl", line 73, in _npm_package
		transitive.append(d.typescript.transitive_declarat...)
	File "/private/var/tmp/_bazel_vmax/a61cd25b9823cba82c925ec919cf6ad0/external/build_bazel_rules_nodejs/internal/npm_package/npm_package.bzl", line 73, in transitive.append
		d.typescript.transitive_declarations
'struct' object has no attribute 'transitive_declarations'
Available attributes: es5_sources, es6_sources, transitive_es5_sources, transitive_es6_sources
ERROR: Analysis of target '//client-nodejs:client-nodejs' failed; build aborted: Analysis of target '//client-nodejs:client-nodejs' failed; build aborted
INFO: Elapsed time: 0,448s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (46 packages loaded)

Possible fix I've figured out for it is to include transitive_declarations = js_providers.transitive_es5_sources in struct returned from rule implementation. Is it a correct way and if it is, could you add the support to the upstream ecosia/rules_nodejs@js-library-poc?

2. Pretty common use case for babel is using babel-preset-minify for minification. Could you include "babel-preset-minify": "^0.5.0" requirement in internal/babel_library/package.json, and if no, how could I add it?

3. Although babel's configuration and rule's attr to pass it is called .babelrc, I find it confusing that with babel starting with version 7 JSON configs don't seem to be supported and instead JS files are expected (you are using one as well named babel.rc.js). Do you know what's the matter here? Is it the limitation of babel@7 (and should I address babel team instead) or it's the limitation of currently-implemented rule?

Again, thank you so much for submitting this PR 🙂

[Globegitter]

@vmax the npm_package error is a limitation indeed will look at fixing that. The eventual goal of this would be that you can install it as a package @bazel/babel and then you provide the installed babel binary to babel_library (if you do not use the default @npm name) and you should also be able to add any further babel dependencies as needed.

The json .babelrc should work in theory but the problem is that babel uses its own method to resolve its plugins so it can not find them when you just pass them in as a string. That is why we use the js file and require them all at the top.

[mattjs]

Hey guys this looks exciting! Any plans to move forward with it? It would be a great tool to have before piping stuff to rollup_bundle.

[alexeagle]

I'll pick this up now - I agree that babel is a good step before rollup (maybe we'd also want to use it after rollup to downlevel for older browsers)

[alexeagle]

Hey @Globegitter do you have some patience to work with me to get this going again?

I think we ought to start over and pull in some of your ideas and tests, because things have changed a lot since you worked on this:

• don't need to use custom module resolver
• things in their own packages/ directory
• more of a "have the tool behave the same as outside of bazel" mentality

I started a new branch at https://github.com/alexeagle/rules_nodejs/tree/babel - could we pair up to get that to parity with what you need? I haven't used Babel much.

@pward123 are you still using rules_nodejs? I see that you also did some hacking on this rule. Want to help get it upstreamed?

Thanks!

[alexeagle]

We now run babel in the examples, so in theory you don't need a babel_library rule anymore.
I think we had more ideas here, but it's the oldest PR and is probably too stale to imagine we'll clean it up to merge.