rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

Discussion: interop with closure compiler

Open LStAmour opened this issue 5 years ago • 26 comments

Note: This ticket is actually about external use of the examples, or how you'd use these rules in practice. But if you get technical, the bug to fix is that examples are tested only with the internal compiler, and so you have to override the compiler to run the examples for closure interop.

I was trying to get tsickle to run and produce output suitable for use with rules_closure. I had my tsconfig.json set up as:

{
    "compilerOptions": {
        "target": "es5",
        "types": []
    },
    "bazelOptions": {
        "es5Mode": true,
        "tsickle": true,
        "tsickleGenerateExterns": true
    }
}

I was trying to get my background.ts file to build using tsickle and an example's es5_consumer:

load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")

ts_library(
    name = "background",
    srcs = ["background.ts"],
    tsconfig = "tsconfig.json",
)

load("@build_bazel_rules_typescript//examples/es5_output:es5_consumer.bzl", "es5_consumer")

es5_consumer(
    name = "background_es5",
    deps = [":background"],
)

But I kept getting the following error:

Error: When setting bazelOpts { tsickle: true }, you must also add a devDependency on the tsickle npm package
    at emitWithTsickle (.../bazel-out/host/bin/external/build_bazel_rules_typescript/@bazel/typescript/tsc_wrapped.runfiles/npm/node_modules/@bazel/typescript/tsc_wrapped/tsc_wrapped.js:290:19)

And nothing I did seemed to resolve it.

Eventually I started digging into rules_typescript, and discovered the custom internal compiler still had a dependency on @npm//tsickle so I managed to write my own compiler wrapper to fix this:

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")

# The following is required to run tsickle
nodejs_binary(
    name = "tsc_wrapped",
    entry_point = "@bazel/typescript/tsc_wrapped/tsc_wrapped.js",
    # The --expose-gc node option is required for tsc_wrapped
    templated_args = ["--node_options=--expose-gc"],
    data = [
        "@npm//protobufjs",
        "@npm//source-map-support",
        "@npm//tsickle",
        "@npm//tsutils",
        "@npm//typescript",
        "@npm//@bazel/typescript",
    ]
)

load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")

ts_library(
    name = "background",
    compiler = ":tsc_wrapped",
    srcs = ["background.ts"],
    tsconfig = "tsconfig.json",
)

load("@build_bazel_rules_typescript//examples/es5_output:es5_consumer.bzl", "es5_consumer")

es5_consumer(
    name = "background_es5",
    deps = [":background"],
)

... and everything started working as expected! 👍

So I'm filing this issue for two reasons:

  1. It would be nice if there were a way to run the examples using the external, public compiler instead of the internal one. (Or to enforce similar enough behaviour from both compilers...)

  2. Any suggestions on how to better run rules_typescript, tsickle and rules_closure together? Am I on the right track? I'd appreciate any pointers or tips, I'm still very new to Bazel, but I'm looking to use the public APIs rather than relying on third-party forks for this integration.

LStAmour avatar Dec 07 '18 17:12 LStAmour

Okay, I think I've got Google Closure compiler hooked up to rules_typescript now. If anyone's interested, here's where I'm at right now. I haven't tested this beyond simple one-line TS files, I probably need to switch to glob functions soon.

I've created a macro called closure_ts_bin to wrap the TypeScript-to-Closure compilation (it takes the name of a .ts file and compiles it to .js using Closure), and I'm sure my use of examples/es5_output:es5_consumer.bzl is going to cause problems as I haven't looked into the data structures of rules_typescript, but it works for now. :) The setup_compiler function wraps the tsc compiler to include the tsickle dep as mentioned earlier, in the description.

BUILD.bazel

package(default_visibility = ["//visibility:public"])

load(":closure_ts_bin.bzl", "setup_compiler", "closure_ts_bin")

setup_compiler()

closure_ts_bin(
    name = "background"
)

closure_ts_bin(
    name = "content_script"
)

closure_ts_bin(
    name = "options"
)

closure_ts_bin(
    name = "popup"
)

closure_ts_bin.bzl

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")
load("@build_bazel_rules_typescript//examples/es5_output:es5_consumer.bzl", "es5_consumer")
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library", "closure_js_binary")

def setup_compiler():
    # The following is required to run tsickle
    nodejs_binary(
        name = "tsc_wrapped",
        entry_point = "@bazel/typescript/tsc_wrapped/tsc_wrapped.js",
        # The --expose-gc node option is required for tsc_wrapped
        templated_args = ["--node_options=--expose-gc"],
        data = [
            "@npm//protobufjs",
            "@npm//source-map-support",
            "@npm//tsickle",
            "@npm//tsutils",
            "@npm//typescript",
            "@npm//@bazel/typescript",
        ]
    )

def closure_ts_bin(name):
    ts_library(
        name = name+"-ts",
        compiler = ":tsc_wrapped",
        srcs = [name+".ts"],
        tsconfig = "tsconfig.json"
    )

    es5_consumer(
        name = name+"-es5",
        deps = [":"+name+"-ts"]
    )

    closure_js_library(
        name = name,
        srcs = [":"+name+"-es5"],
        no_closure_library = True
    )

    closure_js_binary(
        name = name+"_bin",
        deps = [":"+name]
    )

WORKSPACE

(Note: At time of writing, Bazel 0.21.0 seemed most compatible with the HEAD version of rules_closure, which is why I used it. I also had to re-install bazel, because I needed brew install bazelbuild/tap/bazel instead of brew install bazel -- the latter was causing compilation of rules_closure to fail due to JDK differences.)

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "build_bazel_rules_typescript",
    url = "https://github.com/bazelbuild/rules_typescript/archive/0.21.0.zip",
    strip_prefix = "rules_typescript-0.21.0",
)

# Setup Google Closure Compiler rules
# They don't release frequently enough!
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "io_bazel_rules_closure",
    commit = "a34455e0c76769df13d25b67851252559fe7f83d",
    remote = "https://github.com/bazelbuild/rules_closure.git"
)
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_repositories")
closure_repositories()

# Fetch our Bazel dependencies that aren't distributed on npm
load("@build_bazel_rules_typescript//:package.bzl", "rules_typescript_dependencies")
rules_typescript_dependencies()

# Setup TypeScript toolchain
load("@build_bazel_rules_typescript//:defs.bzl", "ts_setup_workspace")
ts_setup_workspace()

# Setup the NodeJS toolchain
load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories", "yarn_install")
node_repositories()

# Setup Bazel managed npm dependencies with the `yarn_install` rule.
# The name of this rule should be set to `npm` so that `ts_library` and `ts_web_test_suite`
# can find your npm dependencies by default in the `@npm` workspace. You may
# also use the `npm_install` rule with a `package-lock.json` file if you prefer.
# See https://github.com/bazelbuild/rules_nodejs#dependencies for more info.
yarn_install(
  name = "npm",
  package_json = "//:package.json",
  yarn_lock = "//:yarn.lock",
)

LStAmour avatar Dec 07 '18 21:12 LStAmour

@LStAmour Use the Rollup toolchain to include external dependencies.

src/module.ts
# The following is required to run tsickle
nodejs_binary(
    name = "tsc_wrapped",
    data = [
        "@npm//@bazel/typescript",
        "@npm//protobufjs",
        "@npm//source-map-support",
        "@npm//tsickle",
        "@npm//tsutils",
        "@npm//typescript",
    ],
    entry_point = "@bazel/typescript/tsc_wrapped/tsc_wrapped.js",
    # The --expose-gc node option is required for tsc_wrapped
    templated_args = ["--node_options=--expose-gc"],
)

ts_library(
    name = "module_ts_lib",
    srcs = glob([
        "*.ts",
    ]),
    compiler = ":tsc_wrapped",
    tsconfig = "//:tsconfig",
    deps = [
        "@npm//@types/chrome",
    ],
)

rollup_bundle(
    name = "module_bundle",
    entry_point = "src/module",
    node_modules = "//:node_modules",
    deps = [
        ":module_ts_lib",
    ],
)

closure_js_library(
    name = "module_js_lib",
    srcs = ["//src:module_bundle.umd.js"],
    suppress = [
        "reportUnknownTypes",
    ],
)

closure_js_binary(
    name = "module_js_bin",
    deps = [":module_js_lib"],
)

Wrapping your build rules with macros adds additional overhead. You won't be able to fine grain your dependencies or closure suppression warnings.

cherryland avatar Dec 08 '18 20:12 cherryland

Thanks for the suggestion, @cherryland! Much appreciated. I'll try that Monday to compare it with my previous approach. I'll also look at it for a better idea on how I could replace or supplement the es5_consumer example code I was using.

But after a brief scan of the source at rollup_bundle.bzl#L395 I'm concerned that some of the benefits of tsickle might be lost as Uglify runs (twice) during the rollup_bundle rule as it creates a umd? Doesn't that defeat the use of Closure compiler in that it won't have type information (why you'd then suppress reportUnknownTypes) and you'd have to tell rollup, uglify, tsc, and closure to preserve existing sourcemap data from the first tsc run, which seems like a lot of work. :)

Of course ... there is always the question, what benefits are there to running Closure Compiler with types from TypeScript, if TypeScript has already checked the types? I'm not sure if Closure Compiler does this, but outside of interop with Closure code natively rather than via TSC, the only mention of types allowing for optimizations appear to be things like BuckleScript where types are optimized to more efficient representations -- something TSC would likely know more about than Closure.

Edit: I now see that trickle adds suppressions for various errors from Closure Compiler that they don't want to deal with: fileoverview_comment_transformer.ts#L46 and I'm getting an additional error as tsickle isn't automatically converting an rpm-imported type d.ts into a closure extern apparently...

Speaking of TSC vs Closure, for now I'm taking the approach of using TypeScript to convert things to ES5, on the assumption that Microsoft would offer more compatibility in TS output to IE than Google would with Closure's support of ES6 to ES5. Another thought is to build browser-specific versions of JS, just as you might build browser-specific versions of CSS. I'm not sure if that's still considered a best practice or simply redundant as more of everything becomes webkit or blink ...

Wrapping your build rules with macros adds additional overhead. You won't be able to fine grain your dependencies or closure suppression warnings.

That's really useful, thanks. I suppose ... I suppose, Googling, that I can create custom deps args for my macro which accept label_list as in -- oh wait ... I see. It makes more sense now. Macros are shorthand for expansion while Rules require additional markup to specify types (presumably to wrap and validate inputs...) and are the basic building blocks of Bazel unlike macros. This also explains the prominent section on debugging macros though there's undocumented experimental support (currently VS Code only) for a Starlark debugger which is exciting to try. 👍 (I've been using the IntelliJ plugin so far.)

Edit: Another reason I've switched from the IntelliJ plugin to the official VS Code extension is that the IJ plugin doesn't support non-executable ts_config rules. Additionally, it's been made clear that I'd have to fork typescript or closure rules in order to create my own rules based on them because rules can't be executed natively by other custom rules in starlark code (.bzl files), only in macros, which is another difference between macros and custom rules. Custom rules have to rely on other Starlark functions, most of which are published with internal visibility. (Visibility would be the next thing I have to investigate and learn about Bazel...)

LouisStAmour avatar Dec 10 '18 08:12 LouisStAmour

While I can’t speak to the overhead issue of using macros, I can report back that it’s possible to create macro arguments defaulting to [] that can pass through to different rules for deps or suppression. I’ll post another example shortly, I used this approach, adding arrays within the macro to combine them for srcs, to add closure externs for one of my scripts equivalent to @types/chrome as for some reason, tsc wasn’t complaining but closure didn’t recognize top-level chrome API calls. After including the externs as a closure_js_library with a build file specified in the workspace to collect the js files from a recent closure release on GitHub, all was right with the world again... at least until the next bit of TypeScript breaks the build. (fingers crossed!)

LouisStAmour avatar Dec 11 '18 08:12 LouisStAmour

The default output of a rollup_bundle rule is the non-debug-minified es5 bundle. However you can request one of the other outputs with a dot-suffix on the target's name.

In the example above I'm requesting an unmodified UMD bundle

https://bazelbuild.github.io/rules_nodejs/rollup/rollup_bundle.html#rollup_bundle

Add the necessary references to your tsconfig.json

{
  "bazelOptions": {
    "tsickle": true
  },
}

You can overwrite the closure_js_binary rule output language, which by default falls back to ES3

closure_js_binary(
    ...
    language = "ECMASCRIPT_2015",
)

Writing a custom build rule is a good starting point to get more familiar with skylark.

cherryland avatar Dec 11 '18 23:12 cherryland

@LouisStAmour I can invite you to a private repository that provides the necessary bazel build rules to package Chrome Extensions crx, if you are interested.

cherryland avatar Dec 11 '18 23:12 cherryland

I am very interested, yes. After a few more days' work, I've managed to simplify (or complicate) things to the following -- I got rid of the macro after I realized it was unnecessary to create so many libraries for otherwise shared source code:

src/BUILD.bazel (in src folder with my *.ts files)

package(default_visibility = ["//visibility:public"])

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")
load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")
load("@build_bazel_rules_typescript//examples/es5_output:es5_consumer.bzl", "es5_consumer")
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library", "closure_js_binary")

nodejs_binary(
    name = "tsc_wrapped",
    entry_point = "@bazel/typescript/tsc_wrapped/tsc_wrapped.js",
    # The --expose-gc node option is required for tsc_wrapped
    templated_args = ["--node_options=--expose-gc"],
    data = [
        "@npm//protobufjs",
        "@npm//source-map-support",
        "@npm//tsickle",
        "@npm//tsutils",
        "@npm//typescript",
        "@npm//@bazel/typescript",
    ]
)

ts_library(
    name = "ts",
    compiler = ":tsc_wrapped",
    srcs = glob(["**/*.ts"]),
    tsconfig = "//:tsconfig.json"
)

es5_consumer(
    name = "es5",
    deps = [":ts"]
)

closure_js_library(
    name = "src",
    srcs = [":es5"],
    no_closure_library = True,
    deps = [
        "@closure-compiler//:externs"
    ]
)

closure_js_binary(
    name = "content_script_bin",
    entry_points = [
        '__main__.src.content_script'
    ],
    deps = [":src"]
)

closure_js_binary(
    name = "options_bin",
    entry_points = [
        '__main__.src.options', 
    ],
    deps = [":src"]
)

resources/BUILD.bazel (contains static files for copying into the genfiles output folder)

package(default_visibility=["//visibility:public"])

filegroup(
    name = "resources",
    srcs = glob(["**"], exclude = ["*.bazel", "*.bzl"]),
)

BUILD.bazel (project root, for compiling a zip/folder)

# https://github.com/google/nomulus/blob/master/java/google/registry/builddefs/zip_file.bzl
# merged with https://github.com/google/nomulus/blob/master/java/google/registry/builddefs/defs.bzl
load(":zip_file.bzl", "zip_file")

package(default_visibility = ["//visibility:public"])

exports_files([
  "tsconfig.json",
])

zip_file(
  name = "chrome-extension-zip",
  srcs = [
    "//resources:resources",
    "//src:content_script_bin",
    "//src:options_bin",
  ],
  out = "chrome-ext.zip",
  mappings = {
    "__main__/src": "",
    "__main__/resources": ""
  }
)

genrule(
  name = "chrome-extension-unpacked",
  srcs = ["//:chrome-extension-zip"],
  outs = ["chrome-ext"],
  cmd = "unzip -q -d $(@D)/chrome-ext $(location //:chrome-extension-zip)",
  executable = 1
)

WORKSPACE (defines @closure-compiler//:externs above)

# ... after all the rules defined in my earlier post, I've now added:

# Download chrome_* contrib externs from a recent release of closure compiler
# TODO: Investigate replacing with rules_closure's copy of closure if it includes externs...
_CHROME_EXTERNS = """
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library")

closure_js_library(
    name = "externs",
    srcs = glob(['contrib/externs/chrome*.js']),
    visibility = ["//visibility:public"],
    no_closure_library = True
)
"""

http_archive(
    name = "closure-compiler",
    url = "https://github.com/google/closure-compiler/archive/v20181125.zip",
    strip_prefix = "closure-compiler-20181125",
    sha256 = "2a49b06a2c68034ef7114b6692fc853a2318c7ead5f8cd1263cc42f9ca1af441",
    build_file_content = _CHROME_EXTERNS
)

And as it turns out this matters a great deal, here's my current tsconfig.json:

tsconfig.json

{
    "compilerOptions": {
        "types": [
            "chrome"
        ],
        "noImplicitAny": true,
    },
    "exclude": [
        "bazel-*"
    ],
    "bazelOptions": {
        "es5Mode": true,
        "tsickle": true,
        "tsickleGenerateExterns": true,
        "tsickleExternsPath": "externs.js",
        "googmodule": true
    }
}

... I should probably make an example repo out of this once I get test cases going. Every time I add more code, something I haven't yet encountered inevitably breaks my build and I have to fix things. I'm really annoyed with having to specify an extern for closure when I've already specified types for typescript -- but I recognize there are limits to what the tsickle team can accomplish mapping TypeScript syntax to Google Closure syntax.

In one place, Closure got confused about the type of some constant I used with document.evaluate xpath querying and so I had to use a suppression comment there. If that were more common, I'd probably permanently suppress that part of Closure, but it's nice to not have any suppressions in my use of Closure (except the ones added by comments from tsickle).

LStAmour avatar Dec 11 '18 23:12 LStAmour

Right now the best example I can find of using Bazel with a Chrome extension is the detangle extension: https://github.com/google/detangle/blob/400d8a321230df9fb1af0b96c2cbf585375f7b3e/chrome_ext/BUILD

And it’s interesting that independently, we both came up with the idea of just packaging Closure externs exported from a Closure Compiler release via a workspace rule. Perhaps highlights a missing feature of rules_closure, or one that requires documentation — how to access externs from a public rules_closure library, if we don’t need a specific version of these externs but can live with whatever ships with the closure compiler version imported by rules_closure at the time... but then that’s a different ticket for the rules_closure folks, I suppose.

One of the next things worth documenting here would be creating test cases for Bazel and getting builds and tests running in Docker (or later remote build). I expect it’ll be straightforward but something always seems to come up unexpectedly so I’m looking forward to the challenge.

I’m getting more convinced that Bazel will be a big contender after oh, maybe one more big rewrite/push from Google to simplify and adopt more of these moving pieces internally. It has quite a lot of promise but rough edges — like the lack of a good story for linting or significant IDE integration, or having to spend over two weeks watching videos and reading source code in order to find the dozens of lines required to build TypeScript/Closure Chrome extension code... that’s an impediment to broad adoption.

I’m a firm believer that widely and permissively sharing tech and examples is the fastest way to improve it if it’s relatively low level or example code, so that’s why I’ve been sharing things here so far — my adoption of Bazel has been this successful because of GH issues, PRs and searching a lot of code samples on GH for keywords. Happy to give back here where I can.

LouisStAmour avatar Dec 12 '18 05:12 LouisStAmour

Well, just a short note that I've been having a heck of a time getting closure_js_library or closure_js_binary to work with a node_modules folder, either as a filegroup or as a rules_nodejs-managed dependency. I'm going to have to investigate either using rollup against node_modules/typescript deps, or I'll have to find another solution. I've switched from my above code with tsickle to just using an es2017 (default) output from typescript, with no special Closure markup processing, but I'm still stuck on getting Closure to load node modules via Bazel.

Temporarily, I'm using rollup as you'd mentioned and I've completely dropped closure compiler. It works fine, but it feels a lot slower than plain closure compiler alone (once you're past the protobuf install phase, that is...) I'm going to have to go through the above PRs for rules_closure and rules_typescript to see what I can learn about getting both projects to make nice.

LStAmour avatar Dec 15 '18 00:12 LStAmour

I've been trying to update my rules_closure interop stuff to the latest version of rules_typescript, but haven't been able to get it working. These are the changes I have in my branch of rules typescript:

  • The tsconfig needs to have the module type set to commonjs so tsickle will generate goog.modules. We used to use es6 import with closure compiler but kept hitting bugs. This is the main reason we're using tsickle is so we can write typescript with es6 imports but get goog.modules out that closure compiler supports completely. https://github.com/bazelbuild/rules_typescript/blob/d489eefa7e43851e2e675e423ea8df0cc857cdd0/internal/common/tsconfig.bzl#L189
  • The externs.js files rules_typescript outputs are empty: https://github.com/bazelbuild/rules_typescript/blob/3783082839a27edcf1cdf207fc370a242f0f9a59/internal/build_defs.bzl#L90

If those things are fixed, you can use my branch of rules_closure to create a closure_js_library that wraps a ts_library: closure_js_library(name = "foo_js", tslib="foo")

For dependencies, things are a bit trickier. I've got two types of dependencies: those that are already closure_js_libraries that I need to use from typescript, or things from npm.

For npm dependencies, I just assume they won't work with closure's advanced compilation, so I don't try to import them directly. Most of the npm packages have a browser version that will expose the package as a global. That's what I use, if I can't find one I build my own using webpack and check that in. Then I find a .d.ts file for the library and if necessary tweak it to work as a global instead of a module. Tsickle will then convert this into the appropriate closure externs

For closure dependencies, what I need to do is create a ts_library that wraps a closure_js_library. You set the srcs to a .d.ts file (I have a partially handwritten one I use for closure library, the rest I auto-generate using clutz), and runtime_deps to the closure_js_library. However there's two problems:

  • ts_library doesn't allow closure_js_library in runtime_deps: https://github.com/bazelbuild/rules_typescript/blob/3783082839a27edcf1cdf207fc370a242f0f9a59/internal/common/compilation.bzl#L43
  • runtime_deps ins't exported in the typescript provider so you can't access it without a custom aspect
  • You need to turn off the externs generation for this library. Based on my reading of the code this means you need to set both isLibrary and generate_externs to False. But there's no way to change isLibrary at the moment.

ribrdb avatar Dec 17 '18 13:12 ribrdb

@ribrdb Thanks for your reply! That's a good point about third-party modules' compatibility with advanced Google Closure compilation. I suppose I'd been taking for granted that if the modules compiled via TypeScript and somehow all the deps for my TS code went through Tsickle, that the output would be guaranteed to compile under Closure Compiler in advanced mode. But thinking about it, historically, my closure compiler webpack config often also relied on pre-compiled code, and I suppose running something like rollup on dependencies might be one way to workaround modules that don't ship pre-compiled versions. It's just ... less compatible with TypeScript that way, usually, as tsc has less knowledge of the code from the compiled version. Hence the need for a .d.ts file as you've noted.

As to getting rules_typescript to output goog.module syntax, have you tried modifying your tsconfig.json file to add the line:

{
    "compilerOptions": {
    },
    "bazelOptions": {
        "tsickle": true,
        "googmodule": true
    }
}

I had some success getting things to work with the above notation, as otherwise rules_typescript will, in ES5 mode, rewrite the paths to be absolute for local source code (as if the local source was being published as a an npm module) but then doesn't export the folder expecting it to be further used as an dep in the external folder by ts_binary, I presume, when that's incompatible with rules_closure's idea of deps or src (because a path for the module is given to tsc but closure compiler doesn't appear to support such paths). The solution to get it to output relative requires appeared to be making sure es2015 or better is being output by the ts_library, then it started working. The reason I preferred that over goog.module was because the goog.module syntax applied to node_modules but then I'd need to re-write or export wrappers for npm modules to identify them with goog.module syntax. The alternative, using relative paths, meant I could maybe get compilation working if I included the npm sources and configured closure compiler to understand commonjs, in loose mode (to avoid goog.provide) and trying to get closure to create the right module require roots, perhaps by reading this file closely.

I guess my thought was that if I couldn't get rules_typescript to rewrite all my external dependencies as build outputs, then my only other option would be to configure or extend rules_closure to better interop with the npm support we currently have in rules_nodejs and barring that, to use filegroups and defs (command line parameters for rules_closure) to hack in node_modules support. Right now, rules_closure pretends that the entirety of rules_nodejs doesn't exist, unlike rules_typescript which was rewritten to support it.

LStAmour avatar Dec 17 '18 15:12 LStAmour

Hmm, it's been a while. I'm pretty sure rules_typescript was overwriting something in my tsconfig so it didn't work. I don't think I tried changing the bazelOptions though. Are you saying you changed the es5 output to go through tsickle? I've only been using the .closure.js files, I don't use the es5 ones at all. What sort of requires are you getting and/or wanting? I want the goog.module ones, which look like: var tsickle_module_0_ = goog.require('workspace.path.to.file'); If I used any imports other than goog.require with closure compiler I'd ocassionally get bugs where things compiled sucessfully but big chunks of code would be missing from output .js file and nothing would work.

ribrdb avatar Dec 17 '18 16:12 ribrdb

I started by only using ES5 output, then went to ES6 output. In both cases, with the latest typescript rules, I was able to get googmodule output for my TS source files using a tsconfig override, as implemented over here: tsconfig.ts#L294

As far as I know, the above flag requires tsickle as implemented in this example: examples/googmodule

The problem I'm having is how to then get rules_closure to work with node_modules under googmodule naming conventions. One option would be to get a rollup rule functioning to create a single source file for each npm module and stick the appropriate goog.module header on it.

But I'd prefer using Closure Compiler's native support for commonjs modules and nodejs package.json files: 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.

So the solution I'm now considering is finding or writing a stripped down rules_closure that is much more permissive, or integrates better with rules_nodejs to allow npm modules as deps. I just don't know enough about either Closure compiler or Bazel to know what this will look like without some experimentation first, and I'm not sure I've that kind of time right now. It's possible I'll get farther by submitting an issue against rules_closure to support nodejs, but they've not shown any interest in supporting npm yet. (They seem excessively picky about things, everything else in this ecosystem.)

And the other option is -- as was possibly proposed earlier by @cherryland -- to use rollup against my npm dependencies (and/or my entire codebase) before passing them in as a single source file to closure. This gets rid of the module resolution problem, and the lack of non-js files support, but it would be ideal to have flexible rules for closure compiler as it would save compiler time.

LStAmour avatar Dec 17 '18 16:12 LStAmour

I briefly looked into using the .zip file support in rules_closure and the hidden --jszip arg for Closure, but without testing, it looks like I might have to ensure all the files in the .zip are compatible with Google Closure, and that it's meant for processing a zip file of externs. If I want rules_nodejs compatibility with rules_closure, it looks like I'll have to investigate further what rules_closure is doing, and I'm not sure I've the time before the new year.

LStAmour avatar Dec 17 '18 18:12 LStAmour

Okay, I thought Rollup was slower than Closure, but it has nothing on how slow Webpack is. Still, for folks wanting to use Webpack with nodejs (and use babel-loader with @babel/preset-typescript etc.) the following rules worked fine for running Webpack under Bazel -- there are probably trivial improvements to make, though, I'm not a Bazel expert. Related: A more Bazel-like version of Webpack might be in the works https://github.com/bazelbuild/rules_nodejs/issues/203

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary")

nodejs_binary(
    name = "webpack_binary",
    entry_point = "webpack-cli/bin/cli",
    install_source_map_support = False,
    node_modules = "//:node_modules",
)

genrule(
    name = "webpack_output",
    srcs = glob(["**/*"]),
    outs = ["dist/index.js"],
    cmd = "$(location :webpack_binary) --output-path $(@D) --config "+PACKAGE_NAME+"/webpack.config.js",
    output_to_bindir = True,
    message = "Webpack",
    tools = [":webpack_binary", "//:node_modules"],
    visibility = ['//:__pkg__']
)

PACKAGE_NAME helped in resolving source code folders as I was running bazel from a project subfolder with its own webpack.config.js file. I'm not sure how portable this approach is...

An alternative approach to using rules_typescript might be to use Babel's limited typescript syntax support and rollup, which I started using when the typescript compiler started choking on .scss imports. Related: There's also a babel_library rule in the works at https://github.com/bazelbuild/rules_nodejs/pull/217

load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "rollup_bundle")
load(":babel.bzl", "run_babel")

nodejs_binary(
    name = "babel_binary",
    entry_point = "@babel/cli/bin/babel",
    install_source_map_support = False,
    node_modules = "//:node_modules",
)

run_babel(
    name = "babel",
    srcs = glob(['**/*.scss','**/*.ts','**/*.tsx'])+['.babelrc'],
    babel_binary = ":babel_binary"
)

rollup_bundle(
    name = "ts_rollup",
    srcs = [":babel_files"],
    entry_point = PACKAGE_NAME+"/babel.out/index.js",
    visibility = ['//:__pkg__']
)

The magic is in the run_babel macro loaded from babel.bzl:

def run_babel(name, srcs, babel_binary):
  
  output_folder = name + ".out"
  outs = []
  non_js_cp_commands = []
  
  for f in srcs:
    if f.endswith(('.ts', '.js', '.tsx', '.jsx')):
      output_filename = output_folder+'/'+f.replace(".ts", ".js").replace(".tsx", ".js").replace(".jsx", ".js")
      outs.append(output_filename)
      outs.append(output_filename+".map")
    elif f.endswith('.bazelrc') == False:
      source_filename = PACKAGE_NAME+'/'+f
      output_filename = output_folder+'/'+f
      outs.append(output_filename)
      non_js_cp_commands.append("cp '"+source_filename+"' '$(@D)/"+output_filename+"'")
  
  native.genrule(
    name = name,
    srcs = srcs,
    outs = outs,
    cmd = "$(location babel_binary) "+PACKAGE_NAME+" --extensions '.ts,.tsx,.js,.jsx' --out-dir $(@D)/"+output_folder+" --source-maps && "+(" && ".join(non_js_cp_commands)),
    output_to_bindir = True,
    message = "Running Babel to transform TS to JS...",
    tools = [babel_binary, "//:node_modules"]
  )
  
  native.filegroup(
    name = name + "_files",
    srcs = outs
  )

There's a bit of extra code in that macro to copy inputs to the output folder, with the exception of the .bazelrc file which I used to bring the .scss source files along for the ride.

For TypeScript support, your .babelrc file could look like:

{
    "presets": [
        "@babel/preset-react",
        "@babel/preset-typescript",
        [
            "@babel/preset-env",
            {
                "targets": "last 8 Chrome versions",
                "modules": false,
            }
        ],
    ],
    "plugins": [
        "@babel/proposal-class-properties",
        "@babel/proposal-object-rest-spread",
        ["react-css-modules", {
            "filetypes": {
                ".scss": {
                    "syntax": "postcss-scss"
                }
            }
        }]
    ]
}

Note that because rollup_bundle does not work with a custom rollup config and you can't set a custom list of plugins, there's no way to use rollup_bundle directly to compile the scss. But there's internal support to use plugins and Angular's version of rollup_bundle has a plugin it loads internally. So maybe I could add plugins that way. If plugin support existed for rollup_bundle, I'd be able to use rollup-plugin-babel and avoid all of my custom macro code for Babel entirely. Some existing tickets to look at: https://github.com/bazelbuild/rules_nodejs/pull/234 and https://github.com/bazelbuild/rules_nodejs/issues/460

I'm actually not sure I want to keep using rollup_bundle for other reasons -- if I've already run tsc, why run it a second or third time if I only want es6 anyway? https://github.com/bazelbuild/rules_nodejs/pull/235 But it seems like a time-saver to not have to write my own rollup.config.js for the Bazel runtime environment. (I tried varying my Webpack rule above and but it's more work than I expected to get the module resolution correct under Rollup.)

Also on my mind considering build systems and minifcation: the merits of JSX vs lit-html vs babel-plugin-htm vs whatever YouTube uses internally, how to add CSS to all this webcomponent v1 malarkey, and when React will add <template> element support for large blocks of unchanging JSX in addition to the expected async rendering improvements next year, and which will happen first: rules_closure getting npm module support or rules_nodejs adding a simple closure wrapper.

It'd be nice if in 2019, Google had some clear coherent guidance from the Chrome/Closure/Polymer teams on replacing chunks of webpack and styles-loader with something more performant for building es6 modules, loaders and web components with special emphasis on CSS support, and some version of lit-html that optimizes its code at compile time to use a VDOM or diffing when heuristics indicate it would perform better than <template>, or something like that. Adding a lightweight browser API for easy reuse of DOM nodes when scrolling in really long tables or infinite scroll, and re-visiting date pickers and other HTML5-native controls to improve them on desktop platforms, would be on my nice to have list also.

LStAmour avatar Dec 21 '18 18:12 LStAmour

Hi, I haven't been paying attention to this thread, but I'm interested to know if we could have Closure interop by making some simple changes? In the past it has seemed like a large-scope project and we haven't had time/priority to take it on. @LStAmour or @ribrdb do you want to take the lead on it?

alexeagle avatar Feb 13 '19 17:02 alexeagle

note that the OP was a specific bug, we're working on fixing it in bazelbuild/rules_typescript#398

I changed the title since this thread has become broader than the bug

alexeagle avatar Feb 13 '19 18:02 alexeagle

I think it should be pretty small at this point, and I'd be happy to take the lead on it. Right now I think the main problem is getting the externs generation to work. If you turn on tsickle and try to build the .closure.js files it fails because both the skylark code and tsickle try to generate the .externs.js file.

ribrdb avatar Feb 13 '19 19:02 ribrdb

@ribrdb thanks for offering to lead this, could you email me alexeagle at google dot com so we can set up a communication mechanism? Don't want this to get lost among everything our team is doing.

alexeagle avatar Feb 22 '19 17:02 alexeagle

@LStAmour, @LouisStAmour, @cherryland - i am also building a chrome extension, among other things, in typescript/closure. thank you for this thread, it offers a ton of useful help

@cherryland - i would be super interested in bazel rules to package extensions, if you are willing to share an invite to that repo. so far i package it manually using pkg_tar and a gulpfile, but of course this is far from ideal.

sgammon avatar Jun 10 '19 03:06 sgammon

@LouisStAmour food for thought... we had requested that elemental2 include generated Java files from Chrome API externs. this would allow chrome extensions to be coded in J2CL. i would be curious if this would help your use case, too

sgammon avatar Jun 10 '19 03:06 sgammon

I have the initial version of my rules for closure compiler interop available at https://github.com/derivita/rules_ts_closure

This is a very early release. We've been using it for several months at my company, but we're still working on documentation and examples. The goal is to eventually get this merged into rules_nodejs.

ribrdb avatar Jun 19 '19 22:06 ribrdb

What is the current status of this? In my reading of https://bazelbuild.github.io/rules_nodejs/TypeScript.html it seems like ts_library is highly discouraged but ts_project does not use tscc or tsickle. I also found this article by @alexeagle which says ts_library will probably be retired soon, and it doesn't sound like ts_project is going to support closure-compatible transpilation.

For context, we currently use bazel with closure-compiler and all of our code is in Javascript. The usage of closure compiler is critical for our site because we operate in low-bandwidth environments so every byte counts. I would like to migrate our codebase to Typescript in order to optimize even further by including some of our external dependencies, but we would still need the optimizations of closure-compiler.

TLDR; is there any method for integrating bazel + typescript + closure-compiler that will be available long-term?

Would like to add that I appreciate the work all of you do to maintain incredible tools like this one.

jimmyt857 avatar Jun 21 '22 02:06 jimmyt857

ts_library is probably going to be deprecated, but you can continue using the @bazel/concatjs npm package which contains it anyway.

If you can express your transformations purely in terms of the CLI of tools, such as tsickle and google-closure-compiler, you could in theory do this in Bazel with just a macro that wraps the generated bin entries for those tools. However in my experience Closure is super complicated to interop with anything. https://github.com/bazelbuild/rules_closure might offer solutions but hasn't been released in a year.

Sorry, I think there's just not enough adoption of these tools outside of Google for anyone to make a living supporting them, they're not much fun to hobby on, and Google themselves don't put much effort behind making them usable externally...

alexeagle avatar Jun 21 '22 22:06 alexeagle

Yeah that makes sense, thank you for the quick response. It's a shame there isn't more support here, as typescript + closure seem like a perfect fit to me: if there was a seamless integration between them, then everyone could have the best of both worlds (cleanliness of typescript, optimizations of closure).

I'll see what I can do, but will probably ending up sticking with what we have for now then.

jimmyt857 avatar Jun 22 '22 05:06 jimmyt857

If it's of use to anyone, I tried a while ago to get things working by chaining ts_library -> filegroup -> closure_js_library -> closure_js_binary. https://github.com/bhainesva/bazel_typescript_closure_sample

Disclaimer that I haven't touched it in over a year and didn't really know what I was doing at the time, but I think it was mostly working. Some of the notes in the readme about problems I ran into might be relevant at least if you try to do it yourself. The approach that seemed most intuitive to me of using ts_library to get es6 output and giving that to closure had a bunch of issues that weren't obvious to me.

bhainesva avatar Jun 22 '22 14:06 bhainesva

Haven't seen any users of closure compiler asking about this in the last years, so I don't think this will ever be fixed.

alexeagle avatar Apr 23 '24 22:04 alexeagle