Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Interoperate with rules_closure #174

Closed
wants to merge 8 commits into from
Closed

Conversation

ribrdb
Copy link

@ribrdb ribrdb commented Mar 22, 2018

Issue #6
There's two parts to this:

  • Update tsc_wrapped to actually use tsickle for the .closure.js files, so rules_closure can compile them.
  • Allow closure_js_library in the deps of a ts_library. This is implemented using an aspect, and depends on being able to find a .d.ts for the closure_js_library.

This works with bazelbuild/rules_closure#261 which makes 2 changes to rules_closure:

  • Use clutz to add a .d.ts generating action to every closure_js_library
  • Allow ts_library in the deps of closure_js_library or closure_js_binary.

@pshields
Copy link
Contributor

How exciting! Thanks for working on this.

@kalbasit
Copy link
Contributor

@ribrdb is this stalled or still being planned for merging?

@ribrdb
Copy link
Author

ribrdb commented Jun 19, 2018

Well it's stalled waiting for the Google teams to decide how they want the projects to work together.
I've been using my forked version for the last few months.

@kalbasit
Copy link
Contributor

@ribrdb is it possible to update both of your PRs to the latest versions (or the latest master)?

@kalbasit
Copy link
Contributor

kalbasit commented Jun 19, 2018

@ribrdb maybe I'm using it wrong, but getting an error

ts_library(
    name = "lib_ts",
    srcs = ["lib.ts"],
    node_modules = "@root_node_modules//:node_modules",
    tsconfig = "//:tsconfig.json",
    visibility = ["//visibility:public"],
    deps = [
        "@io_bazel_rules_closure//closure/library",
        "//some/other/lib",
    ],
)

closure_js_binary(
    name = "closure_bin",
    deps = [":lib_ts"],
)
$ bazel build //assets:closure_bin
(11:30:30) INFO: Current date is 2018-06-19
(11:30:30) ERROR: /home/kalbasit/code/src/github.com/org/prj/src/assets/BUILD.bazel:29:1: in ts_library rule //assets:lib_ts:
Traceback (most recent call last):
        File "/home/kalbasit/code/src/github.com/org/prj/src/assets/BUILD.bazel", line 29
                ts_library(name = 'lib_ts')
        File "/home/kalbasit/.cache/bazel/_bazel_kalbasit/0d879f5af7f2dfa9543ca6b701f805d5/external/build_bazel_rules_typescript/internal/build_defs.bzl", line 145, in _ts_library_impl
                compile_ts(ctx, is_library = True, compile_acti..., <2 more arguments>)
        File "/home/kalbasit/.cache/bazel/_bazel_kalbasit/0d879f5af7f2dfa9543ca6b701f805d5/external/build_bazel_rules_typescript/internal/common/compilation.bzl", line 167, in compile_ts
                assert_js_or_typescript_deps(ctx)
        File "/home/kalbasit/.cache/bazel/_bazel_kalbasit/0d879f5af7f2dfa9543ca6b701f805d5/external/build_bazel_rules_typescript/internal/common/compilation.bzl", line 68, in assert_js_or_typescript_deps
                fail((((("%s is neither a TypeScript ..."))
@io_bazel_rules_closure//closure/library:library is neither a TypeScript nor a JS producing rule.
Dependencies must be ts_library, ts_declaration, or JavaScript library rules (js_library, pinto_library, etc, but also proto_library and some others).
(11:30:30) ERROR: Analysis of target '//assets:closure_bin' failed; build aborted: Analysis of target '//assets:lib_ts' failed; build aborted
(11:30:30) INFO: Elapsed time: 0.209s
(11:30:30) INFO: 0 processes.
(11:30:30) FAILED: Build did NOT complete successfully (1 packages loaded)

WORKSPACE:

http_archive(
    name = "io_bazel_rules_closure",
    sha256 = "cbdeac3e610982e60c1af5695191d46e4f432cdbb30ac8535820aaf1e285abcc",
    strip_prefix = "rules_closure-2a78960a3c64df0bf1311e07269ea28be476e848",
    urls = [
        "https://github.com/ribrdb/rules_closure/archive/2a78960a3c64df0bf1311e07269ea28be476e848.tar.gz",
    ],
)

http_archive(
    name = "build_bazel_rules_typescript",
    sha256 = "c90922d113c081661847e919fb170bfac3c5101efa981328b4fa271e774a5793",
    strip_prefix = "rules_typescript-321a07b8d8c7a7ed26bbe1b8e17b7362b705f5b8",
    url = "https://github.com/ribrdb/rules_typescript/archive/321a07b8d8c7a7ed26bbe1b8e17b7362b705f5b8.zip",
)

@ribrdb
Copy link
Author

ribrdb commented Jun 19, 2018

@kalbasit What are you trying to do?
To compile your typescript code with the closure compiler, you need to create a closure_js_library that wraps your ts_library:

closure_js_library(
    name="lib_js",
    ts_lib=":lib_ts",
)
closure_js_binary(
    name = "closure_bin",
    deps = [":lib_js"],
)

Why do you have closure_library in the dependencies for your ts_library? Are you trying to use the closure library from your typescript code? That's more complicated -- we've been assuming not many people would want to do that.

@kalbasit
Copy link
Contributor

Why do you have closure_library in the dependencies for your ts_library

I was trying to debug the issue, and I saw this in your PR here.

Thanks, I'll give it a try tomorrow and let you know. I would love to have your PRs up to date, do you think you have some time for that?

@ribrdb
Copy link
Author

ribrdb commented Jun 24, 2018

I tried updating, but some things aren't working right. I'll need to spend some more time debugging to see what's going on.

@kalbasit
Copy link
Contributor

Thank you @ribrdb for your time!

@kalbasit
Copy link
Contributor

kalbasit commented Jul 9, 2018

@ribrdb I see you've updated this PR and bazelbuild/rules_closure#261. Are both PRs working together now?

@ribrdb
Copy link
Author

ribrdb commented Jul 11, 2018

Yes, they should be working now.

@LouisStAmour
Copy link

Just in case it helps anyone, here are two alternative approaches (read the full issue thread as I posted updated code later on) on integrating rules_typescript with rules_closure without code changes to either. Feedback welcome! https://github.com/bazelbuild/rules_typescript/issues/344

@alexeagle
Copy link
Contributor

This PR is old. Still excited to get the feature in but I think we'll need a new PR. Probably against rules_nodejs as most of this repo is moving there today.

@alexeagle alexeagle closed this Feb 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants