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

Interoperate with rules_typescript #261

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ribrdb
Copy link
Contributor

@ribrdb ribrdb commented Mar 16, 2018

There are two pieces to this:

  • An aspect which 'upgrades' a ts_library to a closure_js_library by running js_checker. Thus ts_libraries can be included in the deps of a closure_js_library or closure_js_binary. This part adds no dependencies to rules_closure. If users don't use rules_typescript the aspect will simply never do anything.
  • Support for generating .d.ts files for closure_js_libraries. This does add a dependency on clutz. However this seemed like the best way to implement this for several reasons:
    • This is the cleanest place to add the dependency, since clutz depends on the closure compiler.
    • It allows us to have a label for the generated .d.ts file, which wouldn't be possible if they were generated by an aspect.
    • The generated .d.ts file seems useful even to people who aren't using typescript. For example VS Code can read the .d.ts files to provide automatic imports and smart autocomplete in .js files.
    • I've tried to structure this so that clutz is not run (or even downloaded) unless the user is using typescript or manually builds the .d.ts files.

The corresponding changes to rules_typescript are at https://github.com/ribrdb/rules_typescript/tree/closure

@jart
Copy link
Contributor

jart commented Mar 24, 2018

Please see my email response on the rules_closure mailing list. We need the support of the rules_typescript team to go forward with something like this.

@jart jart requested a review from dslomov June 24, 2018 21:56
@jart
Copy link
Contributor

jart commented Jun 24, 2018

[Background: Here's the link to the discussion thread.]

@ribrdb I appreciate the work you've put into this. I want to see something like this happen. I just want to make sure it's supported by rules_typescript folks. Could @alexeagle PTAL?

I'm reassigning to @dslomov who will be leading the Closure Rules project going forward. Dmitry has exciting visions going forward and he's one of the top leaders of the Bazel project as a whole. He was very kind in accepting these responsibilities. The fact that he was willing to do so, speaks highly of what we accomplished, and it demonstrates the level of commitment the Bazel team holds to the community.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@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

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

Successfully merging this pull request may close these issues.

4 participants