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

interoperate with rules_closure #6

Closed
yohcop opened this issue Jun 8, 2017 · 20 comments
Closed

interoperate with rules_closure #6

yohcop opened this issue Jun 8, 2017 · 20 comments

Comments

@yohcop
Copy link

yohcop commented Jun 8, 2017

It would be nice to have a ts_binary rule that turns ts_libraries into a single .js file.
Unless I miss something and there's already a way to do this?

Maybe with bazelbuild/rules_closure, it would be possible to optimized the output code using closure_js_binary.

@alexeagle
Copy link
Contributor

right, we plan to re-use the closure compiler rules to get a single bundle file

@alexeagle alexeagle changed the title ts_binary (merging all deps in a single file) interoperate with rules_closure Jun 12, 2017
@yohcop
Copy link
Author

yohcop commented Jul 21, 2017

As a first approximation, would adding a outputFile option in a generated tsconfig file be possible?

@alexeagle
Copy link
Contributor

I think we more commonly want some other bundling tool that knows how to flatten modules, like rollup. @robwormald might look into it next week, angular and ngrx need it soon.

@achew22
Copy link
Member

achew22 commented Aug 7, 2017

Any luck on this? Feel free to ping me internally if there is any 20% stuff I can do to help 😄

@achew22
Copy link
Member

achew22 commented Sep 17, 2017

I have a semi-working prototype but it requires google/closure-compiler#2641 to be submitted (at a minimum) due to google/closure-compiler#2637 which lines up with the timeline that Alex was talking about with the es6 module import issue

@unlikely-leo
Copy link

Hi there, we're looking into using Bazel for our mixed TS/JS codebase at the moment. Interoperability with the Closure rules to build minified bundles is definitely a key aspect of making that work. It's great to see that this is being worked on!

@achew22 could you clarify what timelines you were referring to in your comment?

@achew22
Copy link
Member

achew22 commented Oct 19, 2017

@alexeagle looks like the snapshot build of Closure compiler now has the fix for deterministic ordering. Does that mean we can actually do this?

@alexeagle
Copy link
Contributor

Sure, you can pick up from https://github.com/alexeagle/rules_typescript/tree/closure_js_binary?files=1
And just point to the new snapshot build?

@alexeagle
Copy link
Contributor

I think closure compiler is released now, so someone could pick this up.

@codesuki
Copy link

codesuki commented Mar 5, 2018

@alexeagle is there any update on this or #112 ?
We want to use protobuf + grpc-web but the resulting js file using rollup are nearly 400kb.
I am expecting the size to go down using the closure compiler. Would be comfortable to use it together with rules_typescript.
Thanks for your work!

@alexeagle
Copy link
Contributor

alexeagle commented Mar 5, 2018 via email

@codesuki
Copy link

codesuki commented Mar 5, 2018

I am not sure yet. Still in the middle of investigating. My front end dev told me using grpc is unacceptable because the bundle size is around 400kb. We are using https://github.com/improbable-eng/grpc-web/ which has a dependency on google protobufjs, that's all I think.
I ran the closure compiler with extended flags on the rollup output and the file became 200kb, but I guess it's broken because I didn't have time to delve deep and set the right exports.

Anyway, if I may ask, are you moving away from closure internally? In another PR / Issue you mentioned you don't want to support any closure stuff because it's not much used.
I thought dead code elimination seemed like a nice feature, but I am not up to date with front end dev anymore.

Are you guys using grpc or protobuf from browsers? If so do you have much smaller file sizes?
That info would help me a lot 😓

I will try to make an example using rules_typescript with grpc-web and rollup.

edit:
If the readme of protobuf js is correct

The library is currently compatible with:
CommonJS-style imports (eg. var protos = require('my-protos');)
Closure-style imports (eg. goog.require('my.package.MyProto');)

That would mean that tree shaking is not supported, but maybe I am wrong.

@pcj
Copy link
Member

pcj commented Mar 6, 2018

For comparison, I'm using an internal grpc-web front-end implementation that's all closure. The whole app (with closure templates and all) minifies down to 154kb with advanced compile/no debug symbols. IIRC the grpc part alone was about 80kb, so it is possible to have reasonable bundle sizes with gRPC. Have not tried with rollup but seems like it should be possible to get close to that.

@codesuki
Copy link

codesuki commented Mar 6, 2018

@pcj thanks for providing that reference data! Very helpful 👍
I got it down to 80kb with gzipping it, but I am not sure that's a normal thing to do.

After doing some more investigation I found that the main culprits are the generated proto files.

app - 254.43 KB (47.08%)
	proto/event/client/event_pb.js - 39.53 KB (15.54%)
	proto/feature/user_pb.js - 39.25 KB (15.43%)
	proto/gateway/service_pb.js - 31.88 KB (12.53%)
	proto/feature/feature_pb.js - 26.21 KB (10.30%)
	proto/feature/strategy_pb.js - 24.77 KB (9.74%)
	proto/feature/evaluation_pb.js - 17.57 KB (6.91%)
	proto/feature/rule_pb.js - 8.33 KB (3.27%)
	proto/feature/clause_pb.js - 7.98 KB (3.14%)
	proto/feature/variation_pb.js - 7.35 KB (2.89%)
	proto/feature/target_pb.js - 6.39 KB (2.51%)

The next big thing is google-protobuf itself.

google-protobuf - 166.34 KB (30.78%)
buffer-es6 - 53.75 KB (9.95%)
grpc-web-client - 42.19 KB (7.81%)

Sorry this went totally off-topic. I know this is not the protobuf repo, but do you have any guidelines or best practices to keep the generated proto files small?

@schmitch
Copy link

@alexeagle is there any progress on this?

@alexeagle
Copy link
Contributor

alexeagle commented Mar 22, 2018 via email

@ribrdb
Copy link

ribrdb commented Mar 22, 2018

@schmitch If you want to compile a ts library down to a .js file using the closure compiler, you can try
https://github.com/ribrdb/rules_closure/tree/ts
With that you can add a ts_library in the deps of any closure_js_binary()
You may also need to use https://github.com/ribrdb/rules_typescript/tree/closure to get the typescript to run through tsickle before compiling it with closure.
I thought I sent a pull request for that, but it looks like I never did.

@alexeagle
Copy link
Contributor

alexeagle commented Mar 22, 2018 via email

@kalbasit
Copy link
Contributor

kalbasit commented Jun 19, 2018

@alexeagle is there any examples of using closure_js_binary with a ts_library? Do we need #174 for this to work? I can't seem to get it to work.

EDIT: I got it to work, see #174

@alexeagle
Copy link
Contributor

#344 has more recent discussion so let's continue there

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

No branches or pull requests

9 participants