Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Distribute closure externs for Zone #727

Closed
alexeagle opened this issue Apr 7, 2017 · 9 comments
Closed

Distribute closure externs for Zone #727

alexeagle opened this issue Apr 7, 2017 · 9 comments

Comments

@alexeagle
Copy link
Contributor

Since Zone.js is a "prollyfill" and since multiple angular apps might be on a single page, we don't want to bundle it together with the app.
This means closure compiler needs an externs file to avoid renaming Zone's symbols.
I currently have such an externs here:
https://github.com/alexeagle/closure-compiler-angular-bundling/blob/master/vendor/zone_externs.js
but it should be distributed with Zone.js so that users don't have to fetch it themselves.

@JiaLiPassion
Copy link
Collaborator

@alexeagle , I would like to make this file, and add test cases to make sure it is ok through optimized: advanced for closurescript compiler. But I am totally new to closurescript, so please help me to review my work after I made the PR.

@alexeagle
Copy link
Contributor Author

Hi!

A bit of context. Ideally this extern file should be generated from the TS sources, by github.com/angular/tsickle - this is what we do internally at Google. But I think this is probably out-of-scope for this issue; we can hand-edit the externs file for now. @mhevery might have some opinion about whether the API is stable enough that we'll rarely have to update the externs.

Testing is the right thing to do. Here's where the material2 project added a test, but it only verifies that compilation succeeds, not that the application can access the library APIs at runtime:
angular/components@51cfb5f
We also need some test that fails if one of the externs is removed from the file.

(by the way, closurescript isn't a thing - there is clojurescript which is actually a different language, that confusingly produces code that can be consumed by closure compiler. closure != clojure)

@JiaLiPassion
Copy link
Collaborator

@alexeagle , Thank you so much for all those information, I will try to understand and build the externs file by hand.

(by the way, closurescript isn't a thing - there is clojurescript which is actually a different language, that confusingly produces code that can be consumed by closure compiler. closure != clojure)

Thank you for pointing that out!

@alexeagle
Copy link
Contributor Author

alexeagle commented Apr 9, 2017 via email

@JiaLiPassion
Copy link
Collaborator

@alexeagle , got it, I will just build the test cases and let you review later, thank you!

@mhevery
Copy link
Contributor

mhevery commented Apr 10, 2017

I think hand coded externs file is fine for now.

@JiaLiPassion
Copy link
Collaborator

@mhevery ,got it,I have made a new one based on @alexeagle 's version, I will make a PR to let you review later.

@mhevery
Copy link
Contributor

mhevery commented Apr 10, 2017 via email

JiaLiPassion added a commit to JiaLiPassion/zone.js that referenced this issue Apr 11, 2017
@JiaLiPassion
Copy link
Collaborator

JiaLiPassion commented Apr 11, 2017

@alexeagle , @mhevery , I have made a PR #731, please review.

  1. I add more type definition for all Zone type
  • add new API, such as Zone.root, Task.cancelScheduleRequest
  • based on @alexeagle 's version, add more type definition for all function's parameters, return values, properties.
  1. add test cases for closure build, and make a test program to run and make sure all properties/functions' names of Zone are keep after closure compiler compile.

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

No branches or pull requests

3 participants