-
Notifications
You must be signed in to change notification settings - Fork 25
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
test-helpers.js: unusual code is blocking closure-compiler improvement #85
Comments
This code is annotated to work with Closure Compiler (without externs). But it didn't work because the compiler didn't see the methods inside the IIFE. I changed that in #69, indeed the code looked a bit weird, but this seemed the easiest way to work around the compiler limitation. There's a requirement that makes this harder: we cannot add goog statements, like goog.scope, because this code also has to work without Closure. So that's why an IIFE with this layout was chosen. We started adding code that calls these methods. So if you roll this back, that code will break if the compiler again will not see the signatures of these methods. The code can be changed of course, in order not to break anything you'd have to make sure that Closure Compiler can see and validate the method signatures and it stays free of goog statemets. |
The version you have is still broken if it's ever executed in a strict environment, since I notice that the code is in fact using window.TestHelpers = {};
window.TestHelpers.flushAsynchronousObjects = ... |
If a method is defined on window, will the compiler still see that method as a global one? Also I think I forgot to mention that probably the main reason for an IIFE was that some methods are considered private. Note that I'm not part of the Polymer team, so whatever the plan, I won't be able to approve it. |
Thanks for the explanation. |
@fejesjoco |
This closure-compiler issue is related to the unusual way methods are being defined in test-helpers.js.
google/closure-compiler#2409
Could @fejesjoco or someone else help me understand the purpose of this arrangement?
I suspect it's trying to work around something for which there's a better solution.
The text was updated successfully, but these errors were encountered: