-
Notifications
You must be signed in to change notification settings - Fork 115
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
Warnings about missing types from compiler #27
Comments
Not following. Please show |
closure-compile-warnings.zip Run: And you will get the following warning (among other bogus warnings that are from angular-externs.js):
angular.Module is defined as a struct in angular-externs.js, however. |
I suspect this could be resolved by adding goog.provide(..) in the extern file and goog.require(...) in the code file. |
Does the problem go away if you change: closure_js_library(
name = "app",
srcs = ["app.js"],
deps = ["@io_bazel_rules_closure//closure/library"],
)
closure_js_library(
name = "ext",
externs = ["angular-externs.js"],
)
closure_js_binary(
name = "app_bin",
deps = [
":app",
":ext",
],
main = "foo.app",
defs = [
"--angular_pass",
"--export_local_property_definitions",
],
) To be: closure_js_library(
name = "app",
srcs = ["app.js"],
deps = [
":ext",
"@io_bazel_rules_closure//closure/library",
],
)
closure_js_library(
name = "ext",
externs = ["angular-externs.js"],
)
closure_js_binary(
name = "app_bin",
deps = [":app"],
main = "foo.app",
defs = [
"--angular_pass",
"--export_local_property_definitions",
],
) That should accurately depict the dependency graph. |
Also you can't put goog.provide statements in externs files. They aren't executed. It's purely declarative code. |
Also can you use |
Adding the ":ext" dependency to app library did not make the warning go away. From running the build with -s, I saw |
I'm not sure if this is a bug in Closure Rules. I'm not sure what's going on, but it seems like it might be something that should be escalated to https://github.com/google/closure-compiler if it continues to present problems. |
I'm not sure either. When I ran the closure compiler in a standalone fashion, the warnings weren't there, but it's possible that either the usage or the version of the compilers is different in each case. When I have time I'll see if I can reproduce the problem with the standalone compiler, and reopen if necessary. |
Thank you. |
@wolfgangmeyers, @jart , I'm getting the same behavior mentioned in this bug. I downloaded Wolfang's code and started playing around with the closure compiler command line and I if you add "--hide_warnings_for=angular-externs.js" you significantly reduce the noise.
Which means that the underlying issue is still there but this could be a temporary workaround to reduce the output noise. It should be as easy as adding the following to your closure_js_binary:
|
@aproxs Does your http_file(
name = "angular_externs",
url = "https://raw.githubusercontent.com/google/closure-compiler/maven-v20160517/contrib/externs/angular-1.5.js",
sha256 = "c7fcda4d045e92c7d1624a98624429af93d54bd0ef7182d1093ec1bdca0d5b77",
)
closure_js_library(
name = "angular",
externs = ["@angular_externs//file"],
)
closure_js_library(
name = "foo_lib",
srcs = ["foo.js"],
deps = [":angular"],
) That should fix the error about the |
Thanks @jart, I tried again making sure that all the files were there and the warning disappears. However, adding angular-1.5.js to the externs seems to add around 90 warnings to my output, all coming from the angular extern, so seems that I still need the hide_warnings_for in the command line options to exclude warnings from externs. I'm guessing that these are things that need to be fixed in the closure_compiler repo right? |
@aproxs What are those warnings? Could you copy and paste them here? Looking at angular-1.5.js it doesn't appear to be poorly written. |
I agree, the angular-1.5* set seems pretty nice at first look. Here is the output that I get. I think you can easily repro this with the files Wolfang shared earlier in this issue. Again, from the text in the warnings seems like this could be legit issues in the angular-1.5* family of files.
|
Oh dear, the way they're using |
It should relatively simple for you to delete all those superfluous method definitions, and then send a pull request to google/closure-compiler. |
Out of curiosity, did JsChecker catch this problem in the |
Hmmm, seems that the library generates a different set of warnings than the binary, which I just found trying to confirm if JsChecker catches this. From what I can tell, the library generates a much 'richer' set of warnings and errors than the binary and I am suspicious that the JsChecker and/or linter are not running for the binary in my project. Here is a simplified version of my BUILD file:
With that build file, I get no errors or warnings when I do:
but I get legit errors on missing dependencies and other real things when I build the library in isolation:
Is there anything missing in my BUILD file that could be causing the difference in behavior? Let me fix the errors in my code and try to answer your question when I get a successful build. |
Yeah the reason JsChecker didn't get invoked is due to a bug I fixed in c810b60 Before this, it would only get invoked if you ran |
Just tried with 0.2.3 and the JsChecker is now running consistently. Thanks @jart! |
I'm glad I could help. Thank you for using Closure Rules. |
Types that are defined in extern files show up as undefined types in the compiler output. These are just warnings, and don't prevent the output of the closure_js_binary. I'll provide a reproduction scenario when I have time, logging this now before I forget.
The text was updated successfully, but these errors were encountered: