-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Change comments for examples_star_json build target #2825
Conversation
OK, I must admit this is waaaaay better! Very clear and complete. I also prefer inserting the comment not within the configuration itself. I should really learn not to document that briefly ;-) |
Change comments for examples_star_json build target
# oli.js and olx.js do not provide or require namespaces (using | ||
# "goog.provide" or "goog.require"). For that reason, if they are | ||
# specified as input files through the "src" property, then | ||
# closure-util will exclude them when creating the dependencies graph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to say "include" instead of "exclude"?
It is our own build.js
task that handles the src
property, passing the pattern to closure-util
as "library files" (yes, I admit taking src
and setting lib
adds confusion - we can have a separate discussion about lib
and main
and decide if we want to expose the same in our build config syntax, but for now the src
pattern becomes lib
). Library files are parsed for goog.provide
and goog.require
, dependencies are ordered, and the result is concatenated with any js
files before calling the compiler. So providing js
files directly in a build config bypasses closure-util
s dependency resolution step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did mean "exclude". Do you see anything incorrect in my comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc says
if they are specified as input files through the "src" property, then closure-util will
exclude them when creating the dependencies graph.
When path patterns are provided in the src
, our build.js
task includes them as lib
files for closure-util
. All lib
files are considered (or you could say included) in resolving dependencies.
By contrast, when you provide paths to the js
property of the compile
config object, these paths are not passed to closure-util
(here you might say that paths are excluded from the dependency resolution work that closure-util
does).
The dependencies resolved based on src
files are concatenated together with any js
paths before being passed to the compiler.
So yes, your comment sounds incorrect to me (including src
paths does not have to do with excluding anything). Maybe I don't get what you mean by exclude.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You quoted only the end of the doc sentence. The first part, and the previous sentence are important.
oli.js
and olx.js
do not use goog.provide
or goog.require
so closure-util
will not include these files as a result of resolving dependencies. This is the reason we need to bypass closure-util
, and use the "js"
compile property instead (with manage_closure_dependencies
).
Hope it makes sense this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tschaub, does it make sense? Is my understanding of closure-util correct?
@GingerIK, what do you think?