Skip to content
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

Add fine-grained Closure Library build rules #269

Merged
merged 1 commit into from
May 9, 2018

Conversation

jart
Copy link
Contributor

@jart jart commented May 9, 2018

Add fine-grained Closure Library build rules

This change introduced a fine-grained build graph for the Closure Library. This
is most useful when the Closure Compiler isn't being used. For example, it's
now possible to depend on @io_bazel_rules_closure//closure/library/object
which is 11mB smaller (in terms of naïvely concatenated JavaScript sources)
than getting the same API from @io_bazel_rules_closure//closure/library.
The latency of tools like Clutz should also be improved.

The following additional changes needed to be made:

  • deps.js no longer needs to be an implicit dependency thanks to
    transitionalforwarddeclarations.js.

  • A lenient attribute is now available for closure_js_library which makes
    the compiler more easy-going.

  • goog.labs, goog.ui, and third party APIs may no longer be exported from
    //closure/library and //closure/library:testing by default.

@jart jart requested a review from jameswex May 9, 2018 06:34
@jart jart force-pushed the thin-library branch 4 times, most recently from e611ef2 to fcd0685 Compare May 9, 2018 13:37
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# DO NOT EDIT -- bazel run //closure/library:regenerate -- "$PWD"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should auto-generated build files still have copyright header? the old build files did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUILD files shouldn't need a copyright header at all.

closure_js_library(
name = "announcer",
srcs = ["@com_google_javascript_closure_library//:closure/goog/a11y/aria/announcer.js"],
lenient = True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the strategy for what has lenient set to True vs what doesn't? the all_js don't seem to and the overall top-level build rule doesn't either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lenient and suppress should only be specified if srcs is nonempty. Failure condition added.

return '//%s:%s' % (os.path.dirname(path), file2name(path))


def main(basejs, outdir):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fn could use more comments throughout

@@ -60,7 +54,6 @@ def unfurl(deps, provider=""):

def collect_js(deps,
closure_library_base=None,
closure_library_deps=None,
has_direct_srcs=False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are the closure_library_deps no longer needed?

This change introduced a fine-grained build graph for the Closure Library. This
is most useful when the Closure Compiler isn't being used. For example, it's
now possible to depend on `@io_bazel_rules_closure//closure/library/object`
which is 11mB smaller (in terms of naïvely concatenated JavaScript sources)
than getting the same API from `@io_bazel_rules_closure//closure/library`.
The latency of tools like Clutz should also be improved.

The following additional changes needed to be made:

- `deps.js` no longer needs to be an implicit dependency thanks to
  `transitionalforwarddeclarations.js`.

- A `lenient` attribute is now available for `closure_js_library` which makes
  the compiler more easy-going.

- `goog.labs`, `goog.ui`, and third party APIs may no longer be exported from
  `//closure/library` and `//closure/library:testing` by default.
Copy link
Contributor Author

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fn could use more comments throughout

Done.

why are the closure_library_deps no longer needed?

See https://github.com/google/closure-library/blob/master/closure/goog/transitionalforwarddeclarations.js

# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# DO NOT EDIT -- bazel run //closure/library:regenerate -- "$PWD"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUILD files shouldn't need a copyright header at all.

closure_js_library(
name = "announcer",
srcs = ["@com_google_javascript_closure_library//:closure/goog/a11y/aria/announcer.js"],
lenient = True,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lenient and suppress should only be specified if srcs is nonempty. Failure condition added.

@jart
Copy link
Contributor Author

jart commented May 9, 2018

Wonderful. This change brings Travis latency down from 16 to 9 minutes. That's even better than it was twelve months ago. See: https://travis-ci.org/bazelbuild/rules_closure/builds cc: @hochhaus

@jart jart merged commit dea93c4 into bazelbuild:master May 9, 2018
@jart jart deleted the thin-library branch May 9, 2018 14:42
ptmphuong pushed a commit to ptmphuong/rules_closure that referenced this pull request Dec 9, 2022
This change introduced a fine-grained build graph for the Closure Library. This
is most useful when the Closure Compiler isn't being used. For example, it's
now possible to depend on `@io_bazel_rules_closure//closure/library/object`
which is 11mB smaller (in terms of naïvely concatenated JavaScript sources)
than getting the same API from `@io_bazel_rules_closure//closure/library`.
The latency of tools like Clutz should also be improved.

The following additional changes needed to be made:

- `deps.js` no longer needs to be an implicit dependency thanks to
  `transitionalforwarddeclarations.js`.

- A `lenient` attribute is now available for `closure_js_library` which makes
  the compiler more easy-going.

- `goog.labs`, `goog.ui`, and third party APIs may no longer be exported from
  `//closure/library` and `//closure/library:testing` by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants