build: create sass_bundle bazel rule #10055

jelbourn · 2018-02-21T00:55:29Z

This defines a bazel rule to bundle sass partials into a single file
using the "sass-bundle" npm package. The rule definition is commented so
that it can serve as an example for creating other rules in the future.

I've also added the missing build file for badge so that its theme can
be included in the bundle.

I probably should have split this up into three PRs but, well, here we are.

cc fyi @devversion

alexeagle · 2018-02-21T01:50:53Z

src/lib/BUILD.bazel

+
+sass_bundle(
+  name = "theming_bundle",
+  srcs = [


introduce a constant in the top-level BUILD file?

ALL_LIBS=["autocomplete", "badge]
then here it's just
srcs = ["//src/lib/%s:%s_scss_partials" % l for l in ALL_LIBS]

Added a .bzl file at the top-level with a constant for this. I didn't think it was possible to import a constant from another BUILD file. Am I missing something?

alexeagle · 2018-02-21T01:51:24Z

src/lib/autocomplete/BUILD.bazel

@@ -21,6 +21,11 @@ ng_module(
  tsconfig = ":tsconfig-build.json",
 )

+# TODO(jelbourn): replace this w/ sass_library when it supports acting like a filegroup


we could make a simple PR to rules_sass to get this process started, at least test the waters to see if anyone is maintaining it, if they offer to hand over the keys?

I'll add it to my general "Making sass better" task list

alexeagle · 2018-02-21T01:52:19Z

src/lib/badge/BUILD.bazel

@@ -0,0 +1,22 @@
+package(default_visibility=["//visibility:public"])
+load("@angular//:index.bzl", "ng_module")
+load("@io_bazel_rules_sass//sass:sass.bzl", "sass_library", "sass_binary")


you might want to hook up skylint to tell you about unused imports and such

and buildifier too

One of the tasks I have tracked in #8369

alexeagle · 2018-02-21T01:53:51Z

tools/sass_bundle.ts

+  const parsedArgs: {srcs: string, output: string, entry: string} = minimist(args);
+  const inputFiles = parsedArgs.srcs.split(',');
+
+  return new Bundler().Bundle(join(workspaceRoot, parsedArgs.entry), inputFiles).then(result => {


did you determine if the CLI in scss-bundle is not suitable? if so, comment why in the fileoverview here

I couldn't get the nodejs_binary to work with the package's existing CLI and it was less work to keep this. It's also easier to debug the calls from bazel this way. Added a comment.

alexeagle · 2018-02-21T01:54:00Z

tools/sass_bundle.ts

@@ -0,0 +1,29 @@
+import {dirname, join} from 'path';


don't you need license headers in this repo?

As far as I knew we only needed license headers for code synced into google.

alexeagle · 2018-02-21T01:55:02Z

tools/sass_bundle.bzl

+    # rule (and will thus be available to the nodejs_binary in the sandbox).
+    "entry_point": attr.label(mandatory = True, allow_single_file = True),
+
+    # The executable (bundler) for this rule. The user will typically never


if you start with underscore, users won't be able to set it - I'd recommend keeping as much API private as possible until you understand why someone wants to set this

Done. (I didn't know you could do that)

alexeagle · 2018-02-21T01:56:51Z

tools/bazel-tools-tsconfig.json

@@ -0,0 +1,8 @@
+{


sadness, why another tsconfig file? the editor won't know about these settings.

The default bazel one doesn't have types: node and lib: es2015. I removed the other two options.

alexeagle · 2018-02-21T01:57:51Z

tools/bazel-tools-tsconfig.json

@@ -0,0 +1,8 @@
+{
+  "compilerOptions": {
+    "module": "commonjs",


this should give you a warning (if your rules_typescript is recent)
--module is controlled by Bazel

alexeagle · 2018-02-21T01:57:59Z

tools/bazel-tools-tsconfig.json

+{
+  "compilerOptions": {
+    "module": "commonjs",
+    "target": "es5",


mmalerba · 2018-02-21T03:58:29Z

src/cdk/a11y/BUILD.bazel

@@ -22,6 +17,17 @@ ng_module(
  tsconfig = ":tsconfig-build.json",
 )

+# TODO(jelbourn): remove this when sass_library acts like a filegroup
+filegroup(


Now that we're in the business of making our own bazel rules, should we just replace these two rules with something like material_sass_library? or maybe go even further and extract all of the rules shared by every component into some sort of material_module rule?

material_module( name = "button", module_name = "@angular/material/button", srcs = [...], deps = [...], scss = [...], theme_scss = [...], templates = [...] )

I'm planning on putting some time into either fixing or replacing the open source sass rules so that they match the ones in Google. These don't change often, so it shouldn't hurt our productivity to leave it until we can get the ecosystem sorted out.

Generally we should try to make the primitives work instead of wrapping them in macros, which are leaky abstractions.

alexeagle · 2018-02-23T17:55:02Z

BUILD.bazel

@@ -46,6 +46,43 @@ ANGULAR_TESTING = [
  "node_modules/@angular/platform-browser-dynamic/bundles/*.umd.js",
 ]

+MATERIAL_COMPONENTS = [


don't forget to remove this copy

This defines a bazel rule to bundle sass partials into a single file using the "scss-bundle" npm package. The rule definition is commented so that it can serve as an example for creating other rules in the future. I've also added the missing build file for badge so that its theme can be included in the bundle.

angular-automatic-lock-bot · 2019-09-08T15:02:52Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

jelbourn added pr: needs review labels Feb 21, 2018

jelbourn requested review from alexeagle and mmalerba February 21, 2018 00:55

jelbourn requested review from amcdnl, andrewseguin, crisbeto, devversion, josephperrott, kara and tinayuangao as code owners February 21, 2018 00:55

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 21, 2018

jelbourn force-pushed the bazel-sass-bundler branch from 07dcd6a to 95dce88 Compare February 21, 2018 00:57

alexeagle approved these changes Feb 21, 2018

View reviewed changes

jelbourn mentioned this pull request Feb 21, 2018

Switch build to bazel #8369

Closed

17 tasks

mmalerba reviewed Feb 21, 2018

View reviewed changes

alexeagle reviewed Feb 23, 2018

View reviewed changes

jelbourn force-pushed the bazel-sass-bundler branch from 7dffd4f to 3264ce9 Compare February 23, 2018 17:58

jelbourn added 3 commits March 12, 2018 10:06

Use constant for material packages

ec30cef

Additional comments

6e5a35c

jelbourn force-pushed the bazel-sass-bundler branch from 3264ce9 to f06c51c Compare March 12, 2018 17:07

Remove extra constant

f20aa37

jelbourn force-pushed the bazel-sass-bundler branch from f06c51c to f20aa37 Compare March 12, 2018 17:11

mmalerba approved these changes Mar 12, 2018

View reviewed changes

mmalerba added pr: lgtm and removed pr: needs review labels Mar 12, 2018

jelbourn merged commit fcc0908 into angular:master Mar 12, 2018

jelbourn deleted the bazel-sass-bundler branch April 2, 2018 22:31

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: create sass_bundle bazel rule #10055

build: create sass_bundle bazel rule #10055

jelbourn commented Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 21, 2018

jelbourn Feb 21, 2018

mmalerba Feb 21, 2018

jelbourn Feb 21, 2018

alexeagle Feb 23, 2018

alexeagle Feb 23, 2018

jelbourn Feb 23, 2018

angular-automatic-lock-bot bot commented Sep 8, 2019

build: create sass_bundle bazel rule #10055

build: create sass_bundle bazel rule #10055

Conversation

jelbourn commented Feb 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 8, 2019