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

feat(bazel): Add support for SASS #28167

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+67 −6
Diff settings

Always

Just for now

@@ -41,15 +41,15 @@ http_archive(
url = "https://registry.yarnpkg.com/rxjs/-/rxjs-%s.tgz" % RXJS_VERSION,
strip_prefix = "package/src",
)

<% if (sass) { %>
# Rules for compiling sass
RULES_SASS_VERSION = "<%= RULES_SASS_VERSION %>"
http_archive(
name = "io_bazel_rules_sass",
url = "https://github.com/bazelbuild/rules_sass/archive/%s.zip" % RULES_SASS_VERSION,
strip_prefix = "rules_sass-%s" % RULES_SASS_VERSION,
)

<% } %>
####################################
# Load and install our dependencies downloaded above.

@@ -85,9 +85,9 @@ browser_repositories(

load("@build_bazel_rules_typescript//:defs.bzl", "ts_setup_workspace")
ts_setup_workspace()

<% if (sass) { %>
load("@io_bazel_rules_sass//sass:sass_repositories.bzl", "sass_repositories")
sass_repositories()

<% } %>
load("@angular//:index.bzl", "ng_setup_workspace")
ng_setup_workspace()
@@ -5,7 +5,17 @@ load("@build_bazel_rules_typescript//:defs.bzl", "ts_library", "ts_web_test_suit
load("@build_bazel_rules_nodejs//:defs.bzl", "rollup_bundle", "history_server")
load("@build_bazel_rules_nodejs//internal/web_package:web_package.bzl", "web_package")
load("@build_bazel_rules_typescript//:defs.bzl", "ts_devserver")
<% if (sass) { %>load("@io_bazel_rules_sass//:defs.bzl", "sass_binary")

[
sass_binary(
name = "style_" + x,
src = x,
deps = [],
)
for x in glob(["**/*.scss"])
]
<% } %>
ng_module(
name = "src",
srcs = glob(
@@ -20,7 +30,7 @@ ng_module(
assets = glob([
"**/*.css",
"**/*.html",
]),
])<% if (sass) { %> + [":style_" + x for x in glob(["**/*.scss"])]<% } %>,
deps = [
"@angular//packages/core",
"@angular//packages/platform-browser",<% if (routing) { %>
@@ -60,6 +60,16 @@ function hasRoutingModule(host: Tree) {
return hasRouting;
}

/**
* Returns true if project uses SASS stylesheets, false otherwise.
*/
function hasSassStylesheet(host: Tree) {
let hasSass = false;
// The proper extension for SASS is .scss
host.visit((file: string) => { hasSass = hasSass || file.endsWith('.scss'); });
return hasSass;
}

export default function(options: BazelWorkspaceOptions): Rule {
return (host: Tree, context: SchematicContext) => {
if (!options.name) {
@@ -103,6 +113,7 @@ export default function(options: BazelWorkspaceOptions): Rule {
...options,
'dot': '.', ...workspaceVersions,
routing: hasRoutingModule(host),
sass: hasSassStylesheet(host),
}),
move(appDir),
]));
@@ -12,7 +12,7 @@ import {clean} from './index';

describe('Bazel-workspace Schematic', () => {
const schematicRunner =
new SchematicTestRunner('@angular/bazel', require.resolve('../collection.json'), );
new SchematicTestRunner('@angular/bazel', require.resolve('../collection.json'));
const defaultOptions = {
name: 'demo',
};
@@ -79,6 +79,46 @@ describe('Bazel-workspace Schematic', () => {
expect(content).toContain('workspace(name = "demo_project"');
});
});

describe('SASS', () => {
let host = new UnitTestTree(new HostTree);
beforeAll(() => {
host.create('/demo/src/app/app.component.scss', '');
expect(host.files).toContain('/demo/src/app/app.component.scss');
const options = {...defaultOptions};
host = schematicRunner.runSchematic('bazel-workspace', options, host);
expect(host.files).toContain('/demo/WORKSPACE');
expect(host.files).toContain('/demo/src/BUILD.bazel');
});

it('should download rules_sass in WORKSPACE', () => {
const content = host.readContent('/demo/WORKSPACE');
expect(content).toContain('RULES_SASS_VERSION');
expect(content).toContain('io_bazel_rules_sass');
});

it('should load sass_repositories in WORKSPACE', () => {
const content = host.readContent('/demo/WORKSPACE');
expect(content).toContain(
'load("@io_bazel_rules_sass//sass:sass_repositories.bzl", "sass_repositories")');
expect(content).toContain('sass_repositories()');
});

it('should add sass_binary rules in src/BUILD', () => {
const content = host.readContent('/demo/src/BUILD.bazel');
expect(content).toContain('load("@io_bazel_rules_sass//:defs.bzl", "sass_binary")');
expect(content).toMatch(/sass_binary\((.*\n)+\)/);
});

it('should add SASS targets to assets of ng_module in src/BUILD', () => {
const content = host.readContent('/demo/src/BUILD.bazel');
expect(content).toContain(`
assets = glob([
"**/*.css",
"**/*.html",
]) + [":style_" + x for x in glob(["**/*.scss"])],`);

This comment has been minimized.

Copy link
@alexeagle

alexeagle Jan 16, 2019

Contributor

nit: this assertion is too brittle, or the test is too fake.

If you just want to check that the conditional logic for turning on and including the sass content is working, it's sufficient to just assert that it contains **/*.scss
If you want to check that this list comprehension works, then the test needs to at least run a bazel query on the file so that we evaluate the expression

the way it is, many changes to the production code will require test maintenance just to copy across the new string, and the test doesn't check if those changes are correct.

});
});
});

describe('clean', () => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.