Skip to content

Commit

Permalink
Replaces @bazel/postcss usage with lightningcss.
Browse files Browse the repository at this point in the history
Refs #46.

This adds a new `css_bundle()` target which calls the `css_bundler` binary (using `lightningcss`) to bundle all the given CSS files. This rule is then swapped in for `postcss_multi_binary()` for use across the whole project.

This necessitated updating `css_library()` to copy all its sources to the bin directory to be compatible with the `css_bundler` `js_binary()` target downstream. I would have liked to land this in a separate commit, but it actually breaks the `@bazel/postcss` pipeline since that relies on the bundled file having the same name as its entry point.

Partially for this reason, I ended up changing the output file paths of the bundled CSS files. Instead of generating the bundled CSS to use the same basename, I rerooted the whole execroot under the target. So if you bundle a file at `@some_wksp//path/to:file.css` at `@some_other_wksp//path/to/pkg:bundle` it will output the bundled version at:

```
.../execroot/some_other_wksp/path/to/pkg/bundle/some_wksp/path/to/file.css
```

Including the workspacename and putting everything under the bundle target means every input file as a 1:1 correlation with its output file, and there is no potential ambiguity or conflict.
  • Loading branch information
dgp1130 committed Feb 18, 2023
1 parent 930981a commit c0491dd
Show file tree
Hide file tree
Showing 21 changed files with 161 additions and 89 deletions.
2 changes: 1 addition & 1 deletion examples/site/components/base/base.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'rules_prerender/examples/site/common/theme.css';
@import '../../common/theme.css';

html[comp-base], [comp-base] body {
margin: 0;
Expand Down
2 changes: 1 addition & 1 deletion examples/site/components/footer/footer.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'rules_prerender/examples/site/common/theme.css';
@import '../../common/theme.css';

:host {
background-color: var(--primary-color);
Expand Down
2 changes: 1 addition & 1 deletion examples/site/components/header/header.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'rules_prerender/examples/site/common/theme.css';
@import '../../common/theme.css';

:host {
background-color: var(--primary-color);
Expand Down
2 changes: 1 addition & 1 deletion examples/styles/styles.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import url("rules_prerender/examples/styles/dep.css");
@import './dep.css';

div {
color: var(--favorite-color);
Expand Down
7 changes: 7 additions & 0 deletions packages/css_bundler/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
load("@aspect_rules_js//js:defs.bzl", "js_binary")
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("//tools:typescript.bzl", "ts_project")
load("//tools:jasmine.bzl", "jasmine_node_test")

bzl_library(
name = "css_bundle",
srcs = ["css_bundle.bzl"],
deps = ["//packages/rules_prerender/css:css_providers"],
)

js_binary(
name = "css_bundler",
entry_point = "css_bundler.js",
Expand Down
65 changes: 65 additions & 0 deletions packages/css_bundler/css_bundle.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
load("@aspect_bazel_lib//lib:paths.bzl", "to_output_relative_path")
load("//packages/rules_prerender/css:css_providers.bzl", "CssInfo")

def _css_bundle_impl(ctx):
# Map each direct dependency source file to an output of the same full file path
# under this target's name. Ideally we would just keep the name the same, but then
# it would conflict with the source file (since that needs to be copied to bin to
# be usable anyways). So instead we "namespace" each file under the target name
# and include the full execroot-relative path to file to be truly unambiguous.
# The output file structure will look like:
# path/to/pkg/css_bundle_target/file_wksp_name/path/to/file.css
sources = ctx.attr.dep[CssInfo].direct_sources
outputs = [ctx.actions.declare_file("%s/%s" % (
ctx.label.name,
_workspace_short_path(src, ctx.label.workspace_name or ctx.workspace_name),
)) for src in sources]

# Map each entry point to its associated output.
args = ctx.actions.args()
for (source, output) in zip(sources, outputs):
args.add("--entry-point", "./%s" % to_output_relative_path(source))
args.add("--output", to_output_relative_path(output))

ctx.actions.run(
mnemonic = "BundleCss",
progress_message = "Bundling styles %{label}",
executable = ctx.executable._bundler,
arguments = [args],
inputs = ctx.attr.dep[CssInfo].transitive_sources,
outputs = outputs,
env = {
"BAZEL_BINDIR": ctx.bin_dir.path,
},
)

return DefaultInfo(files = depset(outputs))

css_bundle = rule(
implementation = _css_bundle_impl,
attrs = {
"dep": attr.label(
mandatory = True,
providers = [CssInfo],
doc = "The `css_library()` target to bundle all its direct sources.",
),
"_bundler": attr.label(
default = "//packages/css_bundler",
executable = True,
cfg = "exec",
),
},
doc = """
Bundles the direct sources of the given dependency by treating each file as its
own entry point and resolving `@import` statements to inline the dependencies.
""",
)

def _workspace_short_path(file, current_wksp):
if file.short_path.startswith("../"):
# File is in an external workspace with path: `../external_wksp/path/to/file`.
# Just drop the `../` and we have the right path.
return file.short_path[len("../"):]
else:
# File is in the main workspace. Use the current workspace name.
return "%s/%s" % (current_wksp, file.short_path)
5 changes: 4 additions & 1 deletion packages/rules_prerender/css/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,8 @@ bzl_library(
bzl_library(
name = "css_providers",
srcs = ["css_providers.bzl"],
visibility = ["//packages/rules_prerender:__pkg__"],
visibility = [
"//packages/css_bundler:__subpackages__",
"//packages/rules_prerender:__pkg__",
],
)
45 changes: 4 additions & 41 deletions packages/rules_prerender/css/css_binaries.bzl
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
"""Defines `css_binaries()` and related rules."""

load("@npm//@bazel/postcss:index.bzl", "postcss_multi_binary")
load(
"//packages/rules_prerender:postcss_import_plugin.bzl",
IMPORT_PLUGIN_CONFIG = "PLUGIN_CONFIG",
)
load("//packages/css_bundler:css_bundle.bzl", "css_bundle")
load(":css_group.bzl", "css_group")
load(":css_map.bzl", "css_map")
load(":css_providers.bzl", "CssInfo")
Expand Down Expand Up @@ -88,33 +84,16 @@ def _css_binary(
visibility: https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes
tags: https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes
"""
# Collect all transitive CSS dependencies into a single `DefaultInfo` target.
css_transitive_deps = "%s_transitive_deps" % name
_css_deps(
name = css_transitive_deps,
dep = dep,
testonly = testonly,
tags = tags,
)

# Compile the CSS. This only compiles one library, but that library may have many
# source files, each one should be compiled into its own output, which requires
# `postcss_multi_binary()` even though `postcss_binary()` sounds like it would be
# sufficient here.
binary = "%s_postcss" % name
postcss_multi_binary(
binary = "%s_parcel" % name
css_bundle(
name = binary,
srcs = [dep],
output_pattern = "{name}",
# Sourcemaps must be turned off because they don't work on Node.js versions v14+.
# https://github.com/bazelbuild/rules_postcss/issues/73/
sourcemap = False,
plugins = {
Label("//tools/internal:postcss_import_plugin"): IMPORT_PLUGIN_CONFIG,
},
dep = dep,
testonly = testonly,
tags = tags,
deps = [":%s" % css_transitive_deps],
)

# Return the binary outputs with a map of import name -> file path.
Expand All @@ -126,19 +105,3 @@ def _css_binary(
visibility = visibility,
tags = tags,
)

def _css_deps_impl(ctx):
return DefaultInfo(
files = ctx.attr.dep[CssInfo].transitive_sources,
)

_css_deps = rule(
implementation = _css_deps_impl,
attrs = {
"dep": attr.label(
mandatory = True,
providers = [CssInfo],
),
},
doc = "Returns transitive CSS sources in `DefaultInfo`.",
)
11 changes: 8 additions & 3 deletions packages/rules_prerender/css/css_library.bzl
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
"""Defines the `css_library()` rule."""

load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_files_to_bin_actions")
load(":css_providers.bzl", "CssInfo")

def _css_library_impl(ctx):
# Copy sources to bin so they are always available for downstream `js_binary()`
# tools.
copied = copy_files_to_bin_actions(ctx, ctx.files.srcs)

return [
DefaultInfo(files = depset(ctx.files.srcs)),
DefaultInfo(files = depset(copied)),
CssInfo(
direct_sources = ctx.files.srcs,
transitive_sources = depset(ctx.files.srcs,
direct_sources = copied,
transitive_sources = depset(copied,
transitive = [dep[CssInfo].transitive_sources
for dep in ctx.attr.deps],
),
Expand Down
25 changes: 25 additions & 0 deletions packages/rules_prerender/css/tests/bundle/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
load("@aspect_bazel_lib//lib:diff_test.bzl", "diff_test")
load("//packages/css_bundler:css_bundle.bzl", "css_bundle")
load("//packages/rules_prerender/css:css_library.bzl", "css_library")

css_bundle(
name = "bundle",
dep = ":lib",
)

css_library(
name = "lib",
srcs = ["lib.css"],
deps = [":transitive"],
)

css_library(
name = "transitive",
srcs = ["transitive.css"],
)

diff_test(
name = "test",
file1 = ":bundle",
file2 = "expected.css",
)
7 changes: 7 additions & 0 deletions packages/rules_prerender/css/tests/bundle/expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.transitive {
color: green;
}

.lib {
color: red;
}
5 changes: 5 additions & 0 deletions packages/rules_prerender/css/tests/bundle/lib.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import './transitive.css';

.lib {
color: red;
}
3 changes: 3 additions & 0 deletions packages/rules_prerender/css/tests/bundle/transitive.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.transitive {
color: green;
}
2 changes: 1 addition & 1 deletion packages/rules_prerender/css/tests/dependencies/bar.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import url("rules_prerender/packages/rules_prerender/css/tests/dependencies/baz.css");
@import './baz.css';

.bar {
color: green;
Expand Down
6 changes: 3 additions & 3 deletions packages/rules_prerender/css/tests/dependencies/expected.css
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
.baz {
color: blue;
color: #00f;
}

.bar {
color: green;
color: green;
}

.foo {
color: red;
color: red;
}
2 changes: 1 addition & 1 deletion packages/rules_prerender/css/tests/dependencies/foo.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import url("rules_prerender/packages/rules_prerender/css/tests/dependencies/bar.css");
@import './bar.css';

.foo {
color: red;
Expand Down
32 changes: 18 additions & 14 deletions packages/rules_prerender/css/tests/group/group_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ def _css_group_test_impl(ctx):
# Verify that `css_group()` re-exports `DefaultInfo`.
default_info = analysistest.target_under_test(env)[DefaultInfo]
expected_files = sets.make([
"packages/rules_prerender/css/tests/group/lib1.css",
"packages/rules_prerender/css/tests/group/lib2.css",
"packages/rules_prerender/css/tests/group/bin1_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib1.css",
"packages/rules_prerender/css/tests/group/bin2_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib2.css",
])
actual_files = sets.make([file.short_path for file in default_info.files.to_list()])
asserts.new_set_equals(env, expected_files, actual_files)
Expand All @@ -22,11 +22,11 @@ def _css_group_test_impl(ctx):
import_map = css_import_map.import_map
asserts.equals(env, 2, len(import_map.items()))

expected_lib1 = "packages/rules_prerender/css/tests/group/lib1.css"
expected_lib1 = "packages/rules_prerender/css/tests/group/bin1_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib1.css"
actual_lib1 = import_map["rules_prerender/packages/rules_prerender/css/tests/group/lib1.css"].short_path
asserts.equals(env, expected_lib1, actual_lib1)

expected_lib2 = "packages/rules_prerender/css/tests/group/lib2.css"
expected_lib2 = "packages/rules_prerender/css/tests/group/bin2_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib2.css"
actual_lib2 = import_map["rules_prerender/packages/rules_prerender/css/tests/group/lib2.css"].short_path
asserts.equals(env, expected_lib2, actual_lib2)

Expand Down Expand Up @@ -80,9 +80,9 @@ def _css_group_fails_with_conflicting_maps_test_impl(ctx):
"rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")
# Look for conflicting file paths.
asserts.expect_failure(env,
"packages/rules_prerender/css/tests/group/conflicting_lib.css")
"packages/rules_prerender/css/tests/group/conflicting_bin1_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")
asserts.expect_failure(env,
"packages/rules_prerender/css/tests/group/subpackage/conflicting_lib.css")
"packages/rules_prerender/css/tests/group/conflicting_bin2_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")

return analysistest.end(env)

Expand All @@ -96,24 +96,28 @@ def _test_css_group_fails_with_conflicting_maps(name):
name = "conflicting_lib",
srcs = ["conflicting_lib.css"],
tags = ["manual"],
visibility = ["//packages/rules_prerender/css/tests/group/subpackage:__pkg__"],
)

# Generates an import map which points `:conflicting_lib` sources to CSS files
# generated in *this* package (`group`).
# Generates conflicting import maps which points `conflicting_lib.css` to both
# bundled CSS files under `conflicting_bin1/` _and_ `conflicting_bin2/`.
css_binaries(
name = "conflicting_bin",
name = "conflicting_bin1",
deps = [":conflicting_lib"],
tags = ["manual"],
)
css_binaries(
name = "conflicting_bin2",
deps = [":conflicting_lib"],
tags = ["manual"],
)

# Should try and fail to merge import maps because they both map `:conflicting_lib`
# sources to CSS files in different packages (`group` and `subpackage`).
# Should try and fail to merge import maps because they both map
# `conflicting_lib.css` to two different locations.
css_group(
name = "conflicting_group",
deps = [
":conflicting_bin",
"//packages/rules_prerender/css/tests/group/subpackage:bin",
":conflicting_bin1",
":conflicting_bin2",
],
tags = ["manual"],
)
Expand Down
13 changes: 0 additions & 13 deletions packages/rules_prerender/css/tests/group/subpackage/BUILD.bazel

This file was deleted.

3 changes: 0 additions & 3 deletions packages/rules_prerender/css/tests/group/subpackage/lib.css

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ def _import_map_test_impl(ctx):
import_map = css_import_map.import_map
asserts.equals(env, 2, len(import_map.items()))

expected_foo = "packages/rules_prerender/css/tests/import_map/foo.css"
expected_foo = "packages/rules_prerender/css/tests/import_map/bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/foo.css"
actual_foo = import_map["rules_prerender/packages/rules_prerender/css/tests/import_map/foo.css"].short_path
asserts.equals(env, expected_foo, actual_foo)

expected_bar = "packages/rules_prerender/css/tests/import_map/bar.css"
expected_bar = "packages/rules_prerender/css/tests/import_map/bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/bar.css"
actual_bar = import_map["rules_prerender/packages/rules_prerender/css/tests/import_map/bar.css"].short_path
asserts.equals(env, expected_bar, actual_bar)

Expand Down Expand Up @@ -49,7 +49,7 @@ def _import_map_from_different_packages_test_impl(ctx):

# Import path contains `subpackage` because that's what the user would import.
# But the actual file it points to uses the same name in the `css_binaries()` package.
expected_lib = "packages/rules_prerender/css/tests/import_map/lib.css"
expected_lib = "packages/rules_prerender/css/tests/import_map/different_packages_bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/subpackage/lib.css"
actual_lib = import_map["rules_prerender/packages/rules_prerender/css/tests/import_map/subpackage/lib.css"].short_path
asserts.equals(env, expected_lib, actual_lib)

Expand Down Expand Up @@ -81,4 +81,3 @@ def import_map_test_suite(name):
":import_map_from_different_packages_test",
],
)

Loading

0 comments on commit c0491dd

Please sign in to comment.