Skip to content

Commit

Permalink
Removes css_map() and inlines its implementation in css_bundle().
Browse files Browse the repository at this point in the history
Refs #46.

The import map is only ever generated as part of `css_bundle()`. It needed to be a different rule before because `postcss_multi_binary()` was a macro and needed to be called at that level. But now that `css_bundle()` is a custom rule, we can generate the import map directly, which is much more straightforward.
  • Loading branch information
dgp1130 committed Feb 18, 2023
1 parent c0491dd commit 3cc94b6
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 159 deletions.
29 changes: 27 additions & 2 deletions packages/css_bundler/css_bundle.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
load("@aspect_bazel_lib//lib:paths.bzl", "to_output_relative_path")
load("//packages/rules_prerender/css:css_providers.bzl", "CssInfo")
load("//packages/rules_prerender/css:css_providers.bzl", "CssImportMapInfo", "CssInfo")

def _css_bundle_impl(ctx):
# Map each direct dependency source file to an output of the same full file path
Expand Down Expand Up @@ -33,7 +33,10 @@ def _css_bundle_impl(ctx):
},
)

return DefaultInfo(files = depset(outputs))
return [
DefaultInfo(files = depset(outputs)),
CssImportMapInfo(import_map = _make_import_map(ctx, sources, outputs)),
]

css_bundle = rule(
implementation = _css_bundle_impl,
Expand Down Expand Up @@ -63,3 +66,25 @@ def _workspace_short_path(file, current_wksp):
else:
# File is in the main workspace. Use the current workspace name.
return "%s/%s" % (current_wksp, file.short_path)

def _make_import_map(ctx, sources, outputs):
"""Generates a map of import paths to the actual file location to import.
When users call `inlineStyle()`, it resolves and looks up that path in this map
and actually inlines the file specified by the map. This allows the file path in
user code to differ from the actual path the bundled CSS file lives at.
"""
import_map = dict()
for (source, output) in zip(sources, outputs):
wksp_path = _workspace_short_path(source, ctx.workspace_name)

if wksp_path in import_map:
fail("CSS library file (%s) mapped twice, once to %s and a second time to %s." % (
wksp_path,
import_map[wksp_path].path,
output.path,
))

import_map[wksp_path] = output

return import_map
7 changes: 0 additions & 7 deletions packages/rules_prerender/css/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ bzl_library(
visibility = ["//packages/rules_prerender:__pkg__"],
deps = [
":css_group",
":css_map",
":css_providers",
"//packages/rules_prerender:postcss_import_plugin",
],
Expand All @@ -36,12 +35,6 @@ bzl_library(
deps = [":css_providers"],
)

bzl_library(
name = "css_map",
srcs = ["css_map.bzl"],
deps = [":css_providers"],
)

bzl_library(
name = "css_providers",
srcs = ["css_providers.bzl"],
Expand Down
71 changes: 11 additions & 60 deletions packages/rules_prerender/css/css_binaries.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

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")

def css_binaries(
Expand All @@ -29,10 +28,20 @@ def css_binaries(
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
"""
# It might look like this implementation should accept multiple `css_library()`
# targets, but doing so would be a bad idea. This is because the library is separated
# into "direct sources" and "transitive dependencies" which are fed into
# `css_bundle()`. If this allowed multiple libraries and same split was done, it
# would be "all libraries' direct sources" and "all libraries' transitive
# dependencies". This sounds reasonable, but would break strict deps. If one library
# had `@import './foo.css';` but forgot to add a dep on `:foo` yet another library in
# this binary had a dep on `:foo`, the first would incorrectly compile successfully.
# As a result, we need a separate `css_bundle()` for each compiled `css_library()`
# until we get proper strict deps support.
binaries = []
for (index, dep) in enumerate(deps):
binary_name = "%s_binary_%s" % (name, index)
_css_binary(
css_bundle(
name = binary_name,
dep = dep,
testonly = testonly,
Expand All @@ -47,61 +56,3 @@ def css_binaries(
visibility = visibility,
tags = tags,
)

# It might look like this implementation should accept multiple `css_library()` targets,
# but doing so would be a bad idea. This is because the library is separated into "direct
# sources" and "transitive dependencies" which are fed into `postcss_multi_binary()`. If
# this allowed multiple libraries and same split was done, it would be "all libraries'
# direct sources" and "all libraries' transitive dependencies". This sounds reasonable,
# but would break strict deps. If one library had `@import url("rules_prerender/foo");`
# but forgot to add a dep on `//:foo` yet another library in this binary had a dep on
# `//:foo`, the first would incorrectly compile successfully. As a result, we need a
# separate `postcss_multi_binary()` for each compiled `css_library()`.
def _css_binary(
name,
dep,
testonly = None,
visibility = None,
tags = None,
):
"""Generates a CSS "binary" for a single `css_library()` dependency.
The `css_library()` dependency gets each of its direct sources compiled into a
"binary", a new CSS file which bundles all the `@import` dependencies of that source.
Note that while this macro is called `_css_binary()` in the singular form, if the
`css_library()` dependency has multiple direct sources, each one will be compiled into
an independent "binary", meaning multiple binaries can be generated by this macro.
Returns:
`DefaultInfo`: Contains bundled CSS.
`CssImportMapInfo`: Maps importable paths to generated CSS binary files.
Args:
name: Name of this target.
dep: Dependency of this target which provides `CssInfo` (generally
`css_library()`).
testonly: https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes
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
"""
# 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_parcel" % name
css_bundle(
name = binary,
dep = dep,
testonly = testonly,
tags = tags,
)

# Return the binary outputs with a map of import name -> file path.
css_map(
name = name,
bin = ":%s" % binary,
lib = dep,
testonly = testonly,
visibility = visibility,
tags = tags,
)
80 changes: 0 additions & 80 deletions packages/rules_prerender/css/css_map.bzl

This file was deleted.

12 changes: 6 additions & 6 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/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",
"packages/rules_prerender/css/tests/group/bin1_binary_0/rules_prerender/packages/rules_prerender/css/tests/group/lib1.css",
"packages/rules_prerender/css/tests/group/bin2_binary_0/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/bin1_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib1.css"
expected_lib1 = "packages/rules_prerender/css/tests/group/bin1_binary_0/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/bin2_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/lib2.css"
expected_lib2 = "packages/rules_prerender/css/tests/group/bin2_binary_0/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_bin1_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")
"packages/rules_prerender/css/tests/group/conflicting_bin1_binary_0/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")
asserts.expect_failure(env,
"packages/rules_prerender/css/tests/group/conflicting_bin2_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")
"packages/rules_prerender/css/tests/group/conflicting_bin2_binary_0/rules_prerender/packages/rules_prerender/css/tests/group/conflicting_lib.css")

return analysistest.end(env)

Expand Down
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/bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/foo.css"
expected_foo = "packages/rules_prerender/css/tests/import_map/bin_binary_0/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/bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/bar.css"
expected_bar = "packages/rules_prerender/css/tests/import_map/bin_binary_0/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/different_packages_bin_binary_0_parcel/rules_prerender/packages/rules_prerender/css/tests/import_map/subpackage/lib.css"
expected_lib = "packages/rules_prerender/css/tests/import_map/different_packages_bin_binary_0/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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('prerender_component_publish_files()', () => {
'script_dep.d.ts',
// Temporary bad assertion. Need to update component publishing to copy
// styles into the correct position in the NPM package.
'component_styles_bin_binary_0_parcel',
'component_styles_bin_binary_0',
'component_resources', // from `resources` attribute.
]);

Expand Down

0 comments on commit 3cc94b6

Please sign in to comment.