diff --git a/packages/rules_prerender/link_prerender_component.bzl b/packages/rules_prerender/link_prerender_component.bzl index d75c1ee1..0a523b38 100644 --- a/packages/rules_prerender/link_prerender_component.bzl +++ b/packages/rules_prerender/link_prerender_component.bzl @@ -45,6 +45,7 @@ def link_prerender_component(name, package, visibility = None, testonly = None): scripts = package, styles = None, resources = None, + component_check = None, visibility = visibility, testonly = testonly, ) diff --git a/packages/rules_prerender/prerender_component2.bzl b/packages/rules_prerender/prerender_component2.bzl index f2433f11..8e5dc147 100644 --- a/packages/rules_prerender/prerender_component2.bzl +++ b/packages/rules_prerender/prerender_component2.bzl @@ -14,6 +14,7 @@ load( "alias_with_metadata", "prerender_metadata", ) +load(":visibility_aspect.bzl", "VisibilityInfo", "visibility_aspect") load(":web_resources.bzl", "web_resources") visibility("public") @@ -97,6 +98,11 @@ def prerender_component( specific re-export you want. * Exception: You may `bazel build` a `prerender_component()` target or have a `build_test()` depend on it to verify that it is buildable. + 4. Any direct dependency of a `prerender_component()` target *must* be + defined in the same Bazel package and have private visibility. + * This is enforced at build time. + * Acts as a guardrail to make it less likely to run afoul of the above + rules. Args: name: The name of this rule. @@ -118,6 +124,21 @@ def prerender_component( %{name}_styles: A reexport of the `styles` attribute. %{name}_resources: A reexport of the `resources` attribute. """ + # Build all dependencies and sanity check the component. This allows + # `bazel build` of a `prerender_component` to validate that the component is + # buildable and usable. + _component_check( + name = name, + prerender = prerender, + scripts = scripts, + styles = styles, + resources = resources, + testonly = testonly, + # Explicitly private because this just validates that the component is + # buildable. It should never be depended upon directly. + visibility = ["//visibility:private"], + ) + styles_reexport = "%s_styles_reexport" % name if styles: _inline_css_reexport( @@ -136,6 +157,9 @@ def prerender_component( styles = ":%s" % styles_reexport if styles else None, styles_import_map = ":%s" % styles_reexport if styles else None, resources = resources, + # Include `_component_check()` output to run its Starlark implementation + # and run its sanity checks. + component_check = ":%s" % name, testonly = testonly, ) @@ -188,6 +212,82 @@ def prerender_component( testonly = testonly, ) +def _component_check_impl(ctx): + # Identify all the provided slices and what kind of dependency edge they use. + nullable_slices = { + "prerender": ctx.attr.prerender, + "scripts": ctx.attr.scripts, + "styles": ctx.attr.styles, + "resources": ctx.attr.resources, + } + slices = {edge: slice for (edge, slice) in nullable_slices.items() if slice} + + for (edge, slice) in slices.items(): + # Check slice is in the same package. + if slice.label.package != ctx.label.package: + fail(""" +{target} depends on {slice} which is in a different Bazel package (BUILD file). + +Direct dependencies of `prerender_component` targets *must* be defined in the same package. This allows `prerender_component` to properly encapsulate all the aspects of a component. + +Possible solutions: +1. If {slice} is only used by {target}, move {slice} into {package}. +2. If {slice} is used elsewhere, make a `prerender_component` in {dep_package} to encapsulate that target. Then use that new component from {target} just like any other. + """.strip().format( + target = ctx.label, + slice = slice.label, + package = "//%s" % ctx.label.package, + dep_package = "//%s" % slice.label.package, + )) + + # Check slice has private visibility. + visibility = slice[VisibilityInfo].visibility + non_private_visibility = [scope for scope in visibility + if scope != Label("//visibility:private")] + if non_private_visibility: + fail(""" +{slice} must have private visibility. Any dependencies on it should depend on {component_slice} instead. + +Direct dependencies of `prerender_component` targets *must* have private visibility so all incoming dependencies flow through {component_slice} instead. This allows `@rules_prerender` to collect all the different parts of a component and bundle them correctly. + +Solution: Update {slice} to use private visibility. This can be done by deleting the `visibility` attribute (Bazel targets are private by default). + +{slice} is currently visible to: +{visibility} + """.strip().format( + slice = slice.label, + component_slice = "%s_%s" % (ctx.label, edge), + visibility = "\n".join([str(label) for label in non_private_visibility]), + )) + + # Collect all the `DefaultInfo` outputs of the component slices and return + # them so `bazel build`-ing a `prerender_component` implicitly builds all + # its dependencies. + files_to_build = [ctx.attr.prerender[DefaultInfo].files] + if ctx.attr.scripts: + files_to_build.append(ctx.attr.scripts[DefaultInfo].files) + if ctx.attr.styles: + files_to_build.append(ctx.attr.styles[DefaultInfo].files) + if ctx.attr.resources: + files_to_build.append(ctx.attr.resources[DefaultInfo].files) + return DefaultInfo( + files = depset([], transitive = files_to_build), + ) + +_component_check = rule( + implementation = _component_check_impl, + attrs = { + "prerender": attr.label( + mandatory = True, + aspects = [visibility_aspect], + ), + "scripts": attr.label(aspects = [visibility_aspect]), + "styles": attr.label(aspects = [visibility_aspect]), + "resources": attr.label(aspects = [visibility_aspect]), + }, + doc = "Performs component-level sanity checks and builds all slices.", +) + def _js_reexport_impl(ctx): merged_js_info = js_info( declarations = depset([], diff --git a/packages/rules_prerender/prerender_metadata.bzl b/packages/rules_prerender/prerender_metadata.bzl index 5a78df01..be6879cc 100644 --- a/packages/rules_prerender/prerender_metadata.bzl +++ b/packages/rules_prerender/prerender_metadata.bzl @@ -14,6 +14,7 @@ PrerenderMetadataInfo = provider( "styles": "The CssInfo of the styles target.", "styles_import_map": "The CssImportMapInfo of the styles target.", "resources": "The WebResourceInfo of the resources target.", + "component_check": "The DefaultInfo of the component check target.", }, ) @@ -24,6 +25,7 @@ def _prerender_metadata_impl(ctx): styles = _safe_get(ctx.attr.styles, CssInfo), styles_import_map = _safe_get(ctx.attr.styles_import_map, CssImportMapInfo), resources = _safe_get(ctx.attr.resources, WebResourceInfo), + component_check = _safe_get(ctx.attr.component_check, DefaultInfo), ) prerender_metadata = rule( @@ -37,6 +39,7 @@ prerender_metadata = rule( "styles": attr.label(providers = [CssInfo]), "styles_import_map": attr.label(providers = [CssImportMapInfo]), "resources": attr.label(providers = [WebResourceInfo]), + "component_check": attr.label(providers = [DefaultInfo]), }, doc = """ Collects all the various "slices" of a component together into a single diff --git a/packages/rules_prerender/visibility_aspect.bzl b/packages/rules_prerender/visibility_aspect.bzl new file mode 100644 index 00000000..0643ad67 --- /dev/null +++ b/packages/rules_prerender/visibility_aspect.bzl @@ -0,0 +1,20 @@ +visibility("private") + +VisibilityInfo = provider(fields = { + "visibility": "Visibility of the target.", +}) + +def _visibility_aspect_impl(target, ctx): + return [VisibilityInfo(visibility = ctx.rule.attr.visibility)] + +visibility_aspect = aspect( + implementation = _visibility_aspect_impl, + # Don't actually walk the depgraph. This aspect should be directly placed on + # any attributes it needs to process. Since it only needs read direct + # dependencies there is no need transitive dependency processing. + attr_aspects = [], + doc = """ + Returns a `VisibilityInfo` provider with the visibility declaration of + the target. + """, +)