Skip to content

Commit

Permalink
Updates TODOs.
Browse files Browse the repository at this point in the history
Refs #48.

This deletes or updates `TODOs` where follow up issues have been filed separate from the `@aspect_rules_js` migration. I generally deleted most of the `TODOs` because I personally feel that this kind of information should be tracked in issues. However I left a couple as notes of exactly where to cleanup when certain tasks are complete.
  • Loading branch information
dgp1130 committed Feb 14, 2023
1 parent 15e278c commit 8e100f1
Show file tree
Hide file tree
Showing 8 changed files with 4 additions and 15 deletions.
4 changes: 1 addition & 3 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ ts_config(
npm_link_all_packages(name = "node_modules")

# Makes an internal NPM package for `rules_prerender` and links it.
# TODO(#48): Move to `//packages/rules_prerender` once dependencies outside
# that package are addressed.
stamp_package(name = "package")
npm_package(
name = "rules_prerender_pkg",
Expand All @@ -41,7 +39,7 @@ npm_package(
package = "rules_prerender",
visibility = ["//visibility:public"],

# TODO(#48): Remove when upstream is fixed.
# TODO(#59): Remove when upstream is fixed.
# See: https://github.com/dgp1130/rules_prerender/issues/48#issuecomment-1425257276
include_external_repositories = ["rules_prerender"],
)
Expand Down
2 changes: 1 addition & 1 deletion WORKSPACE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ npm_install(

# Load the `@aspect_rules_js` version of the `@npm` workspace. Named `@npm_rules_js` to avoid
# conflicting with the existing workspace from `@build_bazel_rules_nodejs`.
# TODO(#48): Rename to `@npm` once migrated off `@build_bazel_rules_nodejs`.
# TODO(#63): Rename to `@npm` once migrated off `@build_bazel_rules_nodejs`.
load("@aspect_rules_js//npm:npm_import.bzl", "npm_translate_lock")
npm_translate_lock(
name = "npm_rules_js",
Expand Down
4 changes: 0 additions & 4 deletions dependencies.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ load("@npm//@bazel/postcss:package.bzl", "rules_postcss_dependencies")

def rules_prerender_dependencies():
"""Installs workspace-level dependencies for `rules_prerender`."""
# TODO(#48): Remove this requirement eventually.
if "build_bazel_rules_nodejs" not in native.existing_rules():
fail("`rules_prerender` requires `build_bazel_rules_nodejs` workspace to be installed under that name.")

# TODO(#48): Remove this dependency.
rules_postcss_dependencies()

maybe(
Expand All @@ -37,7 +35,6 @@ def rules_prerender_dependencies():
url = "https://github.com/aspect-build/rules_ts/releases/download/v1.3.0/rules_ts-v1.3.0.tar.gz",
)

# TODO(#48): Remove from publicly visible dependencies.
maybe(
http_archive,
name = "aspect_rules_jasmine",
Expand All @@ -46,7 +43,6 @@ def rules_prerender_dependencies():
url = "https://github.com/aspect-build/rules_jasmine/archive/refs/tags/v0.3.0.tar.gz",
)

# TODO(#48): Remove from publicly visible dependencies.
maybe(
http_archive,
name = "io_bazel_rules_webtesting",
Expand Down
2 changes: 1 addition & 1 deletion examples/external/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ ts_config(
prerender_pages(
name = "site",
src = "site.ts",
source_map = True, # TODO(#48): This doesn't seem to work?
source_map = True,
tsconfig = "//:tsconfig",
lib_deps = ["//:node_modules/rules_prerender"],
deps = ["//component"],
Expand Down
1 change: 0 additions & 1 deletion examples/external/WORKSPACE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ nodejs_register_toolchains(
node_version = "16.10.0",
)

# TODO(#48): Remove this dependency.
load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")
npm_install(
# Name this npm so that Bazel label references look like `@npm//package`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ npm_package(
package = "@rules_prerender/declarative_shadow_dom",
visibility = ["//visibility:public"],

# TODO(#48): Remove when upstream is fixed.
# TODO(#59): Remove when upstream is fixed.
# See: https://github.com/dgp1130/rules_prerender/issues/48#issuecomment-1425257276
include_external_repositories = ["rules_prerender"],
)
Expand Down
3 changes: 0 additions & 3 deletions packages/rules_prerender/link_prerender_component.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ load("@aspect_rules_js//js:providers.bzl", "JsInfo")
load("//tools:typescript.bzl", "ts_project")
load("//packages/rules_prerender:web_resources.bzl", "WebResourceInfo")

# TODO(#48): Split prerender and scripts files from package.
def link_prerender_component(name, package, visibility = None, testonly = None):
"""Links a `prerender_component()` from a linked NPM package.
Expand Down Expand Up @@ -56,15 +55,13 @@ def link_prerender_component(name, package, visibility = None, testonly = None):
testonly = testonly,
)

# TODO(#48): Support styles in `link_prerender_component()`.
native.filegroup(
name = "%s_styles" % name,
srcs = [], # Empty.
visibility = visibility,
testonly = testonly,
)

# TODO(#48): Support resources in `link_prerender_component()`.
_empty_web_resources(
name = "%s_resources" % name,
visibility = visibility,
Expand Down
1 change: 0 additions & 1 deletion packages/rules_prerender/prerender_pages.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ def prerender_pages(
testonly = testonly,
deps = [
":%s_scripts" % prerender_name,
# TODO(#48): Don't require this dependency in the user's workspace.
"//:node_modules/@rollup/plugin-node-resolve",
],
)
Expand Down

0 comments on commit 8e100f1

Please sign in to comment.