Update to `@aspect_rules_js`

[dgp1130]

The future of the Bazel Node ecosystem appears to be rules_js. It is still in RC right now but hopefully coming to stable soon. Would be worth playing around to see if it can improve things. The model of running actions inside a single source tree could smooth over some rough edges with ts_project() and tools which insist on the node_modules/ structure, like Parcel.

[dgp1130]

Migration doc - https://docs.aspect.build/aspect-build/rules_js/v1.0.0-rc.1/docs/migrate.html.

Attempting to follow the instructions, switching to pnpm seems easy enough. I needed to disable strict peer deps since @bazel/concatjs has a peer dep on Karma packages, but I don't actually use them so it should be safe to ignore. Apparently I'm also missing a dep on jasmine-core from @bazel/jasmine, but I'll worry about that later.

I'm a bit confused by the docs which indicate that "Link the node modules" comes before "Update WORKSPACE", feels like that order should be swapped? Also "Update WORKSPACE" is pretty lacking on details and how the existing workspace configuration should change. I tried dropping @build_bazel_rules_nodejs as I thought it was a dependency of @rules_js, but it turns out that dependency is actually @rules_nodejs, which is different from @build_bazel_rules_nodejs.

I also forced the Node version to stay the same at v12, though this technically isn't supported by pnpm. As long as pnpm isn't executed by Bazel directly (and I don't think it is), then I can use an out-of-band Node version for that separate from the rest of the repository. After this migration I can hopefully have an easier time swapping out rules_postcss for Parcel, which will unblock me from upgrading Node.

Next problem is load("@npm//@bazel/postcss:package.bzl", "rules_postcss_dependencies"), which doesn't work as that package.bzl file doesn't exist anymore. Commenting it out, I have similar errors for all imported Starlark code. I'm not too sure exactly how this is supposed to work as the migration instructions don't really elaborate. Do I need to update these packages now (ie. replace @bazel/typescript with rules_ts?) or should they still work? So far, I think I've only added @rules_js and updated it to generate @npm instead of @build_bazel_rules_nodejs. It seems these have been restructured as @npm//:@bazel/typescript/package_json.bzl. Rewriting the imports to that format seems to kinda work, but I'm unclear on differences between package.bzl and index.bzl (do they get magically merged together into package_json.bzl?).

Also I still need strict_visibility = False for macros and exports_directories_only = False for PostCSS stuff, so I'm not sure if those are hard blockers for this migration? It seems like ts_project() was, but I also still use --bazel_patch_module_resolver and link_workspace_root = True, which also might need to be removed first? Questions around how I should deploy my ruleset also come up, since rules_prerender currently ships index.bzl and the rest of the Starlark in the NPM rules_prerender package. It seems like @rules_ts no longer does this like @bazel/typescript does. Do I need to do the same and have users set this up via their WORKSPACE? Am I screwed for @bazel/postcss or can I use their WORKSPACE-based setup instructions?

Attempting to switch to @rules_postcss WORKSPACE setup finally passes loading however now I get:

Error: file '@npm//:@bazel/typescript/package_json.bzl' does not contain symbol 'ts_project'

which invalidates my hope of just rewriting imports. AFAICT there is no way to load Starlark code from an NPM package. That implies that I need to migrate all the rulesets I'm using to their rules_js versions in a single commit, which doesn't seem right. Maybe I could have two versions of @npm? Instead of replacing @npm, I added a new @npm_rules_js which managed to not break anything.

Not sure where to go next. The docs suggest replacing any binary tool invocations from node_modules to use @rules_js, but it turns out I don't do any of that. rules_prerender only ever imports hand-written macros / rules from Starlark, not generated binary tool wrappers. I guess the next step might be to replace one of my nodejs_binary() targets with a js_binary()? Not sure how that will play with @bazel/typescript ts_project() targets, but if they use the same providers, I think it should work?

I tried updating //packages/annotation_extractor to use js_binary(). This immediately failed because it can't copy_to_bin() from an external repository @npm (probably for the @npm//yargs dep I had. I tried switching this dependency to @npm_rules_js//:yargs, but this failed with an even more cryptic error.

ERROR: /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/external/npm_rules_js/BUILD.bazel:206:12: no such target '@npm_rules_jsyargs16.2.0//:yargs': target 'yargs' not declared in package '' defined by /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/external/npm_rules_jsyargs16.2.0/BUILD.bazel and referenced by '@npm_rules_js//:yargs'

The error is definitely internal to @rules_js, so I'm not sure where I'm going wrong here. It seems that workspace exists, but :yargs doesn't:

$ pnpm run bazel query @npm_rules_js__yargs__16.2.0//:all

> rules_prerender@0.0.0-PLACEHOLDER bazel /home/doug/Source/rules_prerender
> bazel "query" "@npm_rules_js__yargs__16.2.0//:all"

INFO: Invocation ID: 3b5a5313-bb2b-4c44-af46-074d1922620c
@npm_rules_js__yargs__16.2.0//:pkg
@npm_rules_js__yargs__16.2.0//:source_directory
Loading: 0 packages loaded

I think I'm definitely lost on how these two rulesets are supposed to interoperate with each other. I'll reach out to some Aspect folks and see if I can get some direction here. I think some of the questions I have are:

1. How do @rules_js and @build_bazel_rules_nodejs interoperate? Is it possible to incrementally migrate targets/rulesets from one ecosystem to the other?
2. Does @rules_postcss have a viable path here? Can I use it via @rules_js, or do I need to switch to something like Parcel as part of this process?
3. What other migrations need to be landed before @rules_js can be used? In particular, @rules_prerender is still using:
   • strict_visibility = False - Because I export Starlark macros invoked by user code in their repository which calls @rules_prerender dependencies (user transitive dependencies), see https://github.com/bazelbuild/rules_nodejs/issues/2393/.
   • exports_directories_only = False - This is needed by @rules_postcss for reasons I don't really understand.
   • --bazel_patch_module_resolver - Needed for workspace-relative imports.
   • link_workspace_root = True - Needed for workspace-relative imports.
   • How do these complicate the migration to @rules_js and which of them are hard blockers?
4. Since @rules_prerender itself exports a ruleset for other workspaces, do I need to change the deployment model? Is it still ok to ship Starlark code in NPM packages, or is that a no-no now?
   • Does this mean I need to use bzlmod? Is that a part of @rules_js or not?

Current process is in the rules-js, not yet convinced this is at all the right direction I should be going.

[dgp1130]

Looking at some examples, it seems that you're not supposed to depend on @npm_rules_js//:yargs, but rather //:node_modules/yargs from npm_link_all_packages().

Changing that dependency now builds the js_binary() successfully. However workspace-relative imports are busted due to the lack of link_workspace_root and --bazel_patch_module_resolver. Rewriting imports to be relative works and is probably the right solution here. I was able to remove link_workspace_root = True for everything but rollup_bundle(), which is a problem I can deal with separately. I tried to remove --bazel_patch_module_resolver but ran into some challenges with having the rules_prerender NPM package import a dependency in the workspace:

Cannot find module '../../common/models/prerender_resource'
Require stack:
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2493/execroot/rules_prerender/node_modules/rules_prerender/index.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2493/execroot/rules_prerender/bazel-out/host/bin/examples/minimal/page.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2493/execroot/rules_prerender/bazel-out/host/bin/packages/renderer/dynamic_import.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2493/execroot/rules_prerender/bazel-out/host/bin/packages/renderer/entry_point.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2493/execroot/rules_prerender/bazel-out/host/bin/packages/renderer/renderer.js

I get why that's an issue, resolving the path it would search for prerender_resources.js in /.../execroot/rules_prerender/common/models/prerender_resource.js, when the file is actually under /.../execroot/rules_prerender/bazel-out/host/bin/common/models/prerender_resource.js. I feel like I node to copy_to_bin() the node_modules/ directory, but I'm not sure how feasible that actually is. Fortunately I don't think I need to solve this problem right now, an incremental migration to @rules_js should hopefully address it.

Refocusing on migrating //packages/annotation_extractor, updating the dependencies to //:node_modules/%{dep} seems to be good enough. I also had to add the BAZEL_BINDIR variable to its usage, but after that it worked just fine. The other tools should work similarly, though renderer is a weird one because it has to dynamic import a file from an input, not a dependency.

Trying to migrate renderer struggled a bit with the entry point, since I needed to turn that into a relative import from the packages/renderer/dynamic_import.js file to the user's provided entry point in the package which prerender_resources() is generated. Fixing that seems to have been enough though, and it can load the file correctly. Unfortunately, that file imports the runtime at a bare rules_prerender path, which fails. So the next problem is out to simulate a local package and link it into node_modules/.

I feel like npm_package() and npm_link_package() are the right tools for this, but the docs show a lot of private implementation details which aren't exposed publicly and don't seem to work that way, so I'm a bit confused on how I'm supposed to use it.

I eventually discovered this example which shows how to use npm_link_package() from the root package. Using this strategy I'm able to link rules_prerender under node_modules/ in the runfiles tree. This kind of works, I suspect there's a bug in @aspect_rules_js here as adding/removing :node_modules/some_internal_package requires a bazel clean to take effect. After some fiddling I eventually got it working in a test js_binary(). It's able to import from rules_prerender, however rules_prerender cannot import outside its package:

Error: Cannot find module '../../common/models/prerender_resource'
Require stack:
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/rules_prerender/test_bin.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/rules_prerender@0.0.0/node_modules/rules_prerender/index.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/rules_prerender/test_bin.sh.runfiles/rules_prerender/packages/rules_prerender/test.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/rules_prerender/test_bin.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/rules_prerender@0.0.0/node_modules/rules_prerender/index.js:5:28)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/rules_prerender/test_bin.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/rules_prerender@0.0.0/node_modules/rules_prerender/index.js',
    '/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/rules_prerender/test_bin.sh.runfiles/rules_prerender/packages/rules_prerender/test.js'
  ]
}

This is the same problem removing --bazel_patch_module_resolver ran into. Fortunately this makes a little more sense to me now since this version of rules_prerender is under node_modules/, independent of any other sources present in the workspace. I think the solution here is to make rules_prerender vendor any other dependencies from the workspace in its own package. Looking at the package now I see:

$ bazel build //packages/rules_prerender:rules_prerender_pkg && tree dist/bin/packages/rules_prerender/rules_prerender_pkg/
INFO: Invocation ID: 023ce57c-59f0-4080-8bb9-0a9d9791b9c3
INFO: Analyzed target //packages/rules_prerender:rules_prerender_pkg (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //packages/rules_prerender:rules_prerender_pkg up-to-date:
  dist/bin/packages/rules_prerender/rules_prerender_pkg
INFO: Elapsed time: 0.111s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
dist/bin/packages/rules_prerender/rules_prerender_pkg/
├── index.d.ts
├── index.js
├── index.js.map
└── package.json

0 directories, 4 files

The package should have scripts.js and styles.js, but those aren't present. I assume this is because they are transitive dependencies of the npm_package() target, and I think pkg_npm() or some other @build_bazel_rules_nodejs rule worked that way. An easy solution there is to have some rule which collects all the transitive dependencies and exports them for npm_package() to consume. The bigger challenge is that anything outside of //packages/rules_prerender/... (where this npm_package() is defined, has no clear directory to live in. The issue is that this directory is NPM package-relative, not Bazel workspace-relative.

The very hacky workaround to solve this is to move the npm_package() target to the workspace-root BUILD.bazel file. Since that is at the root location, all the files it depends on will be in nested directories. so "NPM package-relative" and "Bazel workspace-relative" become equivalent. This surprisingly actually worked, though is not a great long-term strategy.

I migrated all the examples to include //:node_modules/rules_prerender and the vast majority of them "just worked". There are two use cases which didn't:

1. Styles - The renderer is supposed to dump execroot-relative paths into the rendered HTML, which the resource injector reads, finds those files, and then inlines them. However, migrating the renderer to @aspect_rules_js means it only sees root-relative paths, so the resource injector only sees root-relative paths. It needs to somehow resolve these back to their execroot-relative paths to unambiguously inline them.
2. Declarative shadow DOM - The rules_prerender/declarative_shadow_dom path doesn't import. I didn't get a chance to debug this further, but I'm guessing the problem is that previously I actually had two js_library(package_name = "rules_prerender") targets, one with the typical rules_prerender runtime, and another with the declarative shadow DOM piece. They worked because both happened to exist at the same Bazel package, which worked in @build_bazel_rules_nodejs and was desirable because depending on declarative shadow DOM should also include a client-side polyfill. I'm guessing this won't work in @aspect_rules_js (though I haven't confirmed that). I think the options to fix this are probably 1) split declarative shadow DOM into its own package or 2) put the declarative shadow DOM runtime into the rules_prerender package always. Not sure what the full implications of both of these will be.

Both of these problems feel solvable, so I'll keep hacking on them. At this point I think I have a reasonable understanding of how @aspect_rules_js and @build_bazel_rules_nodejs are supposed to interoperate. I think my remaining questions are:

1. How is @build_bazel_rules_nodejs's ts_project() supposed to work with @aspect_rules_js? //:node_modules/* dependencies don't really work. Should I be migrating that first?
2. How should I deploy rules_prerender? Should I stop putting Starlark code in the NPM package?

[dgp1130]

Fixed the styling issue by having the renderer still generate HTML with execroot-relative paths to CSS files. This is doable because it takes an inline style map as an input from Starlark, which maps "style import specifier" -> "execroot-relative path to CSS file". So even though the renderer is working in a single source tree with all root-relative paths, this map can still use an execroot-relative path and stay compatible with resource injector.

One other challenge I encountered in this process is that the inline style map actually wasn't working. I found that the map would be set correctly in the renderer, but then when it was read it would always be undefined. It seems that the issue here comes back to package management. Since rules_prerender is now its own package in node_modules/, it vendors the inline_style_map.js file. However, other imports of packages/rules_prerender/inline_style_map.js import through the workspace and get a different version of the file. This means the same source content is executed twice at different locations, with a different scope and state.

The solution here is to only use one version of the file. I updated the rules_prerender NPM package to export the inline style map getter and setter under private aliases so that one definition can be used even outside the package. Not elegant, but good enough for now.

Moving on to declarative shadow DOM, merging with the rules_prerender NPM package probably isn't a great idea since only the prerender source code needs to be in that package, meaning I have to break the invariant that a prerender_component() encapsulates all the flavors of its implementation and it is not possible to depend on any one flavor without also depending on the rest. While I could special case it for declarative shadow DOM, there's nothing inherently special about this component, and any use case in which a rules_prerender component is published to NPM will run into the same problem.

Therefore, I think the better solution is to put declarative shadow DOM into its own NPM package. To do that I need to take its published files and dump them into an npm_package() target, which seems easy enough. However it then begs the question of how to use convert that package back into a prerender_component(). This was previously done by manually creating a prerender_component() in the published NPM package's BUILD.bazel file. In theory, I could have a published_prerender_component(dep = ":some_npm_package_target") which converts the npm_package() into a prerender_component(), however I don't know how feasible that is given that an npm_package() packs everything into a TreeArtifact. That basically means I can't have any analysis time knowledge of anything in a prerender_component(). Being unable to ship Starlark in an NPM package also means we lose the context around which sources are for prerendering, which are for client side scripts, which are styles, and which are resources and how they're served.

I think we would need to define an NPM package format for shipping rules_prerender components, maybe something where the package.json defines exactly which files belong to what flavor of the component?

{
  "name": "@rules_prerender/declarative_shadow_dom",
  "rules_prerender": {
    "components": {
      "declarative_shadow_dom": {
        "prerender": ["./index.js", "./index.d.ts"],
        "client": ["./polyfill.js", "./polyfill.d.ts"],
        "styles": ["./styles.css"],
        "resources": {"/declarative_shadow_dom/logo.png": "./dsd_logo.png"}
      },
    }
  }
}

Then you'd need to extract this component from the npm_package():

npm_link_all_packages(name = "node_modules")

published_prerender_component(
    name = "rules_prerender_components/@rules_prerender/declarative_shadow_dom",
    package = ":node_modules/@rules_prerender/declarative_shadow_dom",
    component = "declarative_shadow_dom", # The component extracted from the `package.json`, because it could contain multiple.
)

prerender_pages(
    name = "site",
    # ...
    # Use like `//:node_modules/*`, but where a `prerender_component` is needed.
    deps = [":prerender_components/@rules_prerender/declarative_shadow_dom"],
)

The challenge here is that the implementation of published_prerender_component() requires execution-time only knowledge of source files, which would significantly complicate the implementation. I don't know how well ts_project(), css_library() / css_binaries(), or web_resources() will work without analysis-time knowledge.

Since the core challenge seems to be that @aspect_rules_js doesn't like Starlark code in NPM packages, we could choose to just not fight that. We have the same problem with the rest of the @rules_prerender Starlark rules, and if we have to require users to pull in those rules via a WORKSPACE file like @aspect_rules_ts does, then maybe we should do the same for deployed components. Having a WORKSPACE level dependency on @rules_prerender would enable users to directly depend on the prerender_component(name = "declarative_shadow_dom"). If users have to do this for rules_prerender anyways, then it wouldn't be too much more of an ask for them to do it for any other published components they want to use. It enforces a requirement that those components are built with prerender_component() via @rules_prerender, but I think that's a pretty reasonable requirement.

If we accept that users will depend on the :declarative_shadow_dom target via a WORKSPACE dependency on @rules_prerender, the next question is how they will import it. I'm not sure how to do a relative import across WORKSPACE boundaries, or how cross-WORKSPACE files even work in an @aspect_rules_js world. The most straightforward answer is to import via an NPM package specifier (ie. import { polyfillDeclarativeShadowDom } from '@rules_prerender/declarative_shadow_dom'). However this implies that there is either 1) a single package for both prerender and client scripts or 2) a package for prerender JS and another package for client JS. Neither is ideal since one bends the invariant that you can't import code written for a different platform (browser vs NodeJS) while the other requires multiple NPM packages to ship a single isolated piece of functionality (fortunately these packages don't actually need to be published to NPM, since they would be pulled in via the WORKSPACE).

A middle ground might be to use a single NPM package with two entry points like:

import { prerenderMyComponent } from 'some_component/prerender';
// vs.
import { doSomethingOnTheClinet } from 'some_component/client';

This would still avoid importing across contexts, but you would be depending across contexts, which isn't great either.

I asked on the Bazel Slack and confirmed that publishing Starlark via NPM is no longer supported. So I think exporting a real prerender_component() target and letting users depend on that is fine, however I still need to solve the import problem. Ideally, users would import from a rules_prerender specifier, though it was pointed out on Slack that this could lead to version skew if it uses a real NPM package. Instead, I think the ideal approach is to generate an npm_package() and link it in the user's workspace. I tried to hack through this and managed to make something kinda work for the declarative shadow DOM component. I managed to create a synthetic @rules_prerender/declarative_shadow_dom component containing the prerender and client-side scripts and call it from userspace. However, I couldn't get client-side scripts to build as rollup_bundle() doesn't process //:node_modules/@rules_prerender/declarative_shadow_dom as a dep. I tried upgrading to the @rules_rollup version of this rule which is supposed to be compatible with @aspect_rules_js, however this fails due to some weird fsevents errors in some bundled aspect lifecycle code I can't really debug.

ERROR: /home/doug/Source/rules_prerender/BUILD.bazel:21:22: Running lifecycle hooks on npm package fsevents@2.1.3 failed: (Exit 1): lifecycle-hooks.sh failed: error executing command bazel-out/k8-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh fsevents ../../../external/npm_rules_js__fsevents__2.1.3/package ... (remaining 1 argument skipped)

Use --sandbox_debug to see verbose messages from the sandbox
/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/2160/execroot/rules_prerender/bazel-out/k8-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh.runfiles/aspect_rules_js/npm/private/lifecycle/min/index.min.js:1
(()=>{var __webpack_modules__={7100:(__unused_webpack_module,exports,__nccwpck_require__)=>{"use strict";Object.defineProperty(exports,"__esModule",value:true});exports.codeFrameColumns=codeFrameColumns;exports["default"]=_default;var _highlight=__nccwpck_require__(1420);let deprecationWarningShown=false;function getDefs(chalk)return{gutter:chalk.grey,marker:chalk.red.bold,message:chalk.red.bold}}
// This goes on for quite some time...
9323:module,__unused_webpack_exports,__nccwpck_require__)=>{"use strict";

SyntaxError: Unexpected token '.'
    at wrapSafe (internal/modules/cjs/loader.js:915:16)
    at Module._compile (internal/modules/cjs/loader.js:963:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
INFO: Elapsed time: 0.189s, Critical Path: 0.06s
INFO: 2 processes: 2 internal.
FAILED: Build did NOT complete successfully

Not sure what to do with that.

[dgp1130]

Talked today with @alexeagle and @gregmagolan about some of the challenges (thank you both!). To write down my takeaways while they're still fresh in my mind:

@rules_prerender seems to be a bit unique because it includes both Starlark rules and also a JS runtime which is imported by user code. While both of these problems are solved individually, this seems to be the first ruleset which needs to do both. We discussed a number of solutions and ultimately came up with the following approach:

First, we split JS and Starlark code. Ship only userland rules_prerender JS content to the NPM package and have users set up a WORKSPACE-level dependency on @rules_prerender. Tool-internal code stays in the Bazel workspace. This allows users to easily depend on @rules_prerender Starlark rules and import the rules_prerender NPM package. However the next problem is how does code in the @rules_prerender Bazel workspace depend on the rules_prerender NPM package.

Answer: It doesn't. @rules_prerender//packages/renderer doesn't depend on @user//:node_modules/rules_prerender (because we don't have a label for @user), but instead has a type-only dependency on @rules_prerender//packages/rules_prerender. prerender_resources() is already executed in userland and has a static entry point. We can change this to generate an entry point in the user's Bazel workspace with a dependency on @user//:node_modules/rules_prerender. This generated entry point can then import * as rulesPrerenderLib from 'rules_prerender'; and pass that object down the call stack. This keeps a single definition of rules_prerender NPM used by userland code and @rules_prerender internals.

Splitting the JS and Starlark does mean there is a potential for version skew. I'm thinking prerender_resources() can generate a test which verifies that the //:node_modules/rules_prerender dependency is the same version as hard-coded in Starlark. Need to figure out how to do that without actually hard-coding a version string in the workspace as I've so far managed to avoid bumping any versions as part of releases.

A similar trick can be done for published prerender_component() targets like declarative shadow DOM, though probably less elegantly. I think this would work by publishing the JS code in a @rules_prerender/declarative_shadow_dom package and then using the Starlark dependency on @rules_prerender to depend on a prerender_component() at @rules_prerender//packages/declarative_shadow_dom. The challenge is that @rules_prerender//packages/declarative_shadow_dom cannot depend on @user//:node_modules/@rules_prerender/declarative_shadow_dom. Instead we need to do a similar trick to before. Where prerender_resources() generated a nodejs_binary() in the user workspace with a dependency on @user//:node_modules/rules_prerender, we need some kind of link_prerender_component() rule in the user's workspace which can depend on @user//:node_modules/@rules_prerender/declarative_shadow_dom. This will be a bit extra boilerplate, but I don't think it should be too bad.

There are still a few open questions on the prerender_component() use case. Most notably:

1. How do we split prerender and client side JS to prevent bad cross dependencies?
   • Could process the package TreeArtifact into two different providers, one for prerender JS and the other for client-side JS, then feed each one into a separate ts_project(). This would type check the build correctly but requires multiple entry points in the NPM package and wouldn't align with intellisense in editors which assume both versions of the JS are always available.
   • Could define the package with two exports fields, one for prerender JS and the other for client-side JS and keep one ts_project() for both. This makes it possible to accidentally cross-import between the two, but hopefully a harder mistake to make. Maybe some stronger conventions plus a lint or conformance check might help protect against that.
   • Could use two NPM packages, one for prerender JS and the other for client-side JS. This feels like a lot of overhead and isn't compatible with the abstraction that an NPM package can fully encapsulate a prerender_component().
2. Where do CSS or resources live? In the NPM package or the Starlark prerender_component() in the @rules_prerender workspace?
   • Logically I think it makes sense for these resources to be in the NPM package, so it has all the information needed to use the component.
   • This aligns with the mindset of "How would you use a prerender component outside of Bazel?" though that's not really a goal of this project.
   • Putting everything in the NPM package (and thus a TreeArtifact) means we lose analysis-time knowledge of these files though.

I feel confident that I can get the rules_prerender NPM package to work with this model, though I'm a bit hazy on the details with published prerender_component() targets. I'll have to spend some time hacking through it and see what I can get working.

[dgp1130]

Tried to make something work here. I started by shifting all usages of the rules_prerender runtime to only be imported via the rules_prerender NPM package and passed down to the renderer as an input. This was done using a different entry point for the renderer which looked like:

const { context } = require('rules_prerender');
const { execute } = require('path/to/packages/renderer/renderer.js');

execute(context);

path/to/packages/renderer/renderer.js needs to be dynamically computed based on the Bazel package this entry point is generated in, but I'm hard-coding ./renderer.js which is good enough for the test binary right now.

Despite this, I was still seeing inline_style_map being loaded twice. I eventually traced this back to the fact that even though I didn't actually have any runtime imports of rules_prerender, the JS was still getting pulled in via @rules_prerender//:node_modules/rules_prerender pulled into the renderer runfiles. I found the two files existed at:

.../execroot/rules_prerender/node_modules/rules_prerender/index.js
.../execroot/rules_prerender/bazel-out/k8-dbg/bin/packages/renderer/renderer_test.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/rules_prerender@0.0.0/node_modules/rules_prerender/packages/rules_prerender/index.js

I knew I would want to drop these JS sources, I was just being lazy about it and didn't expect that to be required. I defined a new types_only() rule which propagates DeclarationInfo and LinkablePackageInfo while dropping the JS providers. This removed the second instance of those files and made inline_style_map only get loaded once.

The next problem is that the renderer test code cannot import rules_prerender:

 Cannot find module 'rules_prerender'
    Require stack:
    - /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/314/execroot/rules_prerender/_tmp/66ad16056267f243223fe5901a481a06/useTempDir-c3fyuV/foo.js
    - /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/314/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/renderer/renderer_test.sh.runfiles/rules_prerender/packages/renderer/dynamic_import.js
    - /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/314/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/renderer/renderer_test.sh.runfiles/rules_prerender/packages/renderer/entry_point.js
    - /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/314/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/renderer/renderer_test.sh.runfiles/rules_prerender/packages/renderer/renderer.js

This code is generated by the test with a require('rules_prerender') expression. I used to have to manually resolve runfiles in these tests, but I think they should be able to just import rules_prerender.

# Failed import originates from here:
.../execroot/rules_prerender/_tmp/66ad16056267f243223fe5901a481a06/useTempDir-c3fyuV/foo.js

# Parent `node_modules/` directory exists here:
.../execroot/rules_prerender/node_modules/
# which is a symlink to:
.../execroot/rules_prerender/bazel-out/k8-fastbuild/bin/packages/renderer/renderer_test.sh.runfiles/npm/node_modules

# `rules_prerender` is not present in `node_modules/`.
.../execroot/rules_prerender/node_modules/rules_prerender/ # <--- Does not exist.

The test itself does have a dependency on //:node_modules/rules_prerender but this doesn't seem to get reflected in the output tree. The symlink to renderer_test.sh.runfiles/npm/node_modules/ seems to indicate that this is using the @bulid_bazel_rules_nodejs NPM dependencies, not @rules_js. If I add a dependency on //packages/rules_prerender, the import succeeds and .../execroot/rules_prerender/node_modules/rules_prerender/ exists (but inline_style_map gets duplicated). I'm guessing I'll need to migrate the test to the @rules_js form of jasmine_node_test(). Given that this is a test only issue, I'll ignore it for now.

Running bazel test //examples/... I'm able to get everything which doesn't use declarative shadow DOM to pass. @bazel/runfiles didn't work so I had to manually use process.env['RUNFILES'] to get //examples/data/... to pass.

Starting on the declarative shadow DOM problem, I made a new prerender_package() rule which generates an npm_package() which contains a generated /rules_prerender.json file which looks like:

{
  "components": {
    "declarative_shadow_dom": {
      "prerender": [
        "declarative_shadow_dom.js",
        "declarative_shadow_dom.d.ts",
        "declarative_shadow_dom.js.map"
      ],
      "scripts": [
        "declarative_shadow_dom_polyfill.js",
        "declarative_shadow_dom_polyfill.d.ts",
        "declarative_shadow_dom_polyfill.js.map"
      ],
      "styles": [],
      "resources": "declarative_shadow_dom_resources/",
    }
  }
}

Then, I've got a prerender_link_component() which runs in the root of the user's workspace to generate a prerender_component() target from these files. Aliasing :%{component}_prerender and :%{component}_scripts to //:node_modules/@rules_prerender/declarative_shadow_dom actually gets a lot farther than I thought.

TypeScript can't process the import to @rules_prerender/declarative_shadow_dom, because it would need a dependency on the @build_bazel_rules_nodejs ts_project(), which isn't accessible. I'm ignoring with @ts-ignore for now, but hopefully that's a problem which will go away once the ts_project() targets get updated to @rules_js. It also can't process an import from the declarative shadow DOM component onto the rules_prerender NPM package. I'm a bit surprised by that since it does still have a dependency on @npm//rules_prerender in addition to //:node_modules/rules_prerender. Also @ts-ignore-ing this for now, though I don't feel like I should need to.

This is enough for the renderer to successfully invoke the declarative shadow DOM component and render the HTML output 🎉. Unfortunately Rollup fails to bundle as it doesn't seem to be getting the @rules_prerender/declarative_shadow_dom package in its inputs. I think the problem is that @bazel/rollup uses JSModuleInfo / JSEcmaScriptModuleInfo sources if available, and drops DefaultInfo (link). In prerender_pages_unbundled(), a script entry point is generated and the dependencies are added via js_library(). This generates JSModuleInfo and JSEcmaScriptModuleInfo providers, but they do not contain the NPM package, while DefaultInfo does.

$ bazel-pquery //examples/declarative_shadow_dom:site_page_scripts
@build_bazel_rules_nodejs//internal/providers:js_providers.bzl%JSEcmaScriptModuleInfo = struct(
    direct_sources = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
    ]),
    sources = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
    ]),
),
@rules_nodejs//nodejs/private/providers:js_providers.bzl%JSModuleInfo = struct(
    direct_sources = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
    ]),
    sources = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
    ]),
),
@build_bazel_rules_nodejs//internal/providers:js_providers.bzl%JSNamedModuleInfo = struct(
    direct_sources = depset([]),
    sources = depset([]),
),
@rules_nodejs//nodejs/private/providers:linkable_package_info.bzl%LinkablePackageInfo = struct(
    files = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
    ]),
    package_name =     "",
    package_path =     "",
    path =     "bazel-out/k8-fastbuild/bin/examples/declarative_shadow_dom",
),
@rules_nodejs//nodejs/private/providers:declaration_info.bzl%DeclarationInfo = struct(
    declarations = depset([]),
    transitive_declarations = depset([
        <generated file node_modules/.aspect_rules_js/@rules_prerender+declarative_shadow_dom@0.0.0/node_modules/@rules_prerender/declarative_shadow_dom>,
        <generated file node_modules/@rules_prerender/declarative_shadow_dom>,
    ]),
    type_blocklisted_declarations = depset([]),
),
FileProvider = file_provider(
    files_to_build = depset([
        <generated file examples/declarative_shadow_dom/site_page_scripts.js>,
        <generated file node_modules/.aspect_rules_js/@rules_prerender+declarative_shadow_dom@0.0.0/node_modules/@rules_prerender/declarative_shadow_dom>,
        <generated file node_modules/@rules_prerender/declarative_shadow_dom>,
    ]),
),

The @aspect_rules_js version in @aspect_rules_rollup doesn't appear to conditionalize the inputs, so the solution might be to switch to that. I tried this again and ran into the same error as before. After attempting to narrow down the issue, I found that adding @rollup/plugin-node-resolve to the config was the part that was breaking. I tried upgrading the NodeJS version to 16.10.x and saw the error go away. So this should be an easy upgrade, though currently @rules_postcss is blocking that. As a part of this migration I can move to Parcel, which should unblock a Node.js version upgrade. Unfortunately this has the immediate effect of breaking CSS bundling.

I could possibly downgrade Rollup, but apparently @aspect_rules_rollup only supports the latest version. https://github.com/bazelbuild/rules_postcss/issues/73 mentions a few workarounds, and turning off CSS sourcemaps is the easiest option for now. I'm pretty sure they don't really work anyways, so there's no need to keep them on.

A side effect of switching to @aspect_rules_rollup is that link_workspace_root is no longer supported, so all includeScript() calls are relative to the package where prerender_pages() lives, which is super-unintuitive. Fortunately it's pretty easy to generate and prepend some ../ strings to the import specifiers to make everything workspace relative. Still sucks that there's no easy way to make these paths file-relative, but that's a separate problem.

With Node.js updated to version 16.10.0 and using @aspect_rules_rollup, I was finally able to get the declarative shadow DOM component to work in @aspect_rules_js, hooray! Current work is in ref/rules-js. Biggest issue is still typing, but maybe an upgrade to @rules_ts will fix that. I'll keep hacking and try to migrate more of the repository and clean up the work I've already got functional.

[dgp1130]

I tried migrating some of the ts_project() targets to @aspect_rules_ts. I found the tsconfig behavior confusing given that I would like to generally have one tsconfig.json for the whole repository (with overrides where appropriate). I eventually found:

ts_project(
    name = "proj",
    srcs = ["source.ts"],
    tsconfig = {}, # Removes the requirement for a package-local `tsconfig.json`.
    extends = "//:tsconfig", # Must be `@aspect_rules_ts` version of `ts_config()`.
    declaration = True,
    source_map = True,
    deps = [":dep"],
)

This mostly works the way I want it to. It doesn't require a package-local tsconfig.json and it can compose other ts_project() targets easily. The biggest challenge is that any transitive @npm//... dep will fail the build, meaning all of those dependencies need to be updated to //:node_modules/... at the same time.

jasmine_node_test() targets also fail (can't resolve NPM packages), presumably because they are using nodejs_test() under the hood rather than js_test(). I considered upgrading to @aspect_rules_jasmine before discovering that doesn't exist. The closest equivalent would be @aspect_rules_jest, so I may need to migrate to Jest first. Unfortunately there doesn't seem to be a @bazel/jest package either, so I guess the migration to Jest and @aspect_rules_js have to happen at the same time, though they can probably be done test-by-test.

I tried setting up @aspect_rules_jest and migrated the annotation extractor to use jest_test(). This wasn't too difficult to get working, though I need to remove all my @types/jasmine and import 'jasmine'; deps. Surprisingly, it type checks successfully without any NPM dependency on Jest (or @types/node for that matter). I wonder if a transitive dep is pulling in those sources and ts_project() doesn't apply any strict deps checking? I'm very confused by that and will need to investigate a bit more.

I was also able to move the Jest change before the @aspect_rules_ts migration. So I think it should be possible to migrate all tests to @aspect_rules_jest and then migrate the rest of the toolchain without breaking the tests at the same time. Current snapshot at ref/rules-js-2.

[dgp1130]

I tried running Jasmine within @aspect_rules_js based on this example for Karma and mostly got it to work without too many issues. I do need to use //:node_modules/* deps, and sometimes I still need to keep the @npm//* deps, which I don't fully understand, but the tests do run as expected.

I did run into one novel issue where @aspect_rules_js failed to install bl@4.0.3 because it has a duplicate key (license) in its package.json. I had to manually bump the version in the lockfile (which was way harder than it really should be, given that pnpm update bl@4.0.4 still updated everything and nothing would easily tell me the integrity hash for a manual edit).

I also discovered that web_test_suite() ignores the args of the wrapped test, so Jasmine's --config argument needs to be specified on both the bin.jasmine_test() as well as the web_test_suite() which wraps it to support both use cases.

I got all the //packages/... tests to pass, but the WebDriverIO tests failed because they can't load debug:

Error: Cannot find module 'debug'
Require stack:
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/19/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/devtools@7.16.3/node_modules/devtools/build/utils.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/19/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/devtools@7.16.3/node_modules/devtools/build/devtoolsdriver.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/19/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/devtools@7.16.3/node_modules/devtools/build/index.js
- /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/19/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/webdriverio@7.16.3/node_modules/webdriverio/build/utils/index.js
- ...

Looking in the test runfiles, I don't see debug, only direct dependencies are present:

$ lla dist/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/
total 24
drwxr-xr-x 167 doug doug 12288 Jul 18 21:00 .aspect_rules_js
lrwxrwxrwx   1 doug doug   151 Jul 18 20:45 http-status-codes -> /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/node_modules/http-status-codes
lrwxrwxrwx   1 doug doug   143 Jul 18 20:45 tree-kill -> /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/node_modules/tree-kill
lrwxrwxrwx   1 doug doug   145 Jul 18 21:00 webdriverio -> /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/node_modules/webdriverio

Looking in .aspect_rules_js, I do see the relevant debug package:

$ ll dist/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/
# ...
drwxr-xr-x 3 doug doug 4096 Jul 18 21:00 debug@2.6.9
drwxr-xr-x 3 doug doug 4096 Jul 18 21:00 debug@4.3.1

I'm unclear how Node is supposed to find those packages though. Interesting, webdriverio was able to find devtools, but devtools failed to find debug. Looking through the file tree, I noticed that each package has its own node_modules/ with only its direct dependencies listed and symlinked to their own packages:

$ ll /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/sandbox/linux-sandbox/23/execroot/rules_prerender/bazel-out/k8-fastbuild/bin/examples/components/test_chromium-local.sh.runfiles/rules_prerender/node_modules/.aspect_rules_js/devtools@7.16.3/node_modules/
total 36
drwxr-xr-x 2 doug doug 4096 Jul 18 21:33 @types
drwxr-xr-x 2 doug doug 4096 Jul 18 21:33 @wdio
lrwxrwxrwx 1 doug doug  195 Jul 18 21:33 chrome-launcher
drwxr-xr-x 3 doug doug 4096 Jul 18 21:33 devtools
lrwxrwxrwx 1 doug doug  190 Jul 18 21:33 edge-paths
lrwxrwxrwx 1 doug doug  194 Jul 18 21:33 puppeteer-core
lrwxrwxrwx 1 doug doug  205 Jul 18 21:33 query-selector-shadow-dom
lrwxrwxrwx 1 doug doug  192 Jul 18 21:33 ua-parser-js
lrwxrwxrwx 1 doug doug  184 Jul 18 21:33 uuid

Note that debug is not present. My guess is that this list is generated from package.json, and devtools does not have a dependency on debug. This seems accurate , yet devtools does require debug, apparently to monkey-patch it. It looks for puppeteer-core/node_modules/debug as a dependency of devtools. puppeteer-core does exist, but it does not contain a node_modules/ tree under it, it actually looks like:

puppeteer-core@10.4.0/
  node_modules/
    puppeteer-core/
      node_modules/ <-- Does not exist.
        debug/ <-- `devtools` looking here.
    debug/ <-- `debug` actually here.

This seems to mostly mirror the pnpm package structure: https://pnpm.io/symlinked-node-modules-structure.

Therefore devtools assumes that puppeteer-core does not have debug installed, and tries to require the global debug dependency, which isn't available because it doesn't declare a dependency on debug.

I figured this might be a pnpm-compatibility bug in devtools, but surprisingly pnpm actually works wither WebDriverIO in a minimal reproduction. require.resolve('debug') from devtools actually finds the debug package. Manually following the Node resolution algorithm, I found:

node_modules/.pnpm/node_modules/debug/...

The equivalent does not exist in @aspect_rules_js:

node_modules/.aspect_rules_js/node_modules/ <-- Does not exist.

Trying to understand why this directory exists, I eventually found this discussion which mentions the hoist option. Setting hoist=false in the .npmrc for my pnpm WebDriverIO repro, I'm able to reproduce the error. I'm guessing @aspect_rules_js disables hoisting by default, which is why this failed. The simplest solution is probably to hoist the debug package. Adding hoist-pattern[]=*debug* is enough for the pnpm repro to pass, but the same solution doesn't work with @aspect_rules_js. I'm guessing it doesn't have a dependency on the .npmrc file. Unfortunately I don't see anything in npm_translate_lock() which fixes this.

The migration guide mentions hooks.readPackage which can apparently allow me to declare this dependency for devtools. I don't have time to test this out right now, but I think there are two possible fixes that could be upstreamed here:

1. @aspect_rules_js should have some means of changing hoisting behavior (whether through .npmrc or in the WORKSPACE).
2. devtools shouldn't require hoisting for debug.

[dgp1130]

I made a fix for WebDriverIO in https://github.com/webdriverio/webdriverio/pull/8542, hopefully it'll land soon.

In the mean time, I discovered public_hoist_packages which seems to be the @aspect_rules_js version of hoisting. I was a bit confused by the usage but I think the right answer is:

npm_translate_lock(
    # ...

    # Links `debug` to the "" (root) Bazel package, so it becomes available at `//:node_modules/debug`.
    public_hoist_packages = {
        "debug@4.3.1": [""],
    },
)

Then I can add //:node_modules/debug as a dependency and WebDriverIO works. Not totally sure if this is easier than hooks.readPackage, but good enough for me.

[dgp1130]

After that the @aspect_rules_js Jasmine migration is fairly straightforward. I need to remove @bazel/runfiles dependencies because it complains about $BAZEL_WORKSPACE not being set. I don't fully understand why that doesn't work, but it's not necessary anymore since everything is in a single execution tree, so instead of fs.readFile(runfiles.resolve('foo/bar/baz.js')) we can just do fs.readFile('foo/bar/baz.js').

One challenge is that //examples/site/... has some tests which directly import prerender JS as part of their tests in the same process, meaning that the rules_prerender NPM package needs to be linked in an @aspect_rules_js compatible manner. This pulls on directory layout issues, since rules_prerender depends on //common/... which isn't under //packages/rules_prerender/....

Solution is to move the npm_package() target to the workspace root so everything is a subpackage. This makes the NPM package structure a little weird, but it is able to resolve ../../common/... imports as expected.

I also found that npm_package() only pulls direct source files of a js_library() target, presumably because they are the only ones in DefaultInfo. I wrote a short rule to collect transitive sources and return them in DefaultInfo to feed into npm_package(), which fixed that issue.

With that, I chunked off the Jasmine migration into its own branch, cleaned up the commits, and merged them. I figure that's a safe start to the migration since it's internal changes to testing which don't actually affect the built artifacts.

[dgp1130]

CI failed after merging because apparently @aspect_rules_js breaks the --incompatible_config_setting_private_default_visibility flag. Sent https://github.com/aspect-build/rules_js/pull/301 to fix it and b30bcfa72c0812b3ff4cea4643ee4232b47993d6 removes that flag for now to fix CI.

[dgp1130]

https://github.com/webdriverio/webdriverio/pull/8542 landed but isn't released yet. Should be in v7.20.8 once that is ready.

https://github.com/aspect-build/rules_js/pull/301 also landed but isn't out yet either. Should be in v1.0.0-rc.3.

[dgp1130]

The upstream @aspect_rules_js and WebDriverIO fixes have landed, so I was able to pull them in and remove the workarounds.

Moving forward, I think the next incremental step I can do is take a tool and ship it via js_binary() instead of nodejs_binary(). I'll start with not renderer, since that's the most complicated one. This means that user projects need a workspace dependency on @rules_prerender which they haven't had in the past, so I'll need to figure out that story.

Experimenting with this, I updated the ref/external branch to use @aspect_rules_js and added a local_repository() dependency on @rules_prerender (without breaking the existing @npm//rules_prerender dep). This unfortunately fails because Jasmine isn't in the workspace:

ERROR: /home/doug/Source/rules_prerender/user/app/BUILD.bazel:10:16: every rule of type _multi_extract_annotations implicitly depends upon the target '@rules_prerender//packages/annotation_extractor:annotation_extractor', but this target could not be found because of: error loading package '@rules_prerender//packages/annotation_extractor': at /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/rules_prerender/tools/jasmine.bzl:1:6: at /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/rules_prerender/tools/jasmine_node_test.bzl:4:6: cannot load '@npm_rules_js//:jasmine/package_json.bzl': no such file

I believe the reason for this is because @npm_rules_js is instantiated by the user workspace which does not have a required dependency on Jasmine, yet I load("@npm_rules_js//:jasmine/package_json.bzl", "jasmine_bin") in the annotation extractor package (because its unit tests coexist with the implementation). We're never actually running the npm_translate_lock() from @rules_prerender, so it's package.json and the associated node_modules/ are never installed.

I think the root cause here is that WORKSPACE files aren't transitively loaded, which is the problem bzlmod is supposed to solve, though I haven't looked into it much and don't think it should be necessary to use @rules_prerender, though WORKSPACE files may get a little hacky.

Updating the user WORKSPACE to call a new rules_prerender_dependencies() which instantiates an npm_translate_lock() using a different name than the user's @npm_rules_js workspace I'm able to get the Jasmine error to go away. It then reveals a new error:

ERROR: /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/rules_prerender/packages/annotation_extractor/BUILD.bazel:15:11: no such package '@npm//@types/yargs': BUILD file not found in directory '@types/yargs' of external repository @npm. Add a BUILD file to a directory to mark it as a package. and referenced by '@rules_prerender//packages/annotation_extractor:annotation_extractor_lib'

I think this is the same issue but applied to the @npm workspace instead of the @npm_rules_js workspace. Eventually this dependency will go away entirely, however since I'm migrating only a single tool //common/... is used in both @aspect_rules_js and @build_bazel_rules_nodejs contexts, so I can't just delete these deps and/or migrate the ts_project() targets to @aspect_rules_ts.

I updated the user's WORKSPACE to include an npm_install() of @rules_prerender//:package-lock.json. This again required non-conflicting names for the two @npm workspaces. To add to the confusion, the current implementation of @npm//rules_prerender assumes the user has named it @npm. This means all internal dependencies in the shipped NPM package need to use @user_version_of_npm//rules_prerender/... while any external dependencies need to use @internal_version_of_npm//some_dep/....

After working through those issues, I found that @build_bazel_rules_nodejs requires that all deps come from a single NPM workspace:

ERROR: /home/doug/Source/rules_prerender/user/app/component/BUILD.bazel:3:20: in _ts_project rule //app/component:component_prerender_ts: 
Traceback (most recent call last):
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/npm/@bazel/typescript/index.bzl", line 44, column 42, in _ts_project_impl
                return _ts_project_lib.implementation(ctx, run_node, ExternalNpmPackageInfo)
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/rules_nodejs/nodejs/private/ts_project.bzl", line 169, column 23, in _ts_project_impl
                run_action(
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/providers/node_runtime_deps_info.bzl", line 106, column 51, in run_node
                modules_manifest = write_node_modules_manifest(
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/linker/link_node_modules.bzl", line 124, column 25, in write_node_modules_manifest
                fail("All npm dependencies at the path '%s' must come from a single workspace. Found '%s' and '%s'." % (path, other_workspace, workspace))
Error in fail: All npm dependencies at the path '' must come from a single workspace. Found 'npm' and 'npm_user'.
ERROR: /home/doug/Source/rules_prerender/user/app/component/BUILD.bazel:3:20: Analysis of target '//app/component:component_prerender_ts' failed

In theory, I think everything we need to compile a component should be under @npm_user//.... I found that ts_project() has an implicit dependency on @npm//typescript/bin:tsc, rewriting it to @npm_user//typescript/bin:tsc should make it use the peer dep in the user's project, but this doesn't remove the error. I also don't see any other @npm//... dependencies on that target:

$ (cd user/ && bazel query "somepath(//app/component:component_prerender_ts, @npm//...)" --notool_deps)
INFO: Empty results

Printing out more information, I see that this is actually the renderer which is failing to build because of a dependency on @npm//yargs:

DEBUG: /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/linker/link_node_modules.bzl:118:10: From //app:page_page_annotated_binary with deps: [<merged target @npm//yargs:yargs>, <merged target //app:page_page_component_prerender>, <merged target @npm_user//rules_prerender:rules_prerender__files>]
DEBUG: /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/linker/link_node_modules.bzl:123:18: @npm//yargs:yargs
DEBUG: /home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/linker/link_node_modules.bzl:123:18: //app:page_page_component_prerender
ERROR: /home/doug/Source/rules_prerender/user/app/BUILD.bazel:10:16: in nodejs_binary rule //app:page_page_annotated_binary: 
Traceback (most recent call last):
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/node/node.bzl", line 164, column 56, in _nodejs_binary_impl
                node_modules_manifest = write_node_modules_manifest(ctx, link_workspace_root = ctx.attr.link_workspace_root)
        File "/home/doug/.cache/bazel/_bazel_doug/bae30dce5d5ae268b94024c0d79faa02/external/build_bazel_rules_nodejs/internal/linker/link_node_modules.bzl", line 127, column 25, in write_node_modules_manifest
                fail("All npm dependencies at the path '%s' must come from a single workspace. Found '%s' and '%s'." % (path, other_workspace, workspace))
Error in fail: All npm dependencies at the path '' must come from a single workspace. Found 'npm' and 'npm_user'.
ERROR: /home/doug/Source/rules_prerender/user/app/BUILD.bazel:10:16: Analysis of target '//app:page_page_annotated_binary' failed

Rewriting that to @npm_user//yargs fixes the issue, though it requires the user workspace to have a Yargs dependency, which it really shouldn't need. This is because Yargs is a dependency of the renderer binary, but not a peer dep users should be aware of. The easiest solution for now is to just make Yargs an unofficial peer dep, since its the only offender it's not too big a deal. Maybe depending on two npm_translate_lock() workspaces is fine in @aspect_rules_js, even if it isn't with @build_bazel_rules_nodejs. I'll have to tackle that problem later.

Next error is that my ts_project() targets under //packages/annotation_extractor/... can't build because they are executed under external/rules_prerender/... while the host configuration is actually under bazel-out/host/bin/..., not external/rules_prerender/bazel-out/host/bin/.... Adding ../../ prefix to the tsconfig.json rootDirs option is enough to fix this, though it's a bit awkward. I believe this should go away once we move to @aspect_rules_ts.

That's enough to pass tsconfig validation, however the tsc invocation fails due to missing imports:

$ (cd user/ && bazel build @rules_prerender//packages/annotation_extractor:metadata)
external/rules_prerender/packages/annotation_extractor/metadata.ts(1,37): error TS2307: Cannot find module '../../common/models/prerender_annotation' or its corresponding type declarations.
external/rules_prerender/packages/annotation_extractor/metadata.ts(2,51): error TS2307: Cannot find module '../../common/models/prerender_metadata' or its corresponding type declarations.

Building with --sandbox_debug and looking in the file tree, I see that external/rules_prerender/common/... is missing. Checking the inputs of the action:

$ (cd user/ && bazel aquery "mnemonic(TsProject, @rules_prerender//packages/annotation_extractor:metadata)")
# ...
Inputs: [
    bazel-out/k8-fastbuild/bin/external/rules_prerender/common/models/prerender_annotation.d.ts
    # ...
]

Apparently generated inputs are under bazel-out/${cfg}/bin/external/rules_prerender/..., so they also need to be added to rootPaths.

This is enough to build and extract annotations, however client side bundling fails with the error:

[!] Error: 'rules_prerender/packages/rules_prerender/declarative_shadow_dom/declarative_shadow_dom_polyfill' is imported by bazel-out/k8-fastbuild/bin/app/page_page_scripts.js, but could not be resolved – treating it as an external dependency

I'm a bit surprised by this since I would expect Rollup bundling to be unaffected by this change. It has a dependency on @npm_user//rules_prerender:declarative_shadow_dom, which is the same as the old dependency (not using @npm_rules_js yet). Again a bazel aquery shows:

$ (cd user/ && bazel aquery "mnemonic(Rollup, //app:page_bundle)")
# ...
Inputs: [
external/npm_user/node_modules/rules_prerender/packages/rules_prerender/declarative_shadow_dom/declarative_shadow_dom_polyfill.d.ts,
external/npm_user/node_modules/rules_prerender/packages/rules_prerender/declarative_shadow_dom/declarative_shadow_dom_polyfill.js,
# ...
]

However, I think it's expected that the sources are under external/npm_user/node_modules/rules_prerender/..., this shouldn't have meaningfully changed. I do see other sources under external/npm/node_modules/..., so I wonder if that's confusing @rollup/plugin-node-resolve? Users can bundle their own scripts or other NPM deps under @npm_user/..., so I would expect Rollup to only look in that node_modules/ directory.

Digging through --sandbox_debug, it seems that Rollup is looking under @npm//... since that's where all the Rollup dependencies are (rollup, @rollup/plugin-node-resolve, etc.) It makes sense that those are internal deps which should be hidden from the user's workspace. These are present at node_modules/ in the working directory of the sandbox, while the DSD component is under external/npm_user/node_modules/.... I tried adding that to moduleDirectories in the Rollup config, but this doesn't seem to have any effect.

I tried looking through @rollup/plugin-node-resolve to see if there was a way to log all the paths being attempted. I couldn't find any functionality like that, but I did realize the algorithm is dependent on package.json placement, so I added a dependency from the DSD script onto the package.json so it would be included at external/npm_user/node_modules/rules_prerender/package.json and combined with the moduleDirectories change, this fixes Rollup to bundle successfully.

And with that an external application finally works with an @aspect_rules_js annotation extractor. The most awkward part is that it requires a user project to declare @npm_user and @npm_rules_js_user. This was technically already the case with the @npm workspace, but it's awkward that we now have two of each which need specific names. I could invert the convention so users get @npm and @npm_rules_js, though all internal deps then become something like @rules_prerender_npm and @rules_prerender_npm_rules_js. Maybe I can use keep @npm in @rules_prerender source and use repo_mapping to alias it to some @npm_for_rules_prerender in the user's WORKSPACE. That might work for basic deps, but files will be under external/npm_for_rules_prerender/..., so tools like Rollup need to know that name. Any deps in a macro evaluated in the user workspace will also need to use that name.

The other question is: Do I really care about hard-coding @npm_user? After this migration, @npm and @npm_user will go away. The same problem will exist for @npm_rules_js and @npm_rules_js_user. That hasn't really been an issue yet since only annotation extractor uses @aspect_rules_js and it's an internal tool. However, I suspect that migrating the renderer to @aspect_rules_js will convert all my @npm_user problems into @npm_rules_js_user problems. @aspect_rules_js strategy of putting everything into a root-relative path might alleviate some of the issues (for instance, Rollup doesn't need to know about multiple node_modules/ roots), however that's assuming that it supports depending on multiple npm_translate_lock() targets in the same action. I suspect that @rules_prerender//:node_modules/dep will end up at a path like external/rules_prerender/node_modules/..., so it may still be necessary to configure tools like Rollup to look in the right location.

I'm inclined to just hard-code the workspace names for now (maybe @npm for user deps and @npm_for_rules_prerender for internal deps) and come back to the problem once @npm is dropped altogether.

Current snapshot: ref/rules-js-annotation-extractor-external.

[gregmagolan]

@dgp1130 Heads up that I just landed a big refactor in rules_js that doesn't affect the user facing public API very much but it does affect the API for derivative rules. It adds a common provider (JsInfo) that downstream rules should use, establishes patterns to use and provides helpers for these. The change will go out in rc4 shortly.

You can look at our rulesets for examples of how it affects downstream rules,

https://github.com/aspect-build/rules_esbuild/pull/48 https://github.com/aspect-build/rules_jest/pull/24 https://github.com/aspect-build/rules_ts/pull/113 https://github.com/aspect-build/rules_rollup/pull/26 https://github.com/aspect-build/rules_swc/pull/71 https://github.com/aspect-build/rules_terser/pull/24 https://github.com/aspect-build/rules_webpack/pull/34

LMK if you have a questions or ping me on Bazel slack if you would like some help with the changes required.

[dgp1130]

Thanks @gregmagolan, I'll take a look once that comes out. Is the core change that there's a (new?) JsInfo provider which needs to be manually propagated in some situations and used where DefaultInfo suffices today?

[gregmagolan]

Thanks @gregmagolan, I'll take a look once that comes out. Is the core change that there's a (new?) JsInfo provider which needs to be manually propagated in some situations and used where DefaultInfo suffices today?

In short yes, except the new JsInfo should be propagated by all derivative rules so that they whole ecosystem of rules_js rules play nicely together. Relying on DefaultInfo proved to make it difficult to differentiate between sources & npm dependencies. With DefaultInfo this information plus declarations and npm package stores (when linking npm package stores) is propagated in a consistent way to terminal rules such as npm_package & js_binary / js_test. It made a number of bug fixes and new features possible.

[dgp1130]

Gotcha, I don't have too many custom rules which actually interoperate like a js_library(), so hopefully that won't be too bad. I'll keep that in mind when I bump to rc.4.

[dgp1130]

So I'm finally coming back to this migration and unfortunately I've completely forgotten everything I was doing. There's some good notes in this issue and the associated commits, but everything was half-implemented. I feel like an easier approach might be to start over the migration, upgrade directly to latest @aspect_rules_js instead of going through the prereleases and just those comments / commits as references for re-solving some of these problems.

I upgraded the existing @aspect_rules_js install to the latest version and tried migrating all the tool binaries (except :renderer) to use js_binary(). I was able to do that without too much difficulty. I've still got all the tests passing, though I'm deliberately ignoring external uses, since that would complicate things greatly and I'd rather solve that problem once internal use cases are fully working.

I also noticed that @aspect_rules_jasmine is now a thing, so I updated my bespoke implementation to use that in ed51903dc899c82fcbdfdf4806a0e876f215781e. I opted to leave in the generated config file since that allows each test to limit itself to direct dependencies of the jasmine_test() target, which removes any magic naming conventions.

I then tried to tackle the migration to @aspect_rules_ts and got further than I expected. I was able to build most of the internal packages after adapting most of my rules to be compatible with JsInfo (which I think was Greg's previous comment). Currently I'm stuck on the fact that the @build_bazel_rules_nodejs version of js_library() with package_name isn't compatible with @aspect_rules_ts so I need to migrate it to npm_link_package(). But also, the renderer is still using nodejs_binary(), and isn't compatible with npm_link_package(). I think either I need to convert the renderer to @aspect_rules_js or I need to adapt the @aspect_rules_ts implementation of ts_project() to work with the @build_bazel_rules_nodejs implementation of js_library(). I'm thinking the latter might be good enough to land the TypeScript migration, and then the renderer would be the step after?

[dgp1130]

Regarding how to ship a prerender_component(), I wonder if we could use package.json conditions (or custom conditions) to split the implementation? It's a bit weird to do this since the Node and Browser are two different implementations with two different types (not different formats of the same implementation), but maybe I can find a way to make those work?

[dgp1130]

I tried keeping the js_library(package_name = "rules_prerender") however it doesn't seem to resolve correctly in the renderer binary. I'm able to get it to compile, but it won't load at runtime.

$ bazel build //examples/minimal:page_page_annotated || (cd /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender && \
  exec env - \
  VERBOSE_LOGS=1 bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh --entry-point rules_prerender/examples/minimal/page.js --output-dir bazel-out/k8-fastbuild/bin/examples/minimal/page_page_annotated)
# ...
[page_page_annotated_binary_require_patch.cjs] try to resolve in runfiles /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender
[page_page_annotated_binary_require_patch.cjs] try to resolve in runfiles /home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/npm/node_modules/rules_prerender
Cannot find module 'rules_prerender'. Please verify that the package.json has a valid "main" entry
required in target //examples/minimal:page_page_annotated_binary by '/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender/examples/minimal/page.js'
  looked in:
    built-in, relative, absolute, nested node_modules - Error: Not a built-in module, relative or absolute import
    runfiles - Error: Cannot find module '/home/doug/.cache/bazel/_bazel_doug/0000b3b3fa770637f16a71f2a8299a7e/execroot/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender'

I'm a bit confused by this error given that the package does exist at:

${execroot}/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender/node_modules/rules_prerender/package.json

It seems like the first "try to resolve in runfiles" should work. Tracing through the patched require logic, it seems to expect the package to be outside node_modules/ at:

${execroot}/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender/package.json

I'm not sure whether the package is being written to the wrong place or the resolver is looking in the wrong place. Debugging the same binary at HEAD, I see that it actually resolves the package to:

${execroot}/rules_prerender/bazel-out/host/bin/examples/minimal/page_page_annotated_binary.sh.runfiles/rules_prerender/packages/rules_prerender/index

This is surprising since it seems to be resolving through the rules_prerender/ workspace, not the rules_prerender package. link_workspace_root is not enabled, but --bazel_patch_module_resolver is still included in the renderer binary.

It's also worth noting that at HEAD VERBOSE_LOGS=1 prints out:

MODULE_ROOTS: [
  {
    "module_name": {},
    "module_root": "rules_prerender/packages/rules_prerender"
  }
]

While using @aspect_rules_ts prints out:

MODULE_ROOTS: []

I suspect the nodejs_binary() isn't linking the rules_prerender package because I have to supply it as a data dependency since it isn't compatible with @aspect_rules_ts. That would imply that we can't have an @aspect_rules_ts ts_project() depend on a @build_bazel_rules_nodejs js_library(package_name = "...").

I'm not sure I'll be able to make progress here. I wonder if the better approach might be to migrate the renderer to js_binary() before migrating the whole repository to @aspect_rules_ts?

Trying this out, things went better than expected. I was actually able to get //examples/minimal/... passing relatively easily.

For some reason @bazel/runfiles was throwing errors because ${RUNFILES_DIR} was not set even though ${RUNFILES} was. I just made my own implementation based on that which seemed to work well enough. Renderer tests are also busted since they leverage BAZEL_NODE_RUNFILES_HELPER which doesn't exist anymore. I couldn't find a way of getting those tests to resolve their dependencies correctly, given that they awkwardly skip the linking step. I'm just disabling them for now.

The JavaScript example then broke, which I eventually narrowed down to the fact that prerender_component() with *.js files creates a js_library() target using @build_bazel_rules_nodejs. Switching this to @aspect_rules_js seemed to fix that.

I had to deal with the duplicate rules_prerender definition problem, since the renderer can't relatively import anything underneath it without duplicating the definitions. Instead those symbols need to exposed via export { thing as internalThing } from './internal'; so they can be imported from the package as import { internalThing } from 'rules_prerender';.

The next issue is that declarative shadow DOM isn't able to resolve:

Cannot find module 'rules_prerender/declarative_shadow_dom'

I'll either need to actually embed this in the rules_prerender packages as a real exports path or make a separate @rules_prerender/declarative_shadow_dom package.

[dgp1130]

Of the two options for declarative shadow DOM mentioned previously, I think it is better to convert that component to its own package because it is a component, not a typical NPM package like rules_prerender. You need to depend on it via prerender_component(deps = [...]), whereas rules_prerender can be used in any ts_project(deps = [...]).

I tried introducing a new link_prerender_component() macro which would scaffold the prerender_component() structure based on a single NPM package. Unfortunately @build_bazel_rules_nodejs ts_project() does not seem to support a TreeArtifact in its srcs. Instead, I tried aliasing to the underlying //:node_modules/@rules_prerender/declarative_shadow_dom but this breaks user code which is still using @build_bazel_rules_nodejs ts_project() to depend on @rules_prerender/declarative_shadow_dom.

My takeaway from this is that it is not possible (or at least infeasibly difficult) to migrate to @aspect_rules_js js_binary() for the renderer xor migrate to @aspect_rules_ts independently. It seems like I need to do both for the whole repository in the same commit. I've tried to split out and land as many individual fixes as I can, but I think I'm at the point where I just need to do a big one-off change to get the renderer on @aspect_rules_js js_binary() and move all the ts_project() targets to @aspect_rules_ts.

[dgp1130]

Merging the js_binary() and @aspect_rules_ts changes together I'm able to get the minimal example running again.

The next biggest problem I encountered was that the renderer's dynamic import of the user's entry point wasn't working for some packages. The issue here was that the import() statement actually lived in packages/renderer/dynamic_import.js, meaning imports were relative to that file. I rewrote the generated js_binary() to instead use a synthetic entry point which imported the user module and passed it into the renderer like so:

const { run } = require('.../packages/renderer/renderer');
const mod = require('.../path/to/user/entry/point');

run(mod);

This fixed the file resolution issues and is a bit easier to test as well (no need to mock a dynamic import).

Trying to get @rules_prerender/declarative_shadow_dom to resolve and I'm hitting some headwinds just getting the npm_package() to work independent of link_prerender_component(). There's definitely a hermeticity issue here as I set up a temp package built in the workspace which failed to resolve initially, but was fixed by a bazel clean.

I spent way more time than I should have trying to figure out why @rules_prerender/declarative_shadow_dom wasn't resolving only to find that I needed an index.js file (or main in the package.json). The error message definitely implied that the package wasn't found, not that an entry point was missing, so that threw me off for a while but was an easy fix once I figured it out.

After that getting link_prerender_component() to work minimally was relatively straightforward. I just stubbed the different subtargets of prerender_component() as aliases of the underlying //:node_modules/@rules_prerender/declarative_shadow_dom. This is overly broad and not a great implementation, but good enough to prove the concept. I think the correct solution is to split the package into multiple TreeArtifacts based on metadata in the package.json about which files belong in which "slice" of the prerender_component(). That'll take some refactoring to support though for sure.

This does present one interesting challenge in that @rules_prerender/declarative_shadow_dom needs to call includeScript() on a package-local script, which I hadn't really tried before. I got it working with includeScript('node_modules/@rules_prerender/declarative_shadow_dom/...') which is definitely a bit hacky. I think importing the script with an import assertion would be the right approach here, but I haven't gotten the chance to fully explore it.

I was actually able to get bazel test //... to pass with the exception of one target checking the outputs of prerender_component_publish_files(). Apparently switching that to @aspect_rules_ts dropped the transitive dependencies of the script ts_project() target. I'm not sure what's going on there, the transitive dependencies are present when inspected with bazel-pquery, but not present when inspected in the JsInfo provider within a rule. I'm a bit confused by that one, but will have to come back to it later.

I think the next step is to consolidate the temporary work I have and actually land it on main. Once that's done, the next step will be fixing external repositories.

[dgp1130]

The transitive dependency issue seemed to be because I had a typo in 42d66995b6a5b03eb4ac8a2deefc15a2de91d9a5 and accidentally re-exported declarations as transitive_declarations. Fixed this in eb08bbb787652585201a2f6e07f00c94752cb9ef.

With that bazel test //... is finally passing. I have done what I can to split and merge as many changes as possible without breaking existing tests. There's a couple more things I can split out and land early, but currently the rules-js branch accepts that the first commit breaks CI as long the last commit fixes it. I think that should be good enough to get on @aspect_rules_js.

[gregmagolan]

@alexeagle and I are offsite this week. I'll have a more detailed read of this when we are back. Is there anything specific I can help with or look at when we're back mid-next week?

[dgp1130]

Thanks for checking in @gregmagolan. I think I'm ok right now, most of these comments are just notes to myself for future reference. I think I can land these changes and the get the workspace into a stable state. The next problem I'll have to deal with is getting an external workspace to depend on these rules correctly, which I haven't experimented with yet. I have some ideas based on our previous discussion and I might have more questions for you two after trying it out.

[dgp1130]

Landed all the breaking changes and got the repository back into a consistent state. We're almost entirely running @aspect_rules_js and related rules. I forgot to ref this issue in the relevant commits, but it's this range: https://github.com/dgp1130/rules_prerender/compare/d91ac589c1da1bb872a66b4c0fe426c254a314a3...7ee4aed4a2d32cb96c3004cbbefe14702f981669

I took a quick divergence trying to update concatjs_devserver() to js_run_devserver(). The annoying part here is that js_run_devserver() uses a plain Bazel executable with the built-in args implementation. This is challenging, because we use the devserver in web_resources_devserver(), where the directory to serve is always constant, and the path is given via args. Yet $RUNFILES/path/to/my_devserver --port 1234 will clobber that path and make it serve the wrong directory. Aside from being unergonomic, it actually breaks all the e2e tests, since they use the devserver for hosting the application under test and need to provide a known port for the devserver to run on.

Solved this by adding a _baked_binary() which "bakes" in the arguments directly into the binary so they aren't lost when executed from runfiles. Also filed https://github.com/aspect-build/bazel-lib/issues/356 to see if we could upstream a more generic implementation of this rule, since it seems useful in other contexts.

After migrating to js_run_devserver(), I was able to delete the dependency on @bazel/concatjs, so we're a little bit closer to the finish now. I think the only @build_bazel_rules_nodejs still used is in @bazel/postcss (lots of vestigial code lying around, but I don't think any of it is used). I could try to migrate that, though part of the motivation of migrating to @aspect_rules_js is to switch this out with Parcel. I'm inclined to jump directly to Parcel instead of trying to get PostCSS working with @aspect_rules_js. For now, I suspect bundle_css = False will skip all the PostCSS stuff broadly enough that I don't actually need to get it working. So I think the next step is to try to get rules_prerender working from an external workspace.

[dgp1130]

Took a pass at running rules_prerender inside an external repository and encountered more immediate issues than I expected. I started with a new workspace which added rules_prerender via local_repository() and set up @aspect_rules_js. I added a web_resources() target (since that seems like the simplest) and just started working through errors.

Previously I only had a single package.bzl file which set up workspace-level dependencies. However, it doesn't seem possible to do this in a single macro, since you can't trigger a load() statement outside the top-level scope of a *.bzl file, meaning it's not possible to:

def rules_prerender_dependencies():
    http_archive(
        name = "aspect_rules_rollup",
        sha256 = "e0c1f17fccdeedb40b6864ee0708c8e2a9237b46dd97b596e1ba264483d63a7f",
        strip_prefix = "rules_rollup-0.12.5",
        url = "https://github.com/aspect-build/rules_rollup/archive/refs/tags/v0.12.5.tar.gz",
    )
    load("@aspect_rules_rollup//rollup:dependencies.bzl", "rules_rollup_dependencies")
    rules_rollup_dependencies()
    load("@aspect_rules_rollup//rollup:repositories.bzl", "rollup_repositories")
    rollup_repositories(name = "rollup")
    load("@rollup//:npm_repositories.bzl", rollup_npm_repositories = "npm_repositories")
    rollup_npm_repositories()

Instead, the load() statements need to be in the user's WORKSPACE, or at the top-level scope of the macro. Yet timing doesn't work that way, since @aspect_rules_rollup doesn't exist until the http_archive() call and @rollup doesn't exist until rollup_repositories().

Instead I did my best to just follow that convention with rules_prerender and came up with:

local_repository(
    name = "rules_prerender",
    path = "../..",
)
load("@rules_prerender//:dependencies.bzl", "rules_prerender_dependencies")
rules_prerender_dependencies()
load("@rules_prerender//:repositories.bzl", "rules_prerender_repositories")
rules_prerender_repositories()
load("@rules_prerender//:npm_repositories.bzl", prerender_npm_repositories = "npm_repositories")
prerender_npm_repositories()

It's a bit verbose but provides enough macros at different timings to load dependencies correctly. bzlmod could probably address a lot of this, but I don't want to bite off that complexity just yet.

This is enough to get past the workspace, though the web_resources() target fails tsconfig validation with:

$ (cd examples/external/ && bazel build //:resources)
# ...
/home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/sandbox/linux-sandbox/53/execroot/rules_prerender_external/bazel-out/k8-opt-exec-2B5CBBC6/bin/external/npm_typescript/validator.sh.runfiles/npm_typescript/ts_project_options_validator.js Error: ../rules_prerender/tsconfig.json:error TS5083: Cannot read file '../rules_prerender/tsconfig.json'.

The @rules_prerender//:tsconfig.json file is definitely depended upon, but looks like it's under external/.

$ (cd examples/external/ && bazel aquery @rules_prerender//packages/resource_packager:_validate_packager_options)
action 'TsValidateOptions external/rules_prerender/packages/resource_packager/_validate_packager_options.optionsvalid.d.ts'
  Mnemonic: TsValidateOptions
  Target: @rules_prerender//packages/resource_packager:_validate_packager_options
  Configuration: k8-fastbuild
  Execution platform: @local_config_platform//:host
  ActionKey: 4f6715ff0c7ee93e2a89b08def7769b2df1403870ec6fb431913ad38c2f96548
  Inputs: [bazel-out/k8-fastbuild/bin/external/rules_prerender/tsconfig.json, bazel-out/k8-opt-exec-2B5CBBC6/bin/external/npm_typescript/validator.sh, bazel-out/k8-opt-exec-2B5CBBC6/internal/_middlemen/external_Snpm_Utypescript_Svalidator.sh-runfiles]
  Outputs: [bazel-out/k8-fastbuild/bin/external/rules_prerender/packages/resource_packager/_validate_packager_options.optionsvalid.d.ts]
  Environment: [BAZEL_BINDIR=bazel-out/k8-fastbuild/bin]
  Command Line: (exec bazel-out/k8-opt-exec-2B5CBBC6/bin/external/npm_typescript/validator.sh \
    ../rules_prerender/tsconfig.json \
    ../rules_prerender/packages/resource_packager/_validate_packager_options.optionsvalid.d.ts \
    //packages/resource_packager:packager \
    '{"allow_js":false,"composite":false,"declaration":true,"declaration_map":false,"emit_declaration_only":false,"incremental":false,"preserve_jsx":false,"resolve_json_module":false,"source_map":true,"ts_build_info_file":""}')
# Configuration: 0d5d9e70adcf8d05979358e95d9da15caefc5e7f442fbbabfbbf33eb288fb81f
# Execution platform: @local_config_platform//:host

Note that bazel-out/k8-fastbuild/bin/external/rules_prerender/tsconfig.json is an input, so I feel like this should resolve, though I'm not 100% on where it's resolving from. It's using ts.readConfigFile(), but I don't know what that's relative to and TypeScript docs are pretty scarce. The working directory is listed as /home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/sandbox/linux-sandbox/71/execroot/rules_prerender_external as indicated by --sandbox_debug, but the validator target seems to be a js_binary(), so it should be executed in the output tree. I'm not sure how js_binary() works with external dependencies and if this is a bug in that or if I'm using it wrong somehow?

I suspect it is this bug in @aspect_rules_ts, as ts_project() apparently can't build in external repositories. I was even able to reproduce with a trivial workspace executing a binary from another workspace implemented as a js_binary() backed by a ts_project() which this fails with the same error. Commenters suggest a couple patches though I'm having trouble getting those to work with http_archive() at the moment and need to step away. I'll have to see if I can get one of those patches working and (ideally) get the fix landed upstream.

[dgp1130]

Applying the patch to @aspect_rules_ts seems to fix the bug, can probably turn that into a PR for that repository, but let's stay focused on rules_prerender for a moment. That's apparently sufficient to run web_resources() and generate a directory:

$ (cd examples/external/ && bazel build //:resources && tree dist/bin/resources/)
# ...
dist/bin/resources/
└── index.html

I was actually able to build a prerender_component() fairly easily too. prerender_pages() is where I started encountering issues.

First, as expected all the labels in any macros refer to the user's workspace, not @rules_prerender. I worked around this by just converting any //path/to/some:target to @rules_prerender//path/to/some:target. This works inside the @rules_prerender workspace, since it's just an absolute label to itself, and also works in user workspaces as long as they name the dependency @rules_prerender. Ideally they shouldn't have that restriction, but let's keep it simple for now.

Second, I need to link the renderer binary against the user's //:node_modules/rules_prerender dependency. Currently @rules_prerender//packages/renderer has a direct dependency on @rules_prerender//:node_modules/rules_prerender which I refactored to accept as an input to break that dependency.

// packages/renderer/renderer.ts

import type * as RulesPrerender from 'rules_prerender';

export function createRenderer(rulesPrerender: typeof RulesPrerender, /* ... */) {
  // ...
}

I also pre-built the rules_prerender package and linked it into the user workspace by installing it as a tarball.

$ bazel build //:rules_prerender_pkg && npm pack dist/bin/rules_prerender_pkg && (cd examples/external/ && pnpm install ../../rules_prerender-*.tgz)

This approach still requires that @rules_prerender//packages/renderer has a build-time dependency on @rules_prerender//:node_modules/rules_prerender, but that should be ok, we just need to make sure to drop the JavaScript files in JsInfo from the dependency so it doesn't conflict with the user's //:node_modules/rules_prerender at runtime. However, before I even hit that problem the file fails to build, but only when triggered from the external workspace:

$ bazel build //packages/renderer
Target //packages/renderer:renderer up-to-date:
  dist/bin/packages/renderer/renderer.js
  dist/bin/packages/renderer/renderer.js.map

$ (cd examples/external/ && bazel build @rules_prerender//packages/renderer)
ERROR: /home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/external/rules_prerender/packages/renderer/BUILD.bazel:13:11: Compiling TypeScript project @rules_prerender//packages/renderer:renderer [tsc -p external/rules_prerender/tsconfig.json] failed: (Exit 1): tsc_worker.sh failed: error executing command bazel-out/k8-opt-exec-2B5CBBC6/bin/external/npm_typescript/tsc_worker.sh @bazel-out/k8-fastbuild/bin/external/rules_prerender/packages/renderer/renderer.js-0.params

external/rules_prerender/packages/renderer/renderer.ts(3,38): error TS2307: Cannot find module 'rules_prerender' or its corresponding type declarations.
external/rules_prerender/packages/renderer/renderer.ts(106,17): error TS2571: Object is of type 'unknown'.
external/rules_prerender/packages/renderer/renderer.ts(108,3): error TS2571: Object is of type 'unknown'.

My guess is that depending on an npm_link_workspace() target (@rules_prerender//:node_modules/rules_prerender) doesn't work when it's used in an external workspace, but I'm not totally sure on the specifics. It seems to support other dependencies like @rules_prerender//:node_modules/yargs. The only interesting thing about this dependency is that it is built at HEAD from source with npm_package() whereas the other dependencies are pulled in from NPM. For now I just dropped the dependency and duck-typed the few usages of it which should be good enough for the moment. I'll have to come back to this one.

The next error is that node_modules/rules_prerender can't be copied:

$ (cd examples/external/ && bazel build //:site --verbose_failures)
ERROR: /home/doug/Source/rules_prerender/examples/external/BUILD.bazel:7:16: Copying file node_modules/rules_prerender [for host] failed: (Exit 1): bash failed: error executing command 
  (cd /home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/execroot/rules_prerender_external && \
  exec env - \
    PATH=/bin:/usr/bin:/usr/local/bin \
  /bin/bash -c 'cp -f "$1" "$2"' '' node_modules/rules_prerender bazel-out/host/bin/node_modules/rules_prerender)
# Configuration: a6c5a64e62b34ec861b901fb7f8bbde442f413c36c4fbf7360b4ed3284daaea5
# Execution platform: @local_config_platform//:host
cp: -r not specified; omitting directory 'node_modules/rules_prerender'
Target //:site failed to build

Unfortunately I have no idea where this action is coming from. There's no mnemonic or progress message or even a subtarget. Maybe it's a copy_to_bin()? I'm able to limit the error to only occur if I build the renderer binary. Querying for actions does show the copy action in question:

$ (cd examples/external/ && bazel aquery "mnemonic(CopyFile, //:site_page_annotated_binary)")
action 'Copying file node_modules/rules_prerender'
  Mnemonic: CopyFile
  Target: //:site_page_annotated_binary
  Configuration: k8-fastbuild
  Execution platform: @local_config_platform//:host
  ActionKey: 5ca9dd845b6ea4ff2d63b98ef2c3a5e742c975d13b8a5f98c9e3823c6e2657b0
  Inputs: [node_modules/rules_prerender]
  Outputs: [bazel-out/k8-fastbuild/bin/node_modules/rules_prerender]
  Environment: [PATH=/bin:/usr/bin:/usr/local/bin]
  ExecutionInfo: {local: 1, no-cache: 1, no-remote: 1, no-remote-cache: 1, no-remote-exec: 1, no-sandbox: 1}
  Command Line: (exec /bin/bash \
    -c \
    'cp -f "$1" "$2"' \
    '' \
    node_modules/rules_prerender \
    bazel-out/k8-fastbuild/bin/node_modules/rules_prerender)
# Configuration: 311c3b9159e97b4c6ad54f0f4885a1e0ce733dd8248709573d136eecd8478ea4
# Execution platform: @local_config_platform//:host
  ExecutionInfo: {local: 1, no-cache: 1, no-remote: 1, no-remote-cache: 1, no-remote-exec: 1, no-sandbox: 1}

Interestingly, the //:node_modules/rules_prerender target doesn't seem to actually have a directory in it:

$ (cd examples/external/ && bazel build //:node_modules/rules_prerender)
INFO: Invocation ID: f8ed3548-8d8b-4bd4-b1a2-24f91ba3e667
INFO: Analyzed target //:node_modules/rules_prerender (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
INFO: Elapsed time: 0.119s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

# Compared to any other dependency:

$ (cd examples/external/ && bazel build //:node_modules/@bazel/postcss)
INFO: Invocation ID: fecf9df5-344f-49aa-b498-63af7b23b9cd
INFO: Analyzed target //:node_modules/@bazel/postcss (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:node_modules/@bazel/postcss up-to-date:
  dist/bin/node_modules/@bazel/postcss
INFO: Elapsed time: 0.120s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

In fact if I query //:node_modules/rules_prerender I get nothing. I've never actually seen that before, didn't know it was possible for Bazel to create a target which doesn't print a rule?

$ (cd examples/external/ && bazel query --output build //:node_modules/rules_prerender)
INFO: Invocation ID: 31c43237-4d32-4938-bfe9-02031bbe6b99
Loading: 0 packages loaded

$ (cd examples/external/ && bazel aquery //:node_modules/rules_prerender)
INFO: Invocation ID: 95874a4c-e99e-4218-ba1c-d707ae8f8345
INFO: Analyzed target //:node_modules/rules_prerender (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
INFO: Elapsed time: 0.103s
INFO: 0 processes.
INFO: Build completed successfully, 0 total actions

This might not be related, but maybe I'm installing the package wrong somehow? examples/external/node_modules/rules_prerender/ has the files I expect, so PNPM seems happy, but I guess Bazel isn't picking those up correctly? I tried dropping the tarball altogether and just pnpm install rules_prerender. It's a bit out of date, but I don't think my current changes have affected things too much, so in theory it should still work. Yet even with that state I get the same error and same querying behavior.

I just happened to notice that I was using the wrong npm_link_all_packages(). I was loading from @npm_rules_js//:defs.bzl which was defined by @rules_prerender, whereas the actual label should have been @npm_user//:defs.bzl which was used in the user's WORKSPACE file. Replacing that fixed the above behavior and I'm able to build the renderer binary successfully.

Running the binary fails with:

Error: Cannot find module '../common/binary'

But that sounds like tomorrow's problem. 😴

[dgp1130]

Looking at the error a little closer I see:

Error: Cannot find module '../common/binary'
Require stack:
- /home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/sandbox/linux-sandbox/1/execroot/rules_prerender_external/bazel-out/host/bin/site_page_annotated_binary.sh.runfiles/rules_prerender_external/site_page_annotated_binary_entry.js

The import is coming from rules_prerender_external/site_page_annotated_binary_entry.js. Since the file is generated in the user's workspace, a relative path doesn't really resolve. If we go up one more level and into the right workspace, the import does work:

const { main } = require('../rules_prerender/common/binary');
// ...

I'm not sure that's a great idea, but the alternative would be to create an NPM package, link it, and then import that. I'm not sure if we have to link in the user's workspace or if it could be linked in @rules_prerender and then just create an implicit dependency in the renderer binary. Updated the paths to include the workspace name which seems good enough to fix the immediate problem.

I then got some error about failing to resolve a Rollup plugin, but skipping that with bundle_js = False was able to successfully render! I was even able to serve it with web_resources_devserver() without issue. 😁

I consolidated a few of the changes and landed them in advance. I'm also working on a PR for https://github.com/aspect-build/rules_ts/issues/284.

[dgp1130]

I put together a fix for building external ts_project() targets in https://github.com/aspect-build/rules_ts/pull/310.

Beyond that, just landed a few more fixes and improvements. Next step is to land the external example and get it to use the locally built rules_prerender package instead of the manually built, packed, and linked version I've been hacking things together with. After that, I can start looking at bundling client-side JS and CSS and resolving those problems.

[dgp1130]

I'm trying to get the external workspace test case to use the HEAD build of the rules_prerender NPM package by linking it against the npm_package() target. that's what I do within the @rules_prerender workspace, but attempting to do the same from the external workspace fails to resolve rules_prerender in a ts_project() tracing back a bit I see:

$ bazel build //:rules_prerender_pkg && tree dist/bin/rules_prerender_pkg/
dist/bin/rules_prerender_pkg/
├── common
├── package.json
└── packages
    └── rules_prerender
# ...

$ (cd examples/external/ && bazel build @rules_prerender//:rules_prerender_pkg && tree dist/bin/external/rules_prerender/rules_prerender_pkg/)
dist/bin/external/rules_prerender/rules_prerender_pkg/

0 directories, 0 files

This looks very similar to observed behavior in https://github.com/aspect-build/rules_ts/issues/284, and I wonder if npm_package() has a similar bug? I was able to reproduce this behavior in a minimal test case in the @rules_js repository.

Digging further, it seems that npm_package() (due to the underlying copy_to_directory_bin_action()) only copies files from workspaces explicitly listed in include_external_repositories. The "default" repository is always the main one, even when the target is built in an external repository, so it would never include any files! The workaround is to put include_external_repositories = ["rules_prerender"] which isn't ideal and forces consumers to install @rules_prerender at that specific name. This just seems like a bug and should definitely be fixed upstream. I'll follow up to see if I can make that happen.

[dgp1130]

For now, I've managed to land an example which uses rules_prerender from an external workspace. It doesn't use scripts, styles, or published components, but it's a good start.

[dgp1130]

I filed https://github.com/aspect-build/bazel-lib/issues/359 to fix the copy_to_directory_bin_action() issue for external repositories and proposed a PR (https://github.com/aspect-build/bazel-lib/pull/360) to fix it. Unfortunately the tool it modifies seems to be released independently of the workspace, so I'm not totally sure how to move forward there. Hopefully there will be a straightforward path to validating and releasing the change.

Refocusing on rules_prerender, I removed bundle_js = False from the external example to trigger Rollup to compile client side sources. This immediately hit a module resolution error:

[!] Error: Cannot find module '@rollup/plugin-node-resolve'

I've already updated to use @aspect_rules_rollup and //:node_modules/@rollup/plugin-node-resolve, so I think the migration is valid and it should work. Inspecting the output tree I see:

.../execroot/external/bazel-out/k8-fastbuild/bin/
├── external
│   └── rules_prerender
│       └── node_modules
│           └── @rollup
│               └── plugin-node-resolve
├── site_bundle_config.js
└── site_page_scripts.js

So @rollup/plugin-node-resolve is present, but it's under external/rules_prerender/node_modules/, not node_modules/. This is because I'm using a Label("//:node_modules/@rollup/plugin-node-resolve") dependency to keep it relative to @rules_prerender itself, not the user's workspace. This is because Rollup is intentionally being kept as an implementation detail, hidden from userspace. If I install @rollup/plugin-node-resolve in the user's workspace and depend on that version instead, Rollup runs successfully.

I'm not sure about the best approach here. I don't think there is an easy solution since Bazel deliberately restricts actions from writing outside their package, so I don't think it's possible for anything under @rules_prerender to output anywhere other than the external/rules_prerender/ tree, meaning node_modules/ can't be hoisted any higher. The only solution I can see is to put @rollup/plugin-node-resolve into the user's workspace. I could hide this a bit with something like link_prerender_dependencies() which users need to run in their root BUILD file, but it could be used as a dependency by users and potentially conflict with their own installation of @rollup/plugin-node-resolve.

Ultimately I wonder if this is an instance of me fighting against Bazel's recommended one version rule and I should consider making Rollup a "peer dependency" to use the NPM terminology. Personally I don't like that because Rollup should be a pure implementation detail of rules_prerender, completely unobservable from the public API. I posted in the Bazel slack to see if anyone else has thoughts about the best way to approach this, though my guess is that it's going to be in the user's workspace in some capacity. Maybe bzlmod helps here, but I'm guessing it also doesn't solve the "can't write to other packages" restriction.

https://bazelbuild.slack.com/archives/CEZUUKQ6P/p1676086191512569

For the moment I'll work around by installing in the user's workspace. We'll see what others have to say.

[dgp1130]

I added an actual script to the example and found it bundled successfully, though I noticed two interesting things.

First, includeScript('script') fails to resolve script.js. However includeScript('./script') works. No other script includes use the leading ./. The actual generated Rollup entry point is:

import 'script';

And following Node module resolution, that would be treated as an NPM package, not a relative path. So I can understand why Rollup doesn't resolve that, though I'm surprised that it was an issue with other paths. Eventually I realized the script entry generator adds ../ paths depending on the depth of the prerender_pages() target. In this case the depth was 0, so it didn't prepend anything. I updated it to prepend ./, though arguably this should be authored by the user. I'm not going to worry about it right now, but we should take another look at this in #2.

Second, source maps don't seem to be working anymore. Even in non-external examples sourcemaps don't resolve correctly. It looks like TypeScript and Rollup aren't composing each other correctly given that the JS source files also seem to contain a source map reference at the end. They also reference files like __st_outdir__/script.js.map, and I have no idea where that prefix is coming from. It's not that big a deal for now, but I'll need to debug that eventually.

Otherwise I was able to get client side scripts bundling and executing as expected. Next step is styles and then declarative shadow DOM to tie them all together.

[dgp1130]

Started looking into CSS in the external example and was able to get it working relatively easily. Needed to add Label() to a dep to get it to resolve correctly. The only other issue is that users need to install postcss-import via the @build_bazel_rules_nodejs toolchain. The import shouldn't be required and the separate JS toolchain shouldn't be needed either. This doesn't bother me too much right now, because we already have a plan to switch to Parcel in https://github.com/dgp1130/rules_prerender/issues/46 after @aspect_rules_js has landed.

I also added an image resource to the external site which worked as expected without any issues. Technically prerender_pages() emits a WebResourceInfo so we were already going through the same code paths, but good to see that the actual use case works.

While I was working on that, https://github.com/aspect-build/rules_ts/pull/310 landed with the fix for building ts_project() targets in external workspaces. I upgraded all the @aspect_rules_* dependencies to accommodate and was able to drop my local patch. 😁

The final problem is getting declarative shadow DOM working. I started by adding a npm_link_package() for @rules_prerender/declarative_shadow_dom. Real applications would install from NPM, but I'd like to do this so the test case is always built at HEAD. Unfortunately this means I need to build the DSD prerender_component() within an external workspace, meaning I'll have to work through similar problems to those I've discovered in Aspect repositories.

Immediate error is that merging resources fails due to a bad file path:

ERROR: /home/doug/.cache/bazel/_bazel_doug/8a09637a7a02e007611e1aa784ebe6a2/external/rules_prerender/packages/rules_prerender/declarative_shadow_dom/BUILD.bazel:35:20: Merging resources (@rules_prerender//packages/rules_prerender/declarative_shadow_dom:declarative_shadow_dom_resources) failed: (Exit 1): resource_packager.sh failed: error executing command bazel-out/host/bin/external/rules_prerender/packages/resource_packager/resource_packager.sh --merge-dir ... (remaining 3 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Error: ENOENT: no such file or directory, scandir '../rules_prerender/packages/rules_prerender/declarative_shadow_dom/declarative_shadow_dom_resources_entries'
Target //:node_modules/@rules_prerender/declarative_shadow_dom failed to build

Ultimately this is because web_resources() uses file.short_path to get relative paths to all the files referenced. However, this doesn't quite work for external files, because file.short_path is actually ../my_external_wksp/path/to/file.txt, yet external files actually live in external/my_external_wksp/path/to/file.txt. Basically, we can't use file.short_path directly and instead need to do a little more processing. Fortunately in https://github.com/aspect-build/rules_ts/pull/310, Greg pointed out that to_output_relative_path() exists, which pretty much does exactly the behavior I need. Replacing usages of file.short_path with to_output_relative_path(ctx, file) fixes all the relevant issues. Just had to make sure to call it on the files copied to bin, not the original sources.

After landing that fix, I was able to build @rules_prerender/declarative_shadow_dom in an external workspace and then link it into the example workspace. This actually worked out better than expected. link_prerender_component() basically just worked without issue. The only hiccup was that I needed to work around https://github.com/aspect-build/bazel-lib/issues/359/ much like the rules_prerender package, but that's an easy fix. Otherwise I was able to depend on the DSD component and include a client-side script from it correctly.

I think this is actually in a good enough state to release right now. There's definitely a lot of cleanup and refactoring which can be done, but it's good enough to be usable, even if set up is significantly more complicated than it should be right now. I do need to figure out the release process, given that it's a bit more complicated now. Previously I would just release the rules_prerender NPM package which contained the core runtime, DSD component, and all the Starlark rules. Now we need to release the workspace source on GitHub combined with publishing the rules_prerender and @rules_prerender/declarative_shadow_dom packages (the latter of which doesn't actually exist right now).

The first problem there is stamping the packages, which I've ignored so far. I tried to use stamped_package_json() but found it insufficient because @rules_prerender/declarative_shadow_dom has a peer dependency on rules_prerender which it does not stamp. I filed https://github.com/aspect-build/rules_js/issues/866/ to consider updating it to support stamping dependencies, but for now I just made my own version on top of the existing jq support.

Next I'll need to reevaluate the release action to publish both NPM packages simultaneously. Ideally it would also create the GitHub release and include the instructions for how to set up the WORKSPACE.bazel file, similar to how a number of Bazel projects do things. I'll need to see how they generate those instructions. The other problem is that I never got around to generating release notes from the changelog and always created GitHub releases manually. I can still add the changelog afterwards for now, but if tooling will generate the release, it would be cool if it could generate the changelog as well.

[dgp1130]

Unfortunately it doesn't seem that publishing NPM packages is directly supported by @aspect_rules_js (see https://github.com/aspect-build/rules_js/issues/155).

In theory I don't think it should be that hard, given that we already have an NPM binary and the GitHub action includes authentication. I think all we need is an sh_binary() with data dependencies on the npm_package() directory, the .npmrc, and the NPM binary, then just does (cd path/to/npm/pkg/ && cp ../../../.npmrc/ . && npm publish).

I developed my own npm_publish() macro which seems to work reasonably well. I did a test release (0.0.16) and successfully published rules_prerender and @rules_prerender/declarative_shadow_dom. I messed up the tar files as assets in the Github archive, but the 0.0.17 release seems to have fix that. I updated the release notes with some general information and confirmed via http_archive() that I can depend on and use @rules_prerender as expected with rules_prerender and @rules_prerender/declarative_shadow_dom from NPM. But the important part here is that @rules_prerender is usable with @aspect_rules_js (with a lot of caveats). 🎉

For now, I generated the release notes by hand, this should be integrated with the release process. I found @aspect_rules_js' implementation here, where they tar their own repository and push it as an assert. We should probably do something similar, which would provide a more consistent prefix in the tar and generate the correct hash up front.

I think this issue has gone on long enough, and we've officially reached the point where I was able to successfully release against @aspect_rules_js. There's definitely still a lot of work and cleanup to do to get to a good place, but I think those can follow up separately. I'll look over the previous comments and any submitted TODOs for actions items and file issues for those, then we can probably close this one.

[dgp1130]

I resolved a handful of simple TODOs left over from this issue and also looked through a bunch of the above comments for follow-up tasks. I filed issues where they didn't already exist. A mostly-comprehensive list of them includes:

• https://github.com/dgp1130/rules_prerender/issues/46
• https://github.com/dgp1130/rules_prerender/issues/63
• https://github.com/dgp1130/rules_prerender/issues/62
• https://github.com/dgp1130/rules_prerender/issues/64
• https://github.com/dgp1130/rules_prerender/issues/65
• https://github.com/dgp1130/rules_prerender/issues/66
• https://github.com/dgp1130/rules_prerender/issues/55
• https://github.com/dgp1130/rules_prerender/issues/56
• https://github.com/dgp1130/rules_prerender/issues/68
• https://github.com/dgp1130/rules_prerender/issues/57
• https://github.com/dgp1130/rules_prerender/issues/58
• https://github.com/dgp1130/rules_prerender/issues/59
• https://github.com/dgp1130/rules_prerender/issues/60
• https://github.com/dgp1130/rules_prerender/issues/61
• https://github.com/dgp1130/rules_prerender/issues/67

I also filed https://github.com/aspect-build/rules_ts/issues/319 with a types_only() feature request which I found need for. For now I just have my own implementation which is good enough, but it would be cool to upstream.

I think this is enough for one issue. I'll follow up on those for the remaining tasks.

[alexeagle]

Congrats on landing, sorry I never had time to read all the text :)