build: improve ts_project hybrid interop with node modules

Currently the interop resulting target of a `ts_project` ends up not
necessarily working at runtime. This may be the case because a consuming
Node program may end up with mixes of `node_modules` dependencies from
`rules_nodejs` (old) and `rules_js` (new). This sounds fine at first
glance, but in practice can break very subtly because:

* Rules NodeJS leverages the linker, creating `node_module` directories
  outside of Bazel, at runtime. These don't depend on symlink resolving.
* Rules JS puts real node module folders via Bazel actions. These rely
  on `pnpm` non-hoisting layout, and symlink resolving.

As we can see there is a hard conflict with symlinks. They need to be
enabled with the new toolchain, but the other one doesn't enable symlink
resolution, and enabling is not possible as we'd otherwise risk escaping
the sandbox and cause even more subtle errors.

A good compromise solution is to automatically drop the `rules_js` node
module files/folder in the interop-`rules_nodejs` target and instead
brining in the equivalent `@npm//` dependencies from `rules_nodejs`.
This kind of keeps the logic similar to when not using `rules_js` or the
interop, and enables the simplest & safest mental model; and it works
compared to other solutions I tried with symlinking. Notably, we can't
keep both node module variants as the linker doesn't override existing
node module files from e.g. rules_js then (and would break then).

Author: devversion

packages/angular_devkit/build_angular/BUILD.bazel

@@ -6,6 +6,7 @@
 load("@npm//@angular/build-tooling/bazel/api-golden:index.bzl", "api_golden_test_npm_package")
 load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
 load("//tools:defaults.bzl", "pkg_npm", "ts_library")
+load("//tools:interop.bzl", "ts_project")
 load("//tools:ts_json_schema.bzl", "ts_json_schema")
 
 licenses(["notice"])
@@ -77,9 +78,8 @@ ts_json_schema(
     src = "src/builders/web-test-runner/schema.json",
 )
 
-ts_library(
+ts_project(
     name = "build_angular",
-    package_name = "@angular-devkit/build-angular",
     srcs = glob(
         include = [
             "src/**/*.ts",
@@ -118,87 +118,88 @@ ts_library(
         "builders.json",
         "package.json",
     ],
-    module_name = "@angular-devkit/build-angular",
-    module_root = "src/index.d.ts",
-    deps = [
+    interop_deps = [
         "//packages/angular/build",
         "//packages/angular/build:private",
         "//packages/angular/ssr",
-        "//packages/angular_devkit/architect",
         "//packages/angular_devkit/build_webpack",
         "//packages/angular_devkit/core",
         "//packages/angular_devkit/core/node",
         "//packages/ngtools/webpack",
-        "@npm//@ampproject/remapping",
-        "@npm//@angular/common",
-        "@npm//@angular/compiler-cli",
-        "@npm//@angular/core",
-        "@npm//@angular/localize",
-        "@npm//@angular/platform-server",
-        "@npm//@angular/service-worker",
-        "@npm//@babel/core",
-        "@npm//@babel/generator",
-        "@npm//@babel/helper-annotate-as-pure",
-        "@npm//@babel/helper-split-export-declaration",
-        "@npm//@babel/plugin-transform-async-generator-functions",
-        "@npm//@babel/plugin-transform-async-to-generator",
-        "@npm//@babel/plugin-transform-runtime",
-        "@npm//@babel/preset-env",
-        "@npm//@babel/runtime",
-        "@npm//@discoveryjs/json-ext",
-        "@npm//@types/babel__core",
-        "@npm//@types/browser-sync",
-        "@npm//@types/karma",
-        "@npm//@types/less",
-        "@npm//@types/loader-utils",
-        "@npm//@types/node",
-        "@npm//@types/picomatch",
-        "@npm//@types/semver",
-        "@npm//@types/watchpack",
-        "@npm//@vitejs/plugin-basic-ssl",
-        "@npm//@web/test-runner",
-        "@npm//ajv",
-        "@npm//ansi-colors",
-        "@npm//autoprefixer",
-        "@npm//babel-loader",
-        "@npm//browserslist",
-        "@npm//copy-webpack-plugin",
-        "@npm//css-loader",
-        "@npm//esbuild",
-        "@npm//esbuild-wasm",
-        "@npm//fast-glob",
-        "@npm//http-proxy-middleware",
-        "@npm//istanbul-lib-instrument",
-        "@npm//jsonc-parser",
-        "@npm//karma",
-        "@npm//karma-source-map-support",
-        "@npm//less",
-        "@npm//less-loader",
-        "@npm//license-webpack-plugin",
-        "@npm//loader-utils",
-        "@npm//mini-css-extract-plugin",
-        "@npm//ng-packagr",
-        "@npm//open",
-        "@npm//ora",
-        "@npm//piscina",
-        "@npm//postcss",
-        "@npm//postcss-loader",
-        "@npm//resolve-url-loader",
-        "@npm//rxjs",
-        "@npm//sass",
-        "@npm//sass-loader",
-        "@npm//semver",
-        "@npm//source-map-loader",
-        "@npm//source-map-support",
-        "@npm//terser",
-        "@npm//tree-kill",
-        "@npm//tslib",
-        "@npm//typescript",
-        "@npm//webpack",
-        "@npm//webpack-dev-middleware",
-        "@npm//webpack-dev-server",
-        "@npm//webpack-merge",
-        "@npm//webpack-subresource-integrity",
+    ],
+    module_name = "@angular-devkit/build-angular",
+    deps = [
+        "//:root_modules/@ampproject/remapping",
+        "//:root_modules/@angular/common",
+        "//:root_modules/@angular/compiler-cli",
+        "//:root_modules/@angular/core",
+        "//:root_modules/@angular/localize",
+        "//:root_modules/@angular/platform-server",
+        "//:root_modules/@angular/service-worker",
+        "//:root_modules/@babel/core",
+        "//:root_modules/@babel/generator",
+        "//:root_modules/@babel/helper-annotate-as-pure",
+        "//:root_modules/@babel/helper-split-export-declaration",
+        "//:root_modules/@babel/plugin-transform-async-generator-functions",
+        "//:root_modules/@babel/plugin-transform-async-to-generator",
+        "//:root_modules/@babel/plugin-transform-runtime",
+        "//:root_modules/@babel/preset-env",
+        "//:root_modules/@babel/runtime",
+        "//:root_modules/@discoveryjs/json-ext",
+        "//:root_modules/@types/babel__core",
+        "//:root_modules/@types/browser-sync",
+        "//:root_modules/@types/karma",
+        "//:root_modules/@types/less",
+        "//:root_modules/@types/loader-utils",
+        "//:root_modules/@types/node",
+        "//:root_modules/@types/picomatch",
+        "//:root_modules/@types/semver",
+        "//:root_modules/@types/watchpack",
+        "//:root_modules/@vitejs/plugin-basic-ssl",
+        "//:root_modules/@web/test-runner",
+        "//:root_modules/ajv",
+        "//:root_modules/ansi-colors",
+        "//:root_modules/autoprefixer",
+        "//:root_modules/babel-loader",
+        "//:root_modules/browserslist",
+        "//:root_modules/copy-webpack-plugin",
+        "//:root_modules/css-loader",
+        "//:root_modules/esbuild",
+        "//:root_modules/esbuild-wasm",
+        "//:root_modules/fast-glob",
+        "//:root_modules/http-proxy-middleware",
+        "//:root_modules/istanbul-lib-instrument",
+        "//:root_modules/jsonc-parser",
+        "//:root_modules/karma",
+        "//:root_modules/karma-source-map-support",
+        "//:root_modules/less",
+        "//:root_modules/less-loader",
+        "//:root_modules/license-webpack-plugin",
+        "//:root_modules/loader-utils",
+        "//:root_modules/mini-css-extract-plugin",
+        "//:root_modules/ng-packagr",
+        "//:root_modules/open",
+        "//:root_modules/ora",
+        "//:root_modules/piscina",
+        "//:root_modules/postcss",
+        "//:root_modules/postcss-loader",
+        "//:root_modules/resolve-url-loader",
+        "//:root_modules/rxjs",
+        "//:root_modules/sass",
+        "//:root_modules/sass-loader",
+        "//:root_modules/semver",
+        "//:root_modules/source-map-loader",
+        "//:root_modules/source-map-support",
+        "//:root_modules/terser",
+        "//:root_modules/tree-kill",
+        "//:root_modules/tslib",
+        "//:root_modules/typescript",
+        "//:root_modules/webpack",
+        "//:root_modules/webpack-dev-middleware",
+        "//:root_modules/webpack-dev-server",
+        "//:root_modules/webpack-merge",
+        "//:root_modules/webpack-subresource-integrity",
+        "//packages/angular_devkit/architect",
     ],
 )
 

packages/angular_devkit/build_webpack/BUILD.bazel

@@ -32,6 +32,7 @@ ts_library(
             "src/**/*_spec.ts",
         ],
     ) + [
+        "index.ts",
         "//packages/angular_devkit/build_webpack:src/builders/webpack-dev-server/schema.ts",
         "//packages/angular_devkit/build_webpack:src/builders/webpack/schema.ts",
     ],

packages/angular_devkit/build_webpack/index.ts

@@ -0,0 +1,9 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+export * from './src/index';

tools/interop.bzl

@@ -42,6 +42,17 @@ def _ts_project_module_impl(ctx):
     runfiles = ctx.attr.dep[DefaultInfo].default_runfiles
     info = ctx.attr.dep[JsInfo]
 
+    # Filter runfiles to not `node_modules` from Aspect as this interop
+    # target is supposed to be used downstream by `rules_nodejs` consumers,
+    # and mixing pnpm-style node modules with linker node modules is incompatible.
+    filtered = []
+    for f in runfiles.files.to_list():
+        if f.short_path.startswith("node_modules/"):
+            continue
+        filtered.append(f)
+
+    runfiles = ctx.runfiles(files = filtered)
+
     providers = [
         DefaultInfo(
             runfiles = runfiles,
@@ -83,9 +94,21 @@ ts_project_module = rule(
 )
 
 def ts_project(name, module_name = None, interop_deps = [], deps = [], testonly = False, **kwargs):
+    # Pull in the `rules_nodejs` variants of dependencies we know are "hybrid". This
+    # is necessary as we can't mix `npm/node_modules` from RNJS with the pnpm-style
+    # symlink-dependent node modules. In addition, we need to extract `_rjs` interop
+    # dependencies so that we can forward and capture the module mappings for runtime
+    # execution, with regards to first-party dependency linking.
+    rjs_modules_to_rnjs = []
+    for d in deps:
+        if d.startswith("//:root_modules/"):
+            rjs_modules_to_rnjs.append(d.replace("//:root_modules/", "@npm//"))
+        if d.endswith("_rjs"):
+            rjs_modules_to_rnjs.append(d.replace("_rjs", ""))
+
     ts_deps_interop(
         name = "%s_interop_deps" % name,
-        deps = interop_deps,
+        deps = [] + interop_deps + rjs_modules_to_rnjs,
         testonly = testonly,
     )
 
@@ -99,14 +122,16 @@ def ts_project(name, module_name = None, interop_deps = [], deps = [], testonly
         # worker for efficient, fast DX and avoiding Windows no-sandbox issues.
         supports_workers = 1,
         tsc_worker = "//tools:vanilla_ts_worker",
-        deps = ["%s_interop_deps" % name] + deps,
+        deps = [":%s_interop_deps" % name] + deps,
         **kwargs
     )
 
     ts_project_module(
         name = name,
         testonly = testonly,
         dep = "%s_rjs" % name,
-        deps = interop_deps,
+        # Forwarded dependencies for linker module mapping aspect.
+        # RJS deps can also transitively pull in module mappings from their `interop_deps`.
+        deps = [] + interop_deps + deps,
         module_name = module_name,
     )

tsconfig.json

@@ -27,6 +27,7 @@
       "@angular-devkit/architect/*": ["./packages/angular_devkit/architect/*/index"],
       "@angular-devkit/build-webpack": ["./packages/angular_devkit/build_webpack"],
       "@angular-devkit/*": ["./packages/angular_devkit/*/src"],
+      "@angular/ssr": ["./packages/angular/ssr"],
       "@angular/*": ["./packages/angular/*/src"],
       "@angular/build/private": ["./packages/angular/build/src/private"],
       "@ngtools/webpack": ["./packages/ngtools/webpack/src"],