fix: multi-linker linking when only output files in sandbox

Author: Greg Magolan

WORKSPACE

@@ -91,6 +91,14 @@ yarn_install(
     yarn_lock = "//packages/cypress/test:yarn.lock",
 )
 
+yarn_install(
+    name = "rollup_test_multi_linker_deps",
+    package_json = "//packages/rollup/test/multi_linker:package.json",
+    package_path = "packages/rollup/test/multi_linker",
+    symlink_node_modules = False,
+    yarn_lock = "//packages/rollup/test/multi_linker:yarn.lock",
+)
+
 # We have a source dependency on build_bazel_rules_typescript
 # so we must repeat its transitive toolchain deps
 load("@build_bazel_rules_typescript//:package.bzl", "rules_typescript_dev_dependencies")

internal/linker/index.js

@@ -420,26 +420,17 @@ function main(args, runfiles) {
             const workspaceNodeModules = `${workspacePath}/node_modules`;
             if (packagePath) {
                 if (yield exists(workspaceNodeModules)) {
-                    let resolvedPackagePath;
-                    if (yield exists(packagePath)) {
-                        yield symlinkWithUnlink(workspaceNodeModules, `${packagePath}/node_modules`);
-                        resolvedPackagePath = packagePath;
-                    }
+                    yield mkdirp(packagePath);
+                    yield symlinkWithUnlink(workspaceNodeModules, `${packagePath}/node_modules`);
                     if (!isExecroot) {
                         const runfilesPackagePath = `${startCwd}/${packagePath}`;
                         if (yield exists(runfilesPackagePath)) {
-                            if (resolvedPackagePath) {
-                                yield symlinkWithUnlink(`${resolvedPackagePath}/node_modules`, `${runfilesPackagePath}/node_modules`);
-                            }
-                            else {
-                                yield symlinkWithUnlink(workspaceNodeModules, `${runfilesPackagePath}/node_modules`);
-                            }
-                            resolvedPackagePath = runfilesPackagePath;
+                            yield symlinkWithUnlink(`${packagePath}/node_modules`, `${runfilesPackagePath}/node_modules`);
                         }
                     }
                     const packagePathBin = `${bin}/${packagePath}`;
-                    if (resolvedPackagePath && (yield exists(packagePathBin))) {
-                        yield symlinkWithUnlink(`${resolvedPackagePath}/node_modules`, `${packagePathBin}/node_modules`);
+                    if (yield exists(packagePathBin)) {
+                        yield symlinkWithUnlink(`${packagePath}/node_modules`, `${packagePathBin}/node_modules`);
                     }
                 }
             }

internal/linker/link_node_modules.ts

@@ -618,30 +618,30 @@ export async function main(args: string[], runfiles: Runfiles) {
     if (packagePath) {
       // sub-directory node_modules
       if (await exists(workspaceNodeModules)) {
-        let resolvedPackagePath;
-        if (await exists(packagePath)) {
-          await symlinkWithUnlink(workspaceNodeModules, `${packagePath}/node_modules`);
-          resolvedPackagePath = packagePath;
-        }
+        // in some cases packagePath may not exist in sandbox if there are no source deps
+        // and only generated file deps. we create it so that we that we can link to
+        // packagePath/node_modules since packagePathBin/node_modules is a symlink to
+        // packagePath/node_modules and is unguarded in launcher.sh as we allow symlinks to fall
+        // through to from output tree to source tree to prevent resolving the same npm package to
+        // multiple locations on disk
+        await mkdirp(packagePath);
+        await symlinkWithUnlink(workspaceNodeModules, `${packagePath}/node_modules`);
         if (!isExecroot) {
           // Under runfiles, we symlink into the package in runfiles as well.
           // When inside the sandbox, the execroot location will not exist to symlink to.
           const runfilesPackagePath = `${startCwd}/${packagePath}`;
           if (await exists(runfilesPackagePath)) {
-            if (resolvedPackagePath) {
-              await symlinkWithUnlink(
-                  `${resolvedPackagePath}/node_modules`, `${runfilesPackagePath}/node_modules`);
-            } else {
-              await symlinkWithUnlink(workspaceNodeModules, `${runfilesPackagePath}/node_modules`);
-            }
-            resolvedPackagePath = runfilesPackagePath;
+            await symlinkWithUnlink(
+                `${packagePath}/node_modules`, `${runfilesPackagePath}/node_modules`);
           }
         }
         const packagePathBin = `${bin}/${packagePath}`;
-        if (resolvedPackagePath && await exists(packagePathBin)) {
+        if (await exists(packagePathBin)) {
           // if bin path exists, symlink bin/package/node_modules -> package/node_modules
-          await symlinkWithUnlink(
-              `${resolvedPackagePath}/node_modules`, `${packagePathBin}/node_modules`);
+          // NB: this location is unguarded in launcher.sh to allow symlinks to fall-throught
+          // package/node_modules to prevent resolving the same npm package to multiple locations on
+          // disk
+          await symlinkWithUnlink(`${packagePath}/node_modules`, `${packagePathBin}/node_modules`);
         }
       }
     } else {

packages/rollup/test/multi_linker/BUILD.bazel

@@ -0,0 +1,32 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test")
+load("//packages/rollup:index.bzl", "rollup_bundle")
+load("//packages/typescript:index.bzl", "ts_project")
+
+ts_project(
+    name = "tsconfig",
+    srcs = glob(["*.ts"]),
+    deps = ["@rollup_test_multi_linker_deps//@types"],
+)
+
+# Intentionally test with only generated files by transpiling with ts_project
+# to test multi-linker build action with no source inputs.
+# NB: rollup.config.js is used to generated "_{name}.rollup_config.js" in output
+# tree. Only the generated config is a input to the rollup build action.
+rollup_bundle(
+    name = "bundle",
+    config_file = "rollup.config.js",
+    entry_point = "main.js",
+    deps = [
+        "tsconfig",
+        "@rollup_test_multi_linker_deps//@rollup/plugin-commonjs",
+        "@rollup_test_multi_linker_deps//@rollup/plugin-node-resolve",
+    ],
+)
+
+generated_file_test(
+    name = "test",
+    src = "golden.js_",
+    generated = "bundle.js",
+    # fails in some non-trivial way on Windows; not particularly important to Windows coverage here
+    tags = ["fix-windows"],
+)

packages/rollup/test/multi_linker/golden.js_

@@ -0,0 +1,22 @@
+function createCommonjsModule(fn, basedir, module) {
+	return module = {
+	  path: basedir,
+	  exports: {},
+	  require: function (path, base) {
+      return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
+    }
+	}, fn(module, module.exports), module.exports;
+}
+
+function commonjsRequire () {
+	throw new Error('Dynamic requires are not currently supported by @rollup/plugin-commonjs');
+}
+
+var lib = createCommonjsModule(function (module, exports) {
+exports.__esModule = true;
+exports.key = 'rollup';
+});
+
+var key = lib.key;
+console.log(key);
+//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiYnVuZGxlLmpzIiwic291cmNlcyI6WyJsaWIuanMiLCJtYWluLmpzIl0sInNvdXJjZXNDb250ZW50IjpbIlwidXNlIHN0cmljdFwiO1xuZXhwb3J0cy5fX2VzTW9kdWxlID0gdHJ1ZTtcbmV4cG9ydHMua2V5ID0gJ3JvbGx1cCc7XG4iLCJ2YXIga2V5ID0gcmVxdWlyZSgnLi9saWInKS5rZXk7XG5jb25zb2xlLmxvZyhrZXkpO1xuIl0sIm5hbWVzIjpbInJlcXVpcmUkJDAiXSwibWFwcGluZ3MiOiI7Ozs7Ozs7Ozs7Ozs7OztBQUNBLGtCQUFrQixHQUFHLElBQUksQ0FBQztBQUMxQixXQUFXLEdBQUcsUUFBUTs7O0FDRnRCLElBQUksR0FBRyxHQUFHQSxHQUFnQixDQUFDLEdBQUcsQ0FBQztBQUMvQixPQUFPLENBQUMsR0FBRyxDQUFDLEdBQUcsQ0FBQyJ9

packages/rollup/test/multi_linker/lib.ts

@@ -0,0 +1 @@
+export const key: string = 'rollup';

packages/rollup/test/multi_linker/main.ts

@@ -0,0 +1,2 @@
+const key = require('./lib').key;
+console.log(key);

packages/rollup/test/multi_linker/package.json

@@ -0,0 +1,7 @@
+{
+  "devDependencies": {
+    "@types/node": "14.14.26",
+    "@rollup/plugin-commonjs": "14.0.0",
+    "@rollup/plugin-node-resolve": "8.4.0"
+  }
+}

packages/rollup/test/multi_linker/rollup.config.js

@@ -0,0 +1,9 @@
+const {nodeResolve} = require('@rollup/plugin-node-resolve')
+const commonjs = require('@rollup/plugin-commonjs')
+
+module.exports = {
+  plugins: [
+    nodeResolve(),
+    commonjs(),
+  ],
+}

packages/rollup/test/multi_linker/tsconfig.json

@@ -0,0 +1,5 @@
+{
+  "compilerOptions": {
+    "types": ["node"]
+  }
+}