fix: handle js_library 1p deps correctly in js_run_devserver (#1872)

e2e/webpack_devserver/.bazelignore

@@ -1,2 +1,3 @@
 node_modules
 mylib/node_modules
+mypkg/node_modules

e2e/webpack_devserver/mylib/BUILD.bazel

@@ -1,10 +1,16 @@
-load("@aspect_rules_js//npm:defs.bzl", "npm_package")
+load("@aspect_rules_js//js:defs.bzl", "js_library")
+load("@npm//:defs.bzl", "npm_link_all_packages")
 
-npm_package(
+npm_link_all_packages(name = "node_modules")
+
+js_library(
     name = "pkg",
     srcs = [
         "index.js",
         "package.json",
     ],
     visibility = ["//visibility:public"],
+    deps = [
+        ":node_modules",
+    ],
 )

e2e/webpack_devserver/mylib/index.js

@@ -1,5 +1,5 @@
 const packageJson = require('./package.json')
 const chalk = require('chalk')
 module.exports = {
-    name: () => chalk.blue(packageJson.name),
+    name: () => chalk.green(packageJson.name),
 }

e2e/webpack_devserver/mypkg/BUILD.bazel

@@ -0,0 +1,10 @@
+load("@aspect_rules_js//npm:defs.bzl", "npm_package")
+
+npm_package(
+    name = "pkg",
+    srcs = [
+        "index.js",
+        "package.json",
+    ],
+    visibility = ["//visibility:public"],
+)

e2e/webpack_devserver/mypkg/index.js

@@ -0,0 +1,5 @@
+const packageJson = require('./package.json')
+const chalk = require('chalk')
+module.exports = {
+    name: () => chalk.blue(packageJson.name),
+}

e2e/webpack_devserver/mypkg/package.json

@@ -0,0 +1,6 @@
+{
+    "name": "@mycorp/mypkg",
+    "dependencies": {
+        "chalk": "^4"
+    }
+}

e2e/webpack_devserver/package.json

@@ -8,6 +8,7 @@
     },
     "dependencies": {
         "lodash": "4.17.21",
-        "@mycorp/mylib": "workspace:*"
+        "@mycorp/mylib": "workspace:*",
+        "@mycorp/mypkg": "workspace:*"
     }
 }

e2e/webpack_devserver/pnpm-workspace.yaml

@@ -1,3 +1,4 @@
 packages:
     - '.'
     - 'mylib'
+    - 'mypkg'

e2e/webpack_devserver/serve_test.sh

@@ -26,6 +26,7 @@ ibazel_pid="$!"
 function _exit {
     kill "$ibazel_pid"
     git checkout src/index.html >/dev/null 2>&1
+    git checkout mypkg/index.js >/dev/null 2>&1
     git checkout mylib/index.js >/dev/null 2>&1
     rm -f "$ibazel_logs"
 }
@@ -48,11 +49,18 @@ if ! curl http://localhost:8080/index.html --fail 2>/dev/null | grep "Getting St
     exit 1
 fi
 
+# from @mycorp/mypkg
 if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk.blue(packageJson.name)"; then
     echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk.blue(packageJson.name)'"
     exit 1
 fi
 
+# from @mycorp/mylib
+if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk.green(packageJson.name)"; then
+    echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk.green(packageJson.name)'"
+    exit 1
+fi
+
 _sedi 's#Getting Started#Goodbye#' src/index.html
 
 echo "Waiting 5 seconds for ibazel rebuild after change to src/index.html..."
@@ -63,45 +71,69 @@ if ! curl http://localhost:8080/index.html --fail 2>/dev/null | grep "Goodbye";
     exit 1
 fi
 
-_sedi 's#blue#red#' mylib/index.js
+_sedi 's#blue#red#' mypkg/index.js
 
-echo "Waiting 5 seconds for ibazel rebuild after change to mylib/index.js..."
+echo "Waiting 5 seconds for ibazel rebuild after change to mypkg/index.js..."
 sleep 5
 
+# from @mycorp/mypkg
 if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk.red(packageJson.name)"; then
     echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk.red(packageJson.name)'"
     exit 1
 fi
 
+_sedi 's#green#cyan#' mylib/index.js
+
+echo "Waiting 5 seconds for ibazel rebuild after change to mylib/index.js..."
+sleep 5
+
+# from @mycorp/mylib
+if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk.cyan(packageJson.name)"; then
+    echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk.cyan(packageJson.name)'"
+    exit 1
+fi
+
 echo "Checking log file $ibazel_logs"
 
-count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/index.js" "$ibazel_logs" || true)
+count=$(grep -c "Syncing symlink node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib (1p)" "$ibazel_logs" || true)
+if [[ "$count" -ne 1 ]]; then
+    echo "ERROR: expected to have synced @mycorp/mylib symlink 1 time but found ${count}"
+    exit 1
+fi
+
+count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js" "$ibazel_logs" || true)
 if [[ "$count" -ne 2 ]]; then
-    echo "ERROR: expected to have synced @mycorp/mylib/index.js 2 times but found ${count}"
+    echo "ERROR: expected to have synced @mycorp/mypkg/index.js 2 times but found ${count}"
     exit 1
 fi
 
-count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/index.js since its timestamp has not changed" "$ibazel_logs" || true)
-if [[ "$count" -ne 1 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mylib/index.js due to timestamp 1 time but found ${count}"
+count=$(grep -c "Syncing file mylib/index.js" "$ibazel_logs" || true)
+if [[ "$count" -ne 2 ]]; then
+    echo "ERROR: expected to have synced mylib/index.js 2 times but found ${count}"
     exit 1
 fi
 
-count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/package.json" "$ibazel_logs" || true)
-if [[ "$count" -ne 1 ]]; then
-    echo "ERROR: expected to have synced @mycorp/mylib/package.json 1 time but found ${count}"
+count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js since its timestamp has not changed" "$ibazel_logs" || true)
+if [[ "$count" -ne 2 ]]; then
+    echo "ERROR: expected to have skipped @mycorp/mypkg/index.js due to timestamp 2 times but found ${count}"
     exit 1
 fi
 
-count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/package.json since its timestamp has not changed" "$ibazel_logs" || true)
+count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mylib/package.json due to timestamp 1 time but found ${count}"
+    echo "ERROR: expected to have synced @mycorp/mypkg/package.json 1 time but found ${count}"
+    exit 1
+fi
+
+count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since its timestamp has not changed" "$ibazel_logs" || true)
+if [[ "$count" -ne 2 ]]; then
+    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to timestamp 2 times but found ${count}"
     exit 1
 fi
 
-count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/package.json since contents have not changed" "$ibazel_logs" || true)
+count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since contents have not changed" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mylib/package.json due to contents 1 time but found ${count}"
+    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to contents 1 times but found ${count}"
     exit 1
 fi
 

e2e/webpack_devserver/src/index.js

@@ -1,11 +1,15 @@
 import _ from 'lodash'
-import { name } from '@mycorp/mylib'
+import { name as mylibname } from '@mycorp/mylib'
+import { name as mypkgname } from '@mycorp/mypkg'
 
 function component() {
     const element = document.createElement('div')
 
     // Lodash, currently included via a script, is required for this line to work
-    element.innerHTML = _.join(['Hello', 'webpack', name()], ' ')
+    element.innerHTML = _.join(
+        ['Hello', 'webpack', mylibname(), mypkgname()],
+        ' '
+    )
 
     return element
 }

e2e/webpack_devserver_esm/.bazelignore

@@ -1,2 +1,3 @@
 node_modules
 mylib/node_modules
+mypkg/node_modules

e2e/webpack_devserver_esm/mylib/BUILD.bazel

@@ -1,10 +1,16 @@
-load("@aspect_rules_js//npm:defs.bzl", "npm_package")
+load("@aspect_rules_js//js:defs.bzl", "js_library")
+load("@npm//:defs.bzl", "npm_link_all_packages")
 
-npm_package(
+npm_link_all_packages(name = "node_modules")
+
+js_library(
     name = "pkg",
     srcs = [
         "index.js",
         "package.json",
     ],
     visibility = ["//visibility:public"],
+    deps = [
+        ":node_modules",
+    ],
 )

e2e/webpack_devserver_esm/mylib/index.js

@@ -1,5 +1,5 @@
 import packageJson from './package.json'
 import chalk from 'chalk'
 export function name() {
-    return chalk.blue(packageJson.name)
+    return chalk.green(packageJson.name)
 }

e2e/webpack_devserver_esm/mypkg/BUILD.bazel

@@ -0,0 +1,10 @@
+load("@aspect_rules_js//npm:defs.bzl", "npm_package")
+
+npm_package(
+    name = "pkg",
+    srcs = [
+        "index.js",
+        "package.json",
+    ],
+    visibility = ["//visibility:public"],
+)

e2e/webpack_devserver_esm/mypkg/index.js

@@ -0,0 +1,5 @@
+import packageJson from './package.json'
+import chalk from 'chalk'
+export function name() {
+    return chalk.blue(packageJson.name)
+}

e2e/webpack_devserver_esm/mypkg/package.json

@@ -0,0 +1,6 @@
+{
+    "name": "@mycorp/mypkg",
+    "dependencies": {
+        "chalk": "^4"
+    }
+}

e2e/webpack_devserver_esm/package.json

@@ -9,6 +9,7 @@
     },
     "dependencies": {
         "lodash": "4.17.21",
-        "@mycorp/mylib": "workspace:*"
+        "@mycorp/mylib": "workspace:*",
+        "@mycorp/mypkg": "workspace:*"
     }
 }

e2e/webpack_devserver_esm/pnpm-workspace.yaml

@@ -1,3 +1,4 @@
 packages:
     - '.'
     - 'mylib'
+    - 'mypkg'