From cf18bb4e87642bdc3d9bf4312205e35a9ccba1aa Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Fri, 26 Jul 2024 14:47:57 -0700 Subject: [PATCH] fix: handle js_library 1p deps correctly in js_run_devserver --- e2e/webpack_devserver/.bazelignore | 1 + e2e/webpack_devserver/mylib/BUILD.bazel | 10 ++- e2e/webpack_devserver/mylib/index.js | 2 +- e2e/webpack_devserver/mypkg/BUILD.bazel | 10 +++ e2e/webpack_devserver/mypkg/index.js | 5 ++ e2e/webpack_devserver/mypkg/package.json | 6 ++ e2e/webpack_devserver/package.json | 3 +- e2e/webpack_devserver/pnpm-lock.yaml | 9 +++ e2e/webpack_devserver/pnpm-workspace.yaml | 1 + e2e/webpack_devserver/serve_test.sh | 61 ++++++++++++++----- e2e/webpack_devserver/src/index.js | 8 ++- e2e/webpack_devserver_esm/.bazelignore | 1 + e2e/webpack_devserver_esm/mylib/BUILD.bazel | 10 ++- e2e/webpack_devserver_esm/mylib/index.js | 2 +- e2e/webpack_devserver_esm/mypkg/BUILD.bazel | 10 +++ e2e/webpack_devserver_esm/mypkg/index.js | 5 ++ e2e/webpack_devserver_esm/mypkg/package.json | 6 ++ e2e/webpack_devserver_esm/package.json | 3 +- e2e/webpack_devserver_esm/pnpm-lock.yaml | 9 +++ e2e/webpack_devserver_esm/pnpm-workspace.yaml | 1 + e2e/webpack_devserver_esm/serve_test.sh | 61 ++++++++++++++----- e2e/webpack_devserver_esm/src/index.js | 8 ++- js/private/js_run_devserver.mjs | 48 +++++++++++---- .../js_run_devserver.spec.mjs | 16 ++--- 24 files changed, 234 insertions(+), 62 deletions(-) create mode 100644 e2e/webpack_devserver/mypkg/BUILD.bazel create mode 100644 e2e/webpack_devserver/mypkg/index.js create mode 100644 e2e/webpack_devserver/mypkg/package.json create mode 100644 e2e/webpack_devserver_esm/mypkg/BUILD.bazel create mode 100644 e2e/webpack_devserver_esm/mypkg/index.js create mode 100644 e2e/webpack_devserver_esm/mypkg/package.json diff --git a/e2e/webpack_devserver/.bazelignore b/e2e/webpack_devserver/.bazelignore index 262c043e8..5c5d69621 100644 --- a/e2e/webpack_devserver/.bazelignore +++ b/e2e/webpack_devserver/.bazelignore @@ -1,2 +1,3 @@ node_modules mylib/node_modules +mypkg/node_modules diff --git a/e2e/webpack_devserver/mylib/BUILD.bazel b/e2e/webpack_devserver/mylib/BUILD.bazel index 9fa99d499..6db42b8a6 100644 --- a/e2e/webpack_devserver/mylib/BUILD.bazel +++ b/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", + ], ) diff --git a/e2e/webpack_devserver/mylib/index.js b/e2e/webpack_devserver/mylib/index.js index 27630cec7..e4cb3f3b1 100644 --- a/e2e/webpack_devserver/mylib/index.js +++ b/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), } diff --git a/e2e/webpack_devserver/mypkg/BUILD.bazel b/e2e/webpack_devserver/mypkg/BUILD.bazel new file mode 100644 index 000000000..9fa99d499 --- /dev/null +++ b/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"], +) diff --git a/e2e/webpack_devserver/mypkg/index.js b/e2e/webpack_devserver/mypkg/index.js new file mode 100644 index 000000000..27630cec7 --- /dev/null +++ b/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), +} diff --git a/e2e/webpack_devserver/mypkg/package.json b/e2e/webpack_devserver/mypkg/package.json new file mode 100644 index 000000000..a7e7adea9 --- /dev/null +++ b/e2e/webpack_devserver/mypkg/package.json @@ -0,0 +1,6 @@ +{ + "name": "@mycorp/mypkg", + "dependencies": { + "chalk": "^4" + } +} diff --git a/e2e/webpack_devserver/package.json b/e2e/webpack_devserver/package.json index 8541cf53f..8ba2b94f4 100644 --- a/e2e/webpack_devserver/package.json +++ b/e2e/webpack_devserver/package.json @@ -8,6 +8,7 @@ }, "dependencies": { "lodash": "4.17.21", - "@mycorp/mylib": "workspace:*" + "@mycorp/mylib": "workspace:*", + "@mycorp/mypkg": "workspace:*" } } diff --git a/e2e/webpack_devserver/pnpm-lock.yaml b/e2e/webpack_devserver/pnpm-lock.yaml index bba49f6ef..4a870074d 100644 --- a/e2e/webpack_devserver/pnpm-lock.yaml +++ b/e2e/webpack_devserver/pnpm-lock.yaml @@ -11,6 +11,9 @@ importers: '@mycorp/mylib': specifier: workspace:* version: link:mylib + '@mycorp/mypkg': + specifier: workspace:* + version: link:mypkg lodash: specifier: 4.17.21 version: 4.17.21 @@ -37,6 +40,12 @@ importers: specifier: ^4 version: 4.1.2 + mypkg: + dependencies: + chalk: + specifier: ^4 + version: 4.1.2 + packages: /@bazel/ibazel@0.16.2: diff --git a/e2e/webpack_devserver/pnpm-workspace.yaml b/e2e/webpack_devserver/pnpm-workspace.yaml index 3d432df1c..98fef906b 100644 --- a/e2e/webpack_devserver/pnpm-workspace.yaml +++ b/e2e/webpack_devserver/pnpm-workspace.yaml @@ -1,3 +1,4 @@ packages: - '.' - 'mylib' + - 'mypkg' diff --git a/e2e/webpack_devserver/serve_test.sh b/e2e/webpack_devserver/serve_test.sh index acbac73f3..2fa05de28 100755 --- a/e2e/webpack_devserver/serve_test.sh +++ b/e2e/webpack_devserver/serve_test.sh @@ -26,7 +26,7 @@ ibazel_pid="$!" function _exit { kill "$ibazel_pid" git checkout src/index.html >/dev/null 2>&1 - git checkout mylib/index.js >/dev/null 2>&1 + git checkout mypkg/index.js >/dev/null 2>&1 rm -f "$ibazel_logs" } trap _exit EXIT @@ -48,11 +48,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 +70,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 diff --git a/e2e/webpack_devserver/src/index.js b/e2e/webpack_devserver/src/index.js index 282f86a5f..fb6cc893d 100644 --- a/e2e/webpack_devserver/src/index.js +++ b/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 } diff --git a/e2e/webpack_devserver_esm/.bazelignore b/e2e/webpack_devserver_esm/.bazelignore index 262c043e8..5c5d69621 100644 --- a/e2e/webpack_devserver_esm/.bazelignore +++ b/e2e/webpack_devserver_esm/.bazelignore @@ -1,2 +1,3 @@ node_modules mylib/node_modules +mypkg/node_modules diff --git a/e2e/webpack_devserver_esm/mylib/BUILD.bazel b/e2e/webpack_devserver_esm/mylib/BUILD.bazel index 9fa99d499..6db42b8a6 100644 --- a/e2e/webpack_devserver_esm/mylib/BUILD.bazel +++ b/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", + ], ) diff --git a/e2e/webpack_devserver_esm/mylib/index.js b/e2e/webpack_devserver_esm/mylib/index.js index 0a7aa03c5..f1d261c72 100644 --- a/e2e/webpack_devserver_esm/mylib/index.js +++ b/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) } diff --git a/e2e/webpack_devserver_esm/mypkg/BUILD.bazel b/e2e/webpack_devserver_esm/mypkg/BUILD.bazel new file mode 100644 index 000000000..9fa99d499 --- /dev/null +++ b/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"], +) diff --git a/e2e/webpack_devserver_esm/mypkg/index.js b/e2e/webpack_devserver_esm/mypkg/index.js new file mode 100644 index 000000000..0a7aa03c5 --- /dev/null +++ b/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) +} diff --git a/e2e/webpack_devserver_esm/mypkg/package.json b/e2e/webpack_devserver_esm/mypkg/package.json new file mode 100644 index 000000000..a7e7adea9 --- /dev/null +++ b/e2e/webpack_devserver_esm/mypkg/package.json @@ -0,0 +1,6 @@ +{ + "name": "@mycorp/mypkg", + "dependencies": { + "chalk": "^4" + } +} diff --git a/e2e/webpack_devserver_esm/package.json b/e2e/webpack_devserver_esm/package.json index df1ae0ecc..510427318 100644 --- a/e2e/webpack_devserver_esm/package.json +++ b/e2e/webpack_devserver_esm/package.json @@ -9,6 +9,7 @@ }, "dependencies": { "lodash": "4.17.21", - "@mycorp/mylib": "workspace:*" + "@mycorp/mylib": "workspace:*", + "@mycorp/mypkg": "workspace:*" } } diff --git a/e2e/webpack_devserver_esm/pnpm-lock.yaml b/e2e/webpack_devserver_esm/pnpm-lock.yaml index bba49f6ef..4a870074d 100644 --- a/e2e/webpack_devserver_esm/pnpm-lock.yaml +++ b/e2e/webpack_devserver_esm/pnpm-lock.yaml @@ -11,6 +11,9 @@ importers: '@mycorp/mylib': specifier: workspace:* version: link:mylib + '@mycorp/mypkg': + specifier: workspace:* + version: link:mypkg lodash: specifier: 4.17.21 version: 4.17.21 @@ -37,6 +40,12 @@ importers: specifier: ^4 version: 4.1.2 + mypkg: + dependencies: + chalk: + specifier: ^4 + version: 4.1.2 + packages: /@bazel/ibazel@0.16.2: diff --git a/e2e/webpack_devserver_esm/pnpm-workspace.yaml b/e2e/webpack_devserver_esm/pnpm-workspace.yaml index 3d432df1c..98fef906b 100644 --- a/e2e/webpack_devserver_esm/pnpm-workspace.yaml +++ b/e2e/webpack_devserver_esm/pnpm-workspace.yaml @@ -1,3 +1,4 @@ packages: - '.' - 'mylib' + - 'mypkg' diff --git a/e2e/webpack_devserver_esm/serve_test.sh b/e2e/webpack_devserver_esm/serve_test.sh index c1abdfc09..99000a302 100755 --- a/e2e/webpack_devserver_esm/serve_test.sh +++ b/e2e/webpack_devserver_esm/serve_test.sh @@ -24,7 +24,7 @@ ibazel_pid="$!" function _exit { kill "$ibazel_pid" git checkout src/index.html >/dev/null 2>&1 - git checkout mylib/index.js >/dev/null 2>&1 + git checkout mypkg/index.js >/dev/null 2>&1 rm -f "$ibazel_logs" } trap _exit EXIT @@ -46,11 +46,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__WEBPACK_IMPORTED_MODULE_1___default().blue(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)"; then echo "ERROR: http://localhost:8080/main.js to contain 'chalk__WEBPACK_IMPORTED_MODULE_1___default().blue(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)'" exit 1 fi +# from @mycorp/mylib +if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk__WEBPACK_IMPORTED_MODULE_1___default().blue(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)"; then + echo "ERROR: http://localhost:8080/main.js to contain 'chalk__WEBPACK_IMPORTED_MODULE_1___default().green(_package_json__WEBPACK_IMPORTED_MODULE_0__.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..." @@ -61,43 +68,67 @@ 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__WEBPACK_IMPORTED_MODULE_1___default().red(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)"; then echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk__WEBPACK_IMPORTED_MODULE_1___default().red(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)'" exit 1 fi -count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib/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}" +_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__WEBPACK_IMPORTED_MODULE_1___default().cyan(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)"; then + echo "ERROR: Expected http://localhost:8080/main.js to contain 'chalk__WEBPACK_IMPORTED_MODULE_1___default().cyan(_package_json__WEBPACK_IMPORTED_MODULE_0__.name)'" 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) +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 skipped @mycorp/mylib/index.js due to timestamp 1 time but found ${count}" + 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+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 "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/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/package.json since its timestamp has not changed" "$ibazel_logs" || true) +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 "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 "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 diff --git a/e2e/webpack_devserver_esm/src/index.js b/e2e/webpack_devserver_esm/src/index.js index 282f86a5f..fb6cc893d 100644 --- a/e2e/webpack_devserver_esm/src/index.js +++ b/e2e/webpack_devserver_esm/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 } diff --git a/js/private/js_run_devserver.mjs b/js/private/js_run_devserver.mjs index bd6f26593..f5ee38f79 100644 --- a/js/private/js_run_devserver.mjs +++ b/js/private/js_run_devserver.mjs @@ -54,7 +54,7 @@ export function isNodeModulePath(p) { // Determines if a file path is a 1p dep in the package store. // See js/private/test/js_run_devserver/js_run_devserver.spec.mjs for examples. -export function is1pVirtualStoreDep(p) { +export function is1pPackageStoreDep(p) { // unscoped1p: https://regex101.com/r/hBR08J/1 const unscoped1p = /^.+\/\.aspect_rules_js\/([^@\/]+)@0\.0\.0\/node_modules\/\1$/ @@ -248,25 +248,48 @@ async function syncRecursive(src, dst, sandbox, writePerm) { // Sync list of files to the sandbox async function sync(files, sandbox, writePerm) { - console.error(`Syncing ${files.length} files && folders...`) + console.error(`+ Syncing ${files.length} files && folders...`) const startTime = perf_hooks.performance.now() - const [virtualStore1pFiles, remainingFiles] = partitionArray( + const [nodeModulesFiles, otherFiles] = partitionArray( files, - is1pVirtualStoreDep + isNodeModulePath ) - if (virtualStore1pFiles.length > 0 && process.env.JS_BINARY__LOG_DEBUG) { + const [packageStore1pDeps, otherNodeModulesFiles] = partitionArray( + nodeModulesFiles, + is1pPackageStoreDep + ) + + // Sync non-node_modules files first since syncing 1p js_library linked node_modules symlinks + // requires the files they point to be in place. + if (otherFiles.length > 0 && process.env.JS_BINARY__LOG_DEBUG) { console.error( - `Syncing ${virtualStore1pFiles.length} first party package store dep(s)` + `+ Syncing ${otherFiles.length} non-node_modules files & folders...` ) } - // Sync first-party package store files first since correctly syncing direct 1p node_modules - // symlinks depends on checking if the package store synced files exist. let totalSynced = ( await Promise.all( - virtualStore1pFiles.map(async (file) => { + otherFiles.map(async (file) => { + const src = path.join(RUNFILES_ROOT, file) + const dst = path.join(sandbox, file) + return await syncRecursive(src, dst, sandbox, writePerm) + }) + ) + ).reduce((s, t) => s + t, 0) + + // Sync first-party package store files before other node_modules files since correctly syncing + // direct 1p node_modules symlinks depends on checking if the package store synced files exist. + if (packageStore1pDeps.length > 0 && process.env.JS_BINARY__LOG_DEBUG) { + console.error( + `+ Syncing ${packageStore1pDeps.length} first party package store dep(s)` + ) + } + + totalSynced += ( + await Promise.all( + packageStore1pDeps.map(async (file) => { const src = path.join(RUNFILES_ROOT, file) const dst = path.join(sandbox, file) return await syncRecursive(src, dst, sandbox, writePerm) @@ -274,15 +297,16 @@ async function sync(files, sandbox, writePerm) { ) ).reduce((s, t) => s + t, 0) - if (remainingFiles.length > 0 && process.env.JS_BINARY__LOG_DEBUG) { + // Finally sync all remaining node_modules files + if (otherNodeModulesFiles.length > 0 && process.env.JS_BINARY__LOG_DEBUG) { console.error( - `Syncing ${remainingFiles.length} other files && folders...` + `+ Syncing ${otherNodeModulesFiles.length} other node_modules files` ) } totalSynced += ( await Promise.all( - remainingFiles.map(async (file) => { + otherNodeModulesFiles.map(async (file) => { const src = path.join(RUNFILES_ROOT, file) const dst = path.join(sandbox, file) return await syncRecursive(src, dst, sandbox, writePerm) diff --git a/js/private/test/js_run_devserver/js_run_devserver.spec.mjs b/js/private/test/js_run_devserver/js_run_devserver.spec.mjs index 5b06a0eb2..6d0107fd7 100644 --- a/js/private/test/js_run_devserver/js_run_devserver.spec.mjs +++ b/js/private/test/js_run_devserver/js_run_devserver.spec.mjs @@ -1,6 +1,6 @@ import { isNodeModulePath, - is1pVirtualStoreDep, + is1pPackageStoreDep, friendlyFileSize, } from '../../js_run_devserver.mjs' @@ -23,18 +23,18 @@ for (const p of isNodeModulePath_false) { } } -// is1pVirtualStoreDep -const is1pVirtualStoreDep_true = [ +// is1pPackageStoreDep +const is1pPackageStoreDep_true = [ 'some/path/node_modules/.aspect_rules_js/@mycorp+pkg@0.0.0/node_modules/@mycorp/pkg', 'some/path/node_modules/.aspect_rules_js/mycorp-pkg@0.0.0/node_modules/mycorp-pkg', ] -for (const p of is1pVirtualStoreDep_true) { - if (!is1pVirtualStoreDep(p)) { +for (const p of is1pPackageStoreDep_true) { + if (!is1pPackageStoreDep(p)) { console.error(`ERROR: expected ${p} to be a 1p package store dep`) process.exit(1) } } -const is1pVirtualStoreDep_false = [ +const is1pPackageStoreDep_false = [ 'some/path/node_modules/.aspect_rules_js/@mycorp+pkg@0.0.0/node_modules/mycorp/pkg', 'some/path/node_modules/.aspect_rules_js/@mycorp+pkg0.0.0/node_modules/@mycorp/pkg', 'some/path/node_modules/.aspect_rules_js/mycorp+pkg@0.0.0/node_modules/@mycorp/pkg', @@ -46,8 +46,8 @@ const is1pVirtualStoreDep_false = [ 'some/path/node_modules/.aspect_rules_js/eval@0.1.6/node_modules/eval', 'some/path/node_modules/.aspect_rules_js/eval@0.1.6/node_modules/acorn', ] -for (const p of is1pVirtualStoreDep_false) { - if (is1pVirtualStoreDep(p)) { +for (const p of is1pPackageStoreDep_false) { + if (is1pPackageStoreDep(p)) { console.error(`ERROR: expected ${p} to not be a 1p package store dep`) process.exit(1) }