[Bug]: Upgrading from v0.16.0 to v0.18.0 causes non-browser scripts to throw an error

[BillyBlaze]

What happened?

I am upgrading aspect_rules_esbuild from version v0.16.0 to v0.18.0. Everything is working in v16 however in v18 I am getting the following errors from esbuild. (I couldn't check v0.17.0 because its not in the bazel registry)

✘ [ERROR] Could not read from file: /.../bin/node_modules/.aspect_rules_js/object-inspect@1.13.1/node_modules/object-inspect/util.inspect

    node_modules/.aspect_rules_js/object-inspect@1.13.1/node_modules/object-inspect/index.js:68:26:
      68 │ var utilInspect = require('./util.inspect');`

This happens because the object-inspect main file is requiring the file that is marked as non-browser.

This dependency is sadly commonly used in multiple packages such as react-dev-utils, cypress or webpack

Version

Development (host) and target OS/architectures:
Linux 5.15.146.1-microsoft-standard-WSL2 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Output of bazel --version:
bazel 7.0.2

Version of the Aspect rules, or other relevant rules from your MODULE.bazel file:

• aspect_rules_esbuild @ 0.18.0
• aspect_rules_js @ 1.38.0

Language(s) and/or frameworks involved:
pnpm

How to reproduce

No response

Any other information?

No response

[BillyBlaze]

Still affected, the object-inspect author isn't going to fix this on their end: inspect-js/object-inspect#41

[jbedard]

Is this a rules_esbuild issue or a generic esbuild issue?

[BillyBlaze]

Sorry, I work in a large organization and have my notifications turned off, so I missed your question.

When I try to run this in bazel I still get the error as listed in the first post. However if add pnpm script and run it via pnpm than I do not get an error and its compiling the JS files.

❯ pnpm --filter @redacted/redacted-e2e esbuild-test

> @redacted/redacted-e2e@1.0.0 esbuild-test /home/redacted/e2e
> esbuild ./cypress/integration/dashboard.spec --outfile=test/browser.mjs  --bundle --minify --format=iife --platform=browser


  test/browser.mjs  9.3kb

⚡ Done in 7ms

So I am still not entirely sure who the culprit is between these 3 (object-inspect, rules_esbuild or esbuild), I dont get why object-inspect does this behaivor, but it seems esbuild can handle it while rules_esbuild can't.

For anyone who faces this issue, we have this patch applied in our project:

diff --git a/index.js b/index.js
index a4b2d4cd850adb5cda64eb9e5692457a73c0947e..a0bf7c8f4c2c2b6f96a8ccd815dfcc97389bdb84 100644
--- a/index.js
+++ b/index.js
@@ -65,8 +65,8 @@ function addNumericSeparator(num, str) {
     return $replace.call(str, sepRegex, '$&_');
 }
 
-var utilInspect = require('./util.inspect');
-var inspectCustom = utilInspect.custom;
+var utilInspect = {}
+var inspectCustom = null
 var inspectSymbol = isSymbol(inspectCustom) ? inspectCustom : null;
 
 var quotes = {