Merge #82835

82835: cluster-ui: don't include protobufjs in bundle r=sjbarag a=sjbarag

Prior to the removal of yarn workspaces [1], compiled versions of
cluster-ui included a single copy of protobufjs because of the node
module resolution algorithm [2]. Essentially, yarn workspaces ensured
that only a single copy of [email protected] existed at
pkg/ui/node_modules/protobufjs, and that no copy existed in
pkg/ui/workspaces/db-console/src/js/node_modules. When cluster-ui's
webpack build process attempted to add crdb-protobuf-client to the
bundle, it detected the initial line of the protobuf client
(`import * as $protobuf from "protobufjs/minimal"`) and checked the
following directories for a protobufjs folder:

    pkg/ui/workspaces/db-console/src/js/node_modules (no protobufjs/)
    pkg/ui/workspaces/db-console/src/node_modules    (does not exist)
    pkg/ui/workspaces/db-console/node_modules        (no protobufjs/)
    pkg/ui/workspaces/node_modules                   (does not exist)
    pkg/ui/node_modules                              (v6.8.6 found!)

Cluster-ui also imported "protobufjs/minimal", and so the same process
repeated:

    pkg/ui/workspaces/cluster-ui/src/foo/node_modules (does not exist)
    pkg/ui/workspaces/cluster-ui/src/node_modules     (does not exist)
    pkg/ui/workspaces/cluster-ui/node_modules         (no protobufjs)
    pkg/ui/workspaces/node_modules                    (does not exist)
    pkg/ui/node_modules                               (v6.8.6 found!)

Since both cluster-ui and crdb-protobuf-client resolved
"protobufjs/minimal" to the same path, webpack considered those the same
dependency and only includes a single copy.

It's important at this point to understand _why_ cluster-ui imports
protobufjs at all, since crdb-protobuf-client is intended to hide the
use of protobufs from its consumers. Protobufjs uses `@protobufjs/inquire`
to load the module "long" (which provides support for 64 bit
two's-complement integer values in JavaScript [3]) by using an
obfuscated, `eval`-based require call rather than a standard `require()`
call or `import` statement [4]. This approach is intended to be hidden
from bundlers like webpack, but it's unclear why the protobufjs authors
think that's desirable. Regardless of the reason, protobufjs's
`util.Long` property is `null` when bundled with webpack, because
webpack is unable to provide the "long" module where `util.Long` is
initialized [5]. cluster-ui monkey-patches that util.Long property
immediately [6] in a way that webpack can support. And because
crdb-protobuf-client and cluster-ui share a single copy of the
`protobufjs` module, crdb-protobuf-client's copy of util.Long is
impacted as well!

When yarn workspaces were removed [1], this path overlapping no longer
applied. pkg/ui/workspaces/db-console/src/js/node_modules suddenly _did_
contain a copy of [email protected], and so did
pkg/ui/workspaces/cluster-ui/node_modules! When bundling, webpack
included both copies (despite the fact that they're equivalent),
ensuring cluster-ui and crdb-protobuf-client get their respective
copies. Visually:

```
  src/protobufInit.ts
  └ protobufjs/minimal -> ./node_modules/protobufjs
                          ^^^^^^^^^^^^^^^^^^^^^^^^^
                          util.Long monkey-patched

  src/foo.ts
  └ `@cockroachlabs/crdb-protobuf-client` -> ../db-console/src/js
    └ protobufjs/minimal -> ../db-console/src/js/node_modules/protobufjs
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                            util.Long not monkey-patched
```

db-console was still functional in this state because it included its
_own_ copy of crdb-protobuf-client as a direct dependency (which
includes its own copy of protobufjs) and its own protobufInit.ts [7].
Visually, db-console imports resolved to:

```
  src/protobufInit.ts
  └ protobufjs/minimal -> ./src/js/node_modules/protobufjs
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                          util.Long monkey-patched

  src/bar.ts
  └ `@cockroachlabs/crdb-protobuf-client` -> ./src/js
    └ protobufjs/minimal -> ./src/js/node_modules/protobufjs
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                            util.Long monkey-patched already,
                            which fixes cluster-ui's
                            src/foo.ts import (see above).
```

The direct dependency on crdb-protobuf-client coincidentally uses the
same path as the transitive dependency through cluster-ui, so webpack is
able to de-dupe them and all copies of protobufjs are conveniently
fixed.

Since CockroachDB Cloud doesn't take a manual dependency on
crdb-protobuf-client (it relies on the copy from cluster-ui) and also
doesn't attempt to monkey-patch protobufjs, CockroachDB Cloud isn't able
to ensure that crdb-protobuf-client's copy of util.Long has a value.
Preventing cluster-ui from packaging its own copy of protobufjs means
that its runtime monkey-patching in src/protobufInit.ts always applies
to the copy of protobufjs imported by crdb-protobuf-client, and thus
prevents having an un-patched copy. Don't include a second copy of
protobufjs in cluster-ui bundle.

[1] 5b6c271 (bazel: upgrade to rules_nodejs 5.4.2, 2022-05-17)
[2] https://nodejs.org/docs/latest-v16.x/api/modules.html#loading-from-node_modules-folders
[3] https://github.com/dcodeIO/long.js/tree/088e44e5e3343ef967698565678384fa474b003b
[4] https://github.com/protobufjs/protobuf.js/blob/2cdbba32da9951c1ff14e55e65e4a9a9f24c70fd/lib/inquire/index.js#L10-L17
[5] https://github.com/protobufjs/protobuf.js/blob/2cdbba32da9951c1ff14e55e65e4a9a9f24c70fd/src/util/minimal.js#L176-L182
[6] https://github.com/cockroachdb/cockroach/blob/aac9c44d1284fa93877815f61527a76e110dbda5/pkg/ui/workspaces/cluster-ui/src/protobufInit.ts
[7] https://github.com/cockroachdb/cockroach/blob/d82ac30cc7b3cb5566e347ae1fecd2025c5e0623/pkg/ui/workspaces/db-console/src/protobufInit.ts

Release note: None

Co-authored-by: Sean Barag <[email protected]>

pkg/ui/workspaces/cluster-ui/webpack.config.js

@@ -151,6 +151,9 @@ module.exports = (env, argv) => {
     // dependencies, which allows browsers to cache those libraries between builds.
     externals: {
       protobufjs: "protobufjs",
+      // Importing protobufjs/minimal resolves to the protobufjs module, but webpack's
+      // "externals" checking appears to be based on string comparisons.
+      "protobufjs/minimal": "protobufjs/minimal",
       react: {
         commonjs: "react",
         commonjs2: "react",