Revert "refactor(NODE-6549): use private properties for hex string cache (#830)"

This reverts commit 8664d78.

Author: baileympearson

.evergreen/run-typescript.sh

@@ -22,7 +22,8 @@ echo "Typescript $($TSC -v)"
 echo "import * as BSON from '.'" > file.ts && node $TSC --noEmit --traceResolution file.ts | grep 'bson.d.ts' && rm file.ts
 
 # check compilation
-node $TSC bson.d.ts --target es2022 --module nodenext
+rm -rf node_modules/@types/eslint # not a dependency we use, but breaks the build :(
+node $TSC bson.d.ts
 
 if [[ $TRY_COMPILING_LIBRARY != "false" ]]; then
     npm run build:ts

package-lock.json

@@ -57,7 +57,6 @@
     "tar": "^7.4.3",
     "ts-node": "^10.9.2",
     "tsd": "^0.33.0",
-    "tslib": "^2.8.1",
     "typescript": "^5.8.3",
     "typescript-cached-transpile": "0.0.6",
     "uuid": "^11.1.0"

package.json

@@ -7,6 +7,9 @@ import { NumberUtils } from './utils/number_utils';
 // Unique sequence for the current process (initialized on first use)
 let PROCESS_UNIQUE: Uint8Array | null = null;
 
+/** ObjectId hexString cache @internal */
+const __idCache = new WeakMap(); // TODO(NODE-6549): convert this to #__id private field when target updated to ES2022
+
 /** @public */
 export interface ObjectIdLike {
   id: string | Uint8Array;
@@ -32,20 +35,11 @@ export class ObjectId extends BSONValue {
   /** @internal */
   private static index = Math.floor(Math.random() * 0xffffff);
 
-  static cacheHexString: boolean = false;
+  static cacheHexString: boolean;
 
   /** ObjectId Bytes @internal */
   private buffer!: Uint8Array;
 
-  /**
-   * If hex string caching is enabled, contains the cached hex string.  Otherwise, is null.
-   *
-   * Note that #hexString is populated lazily, and as a result simply checking `this.#hexString != null` is
-   * not sufficient to determine if caching is enabled.  `ObjectId.prototype.isCached()` can be used to
-   * determine if the hex string has been cached yet for an ObjectId.
-   */
-  #cachedHexString: string | null = null;
-
   /** To generate a new ObjectId, use ObjectId() with no argument. */
   constructor();
   /**
@@ -113,7 +107,7 @@ export class ObjectId extends BSONValue {
         this.buffer = ByteUtils.fromHex(workingId);
         // If we are caching the hex string
         if (ObjectId.cacheHexString) {
-          this.#cachedHexString = workingId;
+          __idCache.set(this, workingId);
         }
       } else {
         throw new BSONError(
@@ -136,7 +130,7 @@ export class ObjectId extends BSONValue {
   set id(value: Uint8Array) {
     this.buffer = value;
     if (ObjectId.cacheHexString) {
-      this.#cachedHexString = ByteUtils.toHex(value);
+      __idCache.set(this, ByteUtils.toHex(value));
     }
   }
 
@@ -165,12 +159,15 @@ export class ObjectId extends BSONValue {
 
   /** Returns the ObjectId id as a 24 lowercase character hex string representation */
   toHexString(): string {
-    if (this.#cachedHexString) return this.#cachedHexString.toLowerCase();
+    if (ObjectId.cacheHexString) {
+      const __id = __idCache.get(this);
+      if (__id) return __id;
+    }
 
     const hexString = ByteUtils.toHex(this.id);
 
     if (ObjectId.cacheHexString) {
-      this.#cachedHexString = hexString;
+      __idCache.set(this, hexString);
     }
 
     return hexString;
@@ -368,13 +365,9 @@ export class ObjectId extends BSONValue {
     return new ObjectId(doc.$oid);
   }
 
-  /**
-   * @internal
-   *
-   * used for testing
-   */
+  /** @internal */
   private isCached(): boolean {
-    return ObjectId.cacheHexString && this.#cachedHexString != null;
+    return ObjectId.cacheHexString && __idCache.has(this);
   }
 
   /**

src/objectid.ts

@@ -4,55 +4,8 @@ import * as util from 'util';
 import { expect } from 'chai';
 import { bufferFromHexArray } from './tools/utils';
 import { isBufferOrUint8Array } from './tools/utils';
-import { test } from 'mocha';
 
 describe('ObjectId', function () {
-  describe('hex string caching does not impact deep equality', function () {
-    const original = ObjectId.cacheHexString;
-    before(function () {
-      ObjectId.cacheHexString = true;
-    });
-    after(function () {
-      ObjectId.cacheHexString = original;
-    });
-    test('no hex strings cached', function () {
-      const id = new ObjectId();
-      const id2 = new ObjectId(id.id);
-
-      // @ts-expect-error isCached() is internal
-      expect(id.isCached()).to.be.false;
-      // @ts-expect-error isCached() is internal
-      expect(id2.isCached()).to.be.false;
-
-      expect(new ObjectId(id.id)).to.deep.equal(id);
-    });
-
-    test('one id with cached hex string, one without', function () {
-      const id = new ObjectId();
-      const id2 = new ObjectId(id.id);
-      id2.toHexString();
-
-      // @ts-expect-error isCached() is internal
-      expect(id.isCached()).to.be.false;
-      // @ts-expect-error isCached() is internal
-      expect(id2.isCached()).to.be.true;
-
-      expect(id).to.deep.equal(id2);
-    });
-
-    test('both with cached hex string', function () {
-      const id = new ObjectId();
-      const id2 = new ObjectId(id.toHexString());
-
-      // @ts-expect-error isCached() is internal
-      expect(id.isCached()).to.be.true;
-      // @ts-expect-error isCached() is internal
-      expect(id2.isCached()).to.be.true;
-
-      expect(id).to.deep.equal(id2);
-    });
-  });
-
   describe('static createFromTime()', () => {
     it('creates an objectId with user defined value in the timestamp field', function () {
       const a = ObjectId.createFromTime(1);
@@ -312,7 +265,7 @@ describe('ObjectId', function () {
     let a = 'AAAAAAAAAAAAAAAAAAAAAAAA';
     let b = new ObjectId(a);
     let c = b.equals(a); // => false
-    expect(c).to.be.true;
+    expect(true).to.equal(c);
 
     a = 'aaaaaaaaaaaaaaaaaaaaaaaa';
     b = new ObjectId(a);