refactor(NODE-6549): use private properties for hex string cache

.evergreen/run-typescript.sh

@@ -22,8 +22,7 @@ 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
-rm -rf node_modules/@types/eslint # not a dependency we use, but breaks the build :(
-node $TSC bson.d.ts
+node $TSC bson.d.ts --target es2022 --module nodenext
 
 if [[ $TRY_COMPILING_LIBRARY != "false" ]]; then
     npm run build:ts

package.json

@@ -57,6 +57,7 @@
     "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"

src/objectid.ts

@@ -7,9 +7,6 @@ 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;
@@ -35,11 +32,20 @@ export class ObjectId extends BSONValue {
   /** @internal */
   private static index = Math.floor(Math.random() * 0xffffff);
 
-  static cacheHexString: boolean;
+  static cacheHexString: boolean = false;
 
   /** 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();
   /**
@@ -107,7 +113,7 @@ export class ObjectId extends BSONValue {
         this.buffer = ByteUtils.fromHex(workingId);
         // If we are caching the hex string
         if (ObjectId.cacheHexString) {
-          __idCache.set(this, workingId);
+          this.#cachedHexString = workingId;
         }
       } else {
         throw new BSONError(
@@ -130,7 +136,7 @@ export class ObjectId extends BSONValue {
   set id(value: Uint8Array) {
     this.buffer = value;
     if (ObjectId.cacheHexString) {
-      __idCache.set(this, ByteUtils.toHex(value));
+      this.#cachedHexString = ByteUtils.toHex(value);
     }
   }
 
@@ -159,15 +165,12 @@ export class ObjectId extends BSONValue {
 
   /** Returns the ObjectId id as a 24 lowercase character hex string representation */
   toHexString(): string {
-    if (ObjectId.cacheHexString) {
-      const __id = __idCache.get(this);
-      if (__id) return __id;
-    }
+    if (this.#cachedHexString) return this.#cachedHexString.toLowerCase();
 
     const hexString = ByteUtils.toHex(this.id);
 
     if (ObjectId.cacheHexString) {
-      __idCache.set(this, hexString);
+      this.#cachedHexString = hexString;
     }
 
     return hexString;
@@ -365,9 +368,13 @@ export class ObjectId extends BSONValue {
     return new ObjectId(doc.$oid);
   }
 
-  /** @internal */
+  /**
+   * @internal
+   *
+   * used for testing
+   */
   private isCached(): boolean {
-    return ObjectId.cacheHexString && __idCache.has(this);
+    return ObjectId.cacheHexString && this.#cachedHexString != null;
   }
 
   /**

test/node/object_id.test.ts

@@ -4,8 +4,55 @@ 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);
@@ -265,7 +312,7 @@ describe('ObjectId', function () {
     let a = 'AAAAAAAAAAAAAAAAAAAAAAAA';
     let b = new ObjectId(a);
     let c = b.equals(a); // => false
-    expect(true).to.equal(c);
+    expect(c).to.be.true;
 
     a = 'aaaaaaaaaaaaaaaaaaaaaaaa';
     b = new ObjectId(a);