cache hex string by default and use private property

Author: baileympearson

package-lock.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"

package.json

@@ -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,28 @@ export class ObjectId extends BSONValue {
   /** @internal */
   private static index = Math.floor(Math.random() * 0xffffff);
 
-  static cacheHexString: boolean;
+  static cacheHexString: boolean = true;
 
   /** 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.
+   */
+  #_hexString: string | null = null;
+
+  get #hexString(): string | null {
+    return this.#_hexString;
+  }
+
+  set #hexString(s: string) {
+    this.#_hexString = s.toLowerCase();
+  }
+
   /** To generate a new ObjectId, use ObjectId() with no argument. */
   constructor();
   /**
@@ -107,7 +121,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.#hexString = workingId;
         }
       } else {
         throw new BSONError(
@@ -130,7 +144,7 @@ export class ObjectId extends BSONValue {
   set id(value: Uint8Array) {
     this.buffer = value;
     if (ObjectId.cacheHexString) {
-      __idCache.set(this, ByteUtils.toHex(value));
+      this.#hexString = ByteUtils.toHex(value);
     }
   }
 
@@ -159,15 +173,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.#hexString) return this.#hexString;
 
     const hexString = ByteUtils.toHex(this.id);
 
     if (ObjectId.cacheHexString) {
-      __idCache.set(this, hexString);
+      this.#hexString = hexString;
     }
 
     return hexString;
@@ -365,9 +376,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.#hexString != null;
   }
 
   /**

src/objectid.ts

@@ -1572,6 +1572,7 @@ describe('BSON', function () {
    * @ignore
    */
   it('ObjectId should have a correct cached representation of the hexString', function (done) {
+    const cacheHexString = ObjectId.cacheHexString;
     ObjectId.cacheHexString = true;
     // generated ObjectID uses lazy caching
     var a = new ObjectId();
@@ -1622,6 +1623,8 @@ describe('BSON', function () {
     a.toHexString();
     expect(a.isCached()).to.be.false;
 
+    ObjectId.cacheHexString = cacheHexString;
+
     done();
   });
 

test/node/bson_test.js

@@ -4,8 +4,48 @@ 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 () {
+    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 +305,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);