From 3a19a86d798936c9fda0f97439c4eda8c5b5cfe9 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Thu, 22 Sep 2022 18:14:20 +0100 Subject: [PATCH 1/4] fix: remove use of Object.defineProperties in CID class `Object.defineProperties` is a performance bottleneck in applications that create lots and lots of CIDs (e.g. IPFS) so this PR removes it. The `asCID` property is changed to be a [private class field](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields) which requires increasing the minimum supported EcmaScript version but I don't know if that's a big deal or not. It does seem to make the property non-enumerable though. The CID class now implements a `Link` interface that has public `byteOffset` and `byteLength` properties so these become regular properties `code`, `version`, `multihash` and `bytes` become writable/configurable but they are marked with `@readonly` so maybe that's enough? Fixes #200 --- package.json | 3 ++- src/cid.js | 22 +++++++--------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 4aec0e42..69368fd2 100644 --- a/package.json +++ b/package.json @@ -157,7 +157,8 @@ "eslintConfig": { "extends": "ipfs", "parserOptions": { - "sourceType": "module" + "sourceType": "module", + "ecmaVersion": 13 } }, "release": { diff --git a/src/cid.js b/src/cid.js index cbb5fd80..ee563f52 100644 --- a/src/cid.js +++ b/src/cid.js @@ -61,6 +61,9 @@ const baseCache = cid => { */ export class CID { + /** @type {CID} */ + #asCID + /** * @param {Version} version - Version of the CID * @param {Format} code - Code of the codec content is encoded in, see https://github.com/multiformats/multicodec/blob/master/table.csv @@ -86,20 +89,11 @@ export class CID { // Circular reference /** @readonly */ - this.asCID = this - - // Configure private properties - Object.defineProperties(this, { - byteOffset: hidden, - byteLength: hidden, - - code: readonly, - version: readonly, - multihash: readonly, - bytes: readonly, + this.#asCID = this + } - asCID: hidden - }) + get asCID () { + return this.#asCID } /** @@ -568,5 +562,3 @@ const encodeCID = (version, code, multihash) => { } const cidSymbol = Symbol.for('@ipld/js-cid/CID') -const readonly = { writable: false, configurable: false, enumerable: true } -const hidden = { writable: false, enumerable: false, configurable: false } From ce644019d0e9d638ecdffe837d6fff7e627dfe8b Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 23 Sep 2022 07:20:47 +0100 Subject: [PATCH 2/4] test: check for non-enumerability of asCID property --- test/test-cid.spec.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/test-cid.spec.js b/test/test-cid.spec.js index 5bdd3b1d..47754aa8 100644 --- a/test/test-cid.spec.js +++ b/test/test-cid.spec.js @@ -696,4 +696,12 @@ describe('CID', () => { const encoded = varint.encodeTo(2, new Uint8Array(32)) assert.throws(() => CID.decode(encoded), 'Invalid CID version 2') }) + + it('asCID is non-enumerable', () => { + const cid = CID.parse('bafybeif2pall7dybz7vecqka3zo24irdwabwdi4wc55jznaq75q7eaavvu') + + assert.isFalse(Object.prototype.propertyIsEnumerable.call(cid, 'asCID')) + assert.isFalse(Object.keys(cid).includes('asCID')) + assert.equal(cid.asCID, cid) + }) }) From 481b14077668b8ec800220aa12c79ffb3de8fbd5 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Tue, 4 Oct 2022 15:28:36 -0700 Subject: [PATCH 3/4] chore: add test for structural copying (#206) * chore: add test for structural copying * fix: remove generated import * fix: lint errors * fix: another attempt to make eslint happy * chore: and another attemp to make eslint happy --- test/test-cid.spec.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test-cid.spec.js b/test/test-cid.spec.js index 47754aa8..d203c17d 100644 --- a/test/test-cid.spec.js +++ b/test/test-cid.spec.js @@ -704,4 +704,14 @@ describe('CID', () => { assert.isFalse(Object.keys(cid).includes('asCID')) assert.equal(cid.asCID, cid) }) + + it('CID can be moved across JS realms', async () => { + const cid = CID.parse('bafybeif2pall7dybz7vecqka3zo24irdwabwdi4wc55jznaq75q7eaavvu') + const { port1: sender, port2: receiver } = new MessageChannel() + sender.postMessage(cid) + const cid2 = await new Promise((resolve) => { + receiver.onmessage = (event) => { resolve(event.data) } + }) + assert.equal(cid2.asCID, cid2) + }) }) From fb695442c1dde262b61d9431ec94b5c6a9a80011 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Sat, 8 Oct 2022 14:00:00 +1100 Subject: [PATCH 4/4] fix: make CID#asCID a regular property --- package.json | 3 +-- src/cid.js | 9 +-------- test/test-cid.spec.js | 12 +++--------- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index 69368fd2..4aec0e42 100644 --- a/package.json +++ b/package.json @@ -157,8 +157,7 @@ "eslintConfig": { "extends": "ipfs", "parserOptions": { - "sourceType": "module", - "ecmaVersion": 13 + "sourceType": "module" } }, "release": { diff --git a/src/cid.js b/src/cid.js index ee563f52..4736898f 100644 --- a/src/cid.js +++ b/src/cid.js @@ -61,9 +61,6 @@ const baseCache = cid => { */ export class CID { - /** @type {CID} */ - #asCID - /** * @param {Version} version - Version of the CID * @param {Format} code - Code of the codec content is encoded in, see https://github.com/multiformats/multicodec/blob/master/table.csv @@ -89,11 +86,7 @@ export class CID { // Circular reference /** @readonly */ - this.#asCID = this - } - - get asCID () { - return this.#asCID + this.asCID = this } /** diff --git a/test/test-cid.spec.js b/test/test-cid.spec.js index d203c17d..6de0f6c5 100644 --- a/test/test-cid.spec.js +++ b/test/test-cid.spec.js @@ -697,14 +697,6 @@ describe('CID', () => { assert.throws(() => CID.decode(encoded), 'Invalid CID version 2') }) - it('asCID is non-enumerable', () => { - const cid = CID.parse('bafybeif2pall7dybz7vecqka3zo24irdwabwdi4wc55jznaq75q7eaavvu') - - assert.isFalse(Object.prototype.propertyIsEnumerable.call(cid, 'asCID')) - assert.isFalse(Object.keys(cid).includes('asCID')) - assert.equal(cid.asCID, cid) - }) - it('CID can be moved across JS realms', async () => { const cid = CID.parse('bafybeif2pall7dybz7vecqka3zo24irdwabwdi4wc55jznaq75q7eaavvu') const { port1: sender, port2: receiver } = new MessageChannel() @@ -712,6 +704,8 @@ describe('CID', () => { const cid2 = await new Promise((resolve) => { receiver.onmessage = (event) => { resolve(event.data) } }) - assert.equal(cid2.asCID, cid2) + assert.strictEqual(cid2.asCID, cid2) + sender.close() + receiver.close() }) })