Skip to content

Commit

Permalink
fix: use slash as flag that an object is a CID (#217)
Browse files Browse the repository at this point in the history
As per #212 making `asCID` enumerable breaks tests where modules don't handle self-referential data properly.

As proposed in #213 this swaps `cid.CID === cid` for `cid['/'] === cid.bytes` as a mechanism to tell consumers that the object in question is a `CID` which lets them write CBOR with the correct tags, for example.

Fixes #212
Closes #213
  • Loading branch information
achingbrain committed Oct 19, 2022
1 parent a43ffff commit 1cec619
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 12 deletions.
33 changes: 24 additions & 9 deletions src/cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,20 @@ export class CID {
/** @readonly */
this.bytes = bytes

// Circular reference
// flag to serializers that this is a CID and
// should be treated specially
/** @readonly */
this.asCID = this
this['/'] = bytes
}

/**
* Signalling `cid.asCID === cid` has been replaced with `cid['/'] === cid.bytes`
* please either use `CID.asCID(cid)` or switch to new signalling mechanism
*
* @deprecated
*/
get asCID () {
return this
}

// ArrayBufferView
Expand Down Expand Up @@ -229,24 +240,28 @@ export class CID {
* @returns {CID<Data, Format, Alg, Version>|null}
*/
static asCID (input) {
if (input == null) {
return null
}

const value = /** @type {any} */ (input)
if (value instanceof CID) {
// If value is instance of CID then we're all set.
return value
} else if (value != null && value.asCID === value) {
// If value isn't instance of this CID class but `this.asCID === this` is
// true it is CID instance coming from a different implementation (diff
// version or duplicate). In that case we rebase it to this `CID`
// implementation so caller is guaranteed to get instance with expected
// API.
} else if ((value['/'] != null && value['/'] === value.bytes) || value.asCID === value) {
// If value isn't instance of this CID class but `this.asCID === this` or
// `value['/'] === value.bytes` is true it is CID instance coming from a
// different implementation (diff version or duplicate). In that case we
// rebase it to this `CID` implementation so caller is guaranteed to get
// instance with expected API.
const { version, code, multihash, bytes } = value
return new CID(
version,
code,
/** @type {API.MultihashDigest<Alg>} */ (multihash),
bytes || encodeCID(version, code, multihash.bytes)
)
} else if (value != null && value[cidSymbol] === true) {
} else if (value[cidSymbol] === true) {
// If value is a CID from older implementation that used to be tagged via
// symbol we still rebase it to the this `CID` implementation by
// delegating that to a constructor.
Expand Down
21 changes: 19 additions & 2 deletions src/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,25 @@ export const create = (code, digest) => CID.create(1, code, digest)
* @param {unknown|L} value
* @returns {value is L & CID}
*/
export const isLink = value =>
value != null && /** @type {{asCID: unknown}} */ (value).asCID === value
export const isLink = value => {
if (value == null) {
return false
}

const withSlash = /** @type {{'/'?: Uint8Array, bytes: Uint8Array}} */ (value)

if (withSlash['/'] != null && withSlash['/'] === withSlash.bytes) {
return true
}

const withAsCID = /** @type {{'asCID'?: unknown}} */ (value)

if (withAsCID.asCID === value) {
return true
}

return false
}

/**
* Takes cid in a string representation and creates an instance. If `base`
Expand Down
2 changes: 1 addition & 1 deletion test/test-cid.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,9 @@ describe('CID', () => {
const cid2 = await new Promise((resolve) => {
receiver.onmessage = (event) => { resolve(event.data) }
})
assert.strictEqual(cid2.asCID, cid2)
sender.close()
receiver.close()
assert.strictEqual(cid2['/'], cid2.bytes)
})

describe('decode', () => {
Expand Down

0 comments on commit 1cec619

Please sign in to comment.