bluesky-social / atproto-website

https://atproto.com
Other
214 stars 139 forks source link

`left` and `tree` in MST nodes are documented as "optional" but implemented as "nullable" #312

Open str4d opened 1 month ago

str4d commented 1 month ago

The documentation: https://github.com/bluesky-social/atproto-website/blob/0a7e9429edfa187135f06ded2dd38d55429385d9/content/specs/repository.md?plain=1#L87-L92

The implementation: https://github.com/bluesky-social/atproto/blob/4184a652225eaf2de5f4d4702562e84c2fd3fde7/packages/repo/src/mst/mst.ts#L46-L56

const subTreePointer = z.nullable(common.cid)
const treeEntry = z.object({
  p: z.number(), // prefix count of ascii chars that this key shares with the prev key
  k: common.bytes, // the rest of the key outside the shared prefix
  v: common.cid, // value
  t: subTreePointer, // next subtree (to the right of leaf)
})
const nodeData = z.object({
  l: subTreePointer, // left-most subtree
  e: z.array(treeEntry), //entries
})

"nullable" is used elsewhere in the same document, and the corresponding implementation in that case matches (although with different syntax that I haven't yet traced): https://github.com/bluesky-social/atproto-website/blob/0a7e9429edfa187135f06ded2dd38d55429385d9/content/specs/repository.md?plain=1#L52 https://github.com/bluesky-social/atproto/blob/4184a652225eaf2de5f4d4702562e84c2fd3fde7/packages/repo/src/types.ts#L17-L18

  // `prev` added for backwards compatibility with v2, no requirement of keeping around history
  prev: common.cid.nullable(),
bnewbold commented 1 month ago

I think you are probably right and these are nullable, but will double-check next week before updating the specs.