Skip to content

Conversation

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Jan 27, 2022

Description

This adds the methods of toJSON, fromJSONand equals.

The JSON methods matches the behaviour of Node's buffer. It allows us to marshal IdInternal into JSON and back with the appropriate reviver.

    const id = IdInternal.create([97, 98, 99]);
    const json = JSON.stringify(id);
    const id_ = JSON.parse(json, (k, v) => {
      return IdInternal.fromJSON(v) ?? v;
    });

Notice that JSON.stringify does not need a special replacer. This is already done by having a toJSON method in the IdInternal class.

In the case of JSON.parse, when it will convert any { type: 'IdInternal', data: [ ... ] } into the IdInternal object.

This is important as in some cases, when storing Ids where JSON is expected, this allow us to unmarshal back to the canonical binary ID representation rather than always encoding it as a string.

The above reviver will default if it's not an valid IdInternal object.

Additionally the equals method provides an efficient way to check for IdInternal equality.

    const id1 = IdInternal.create([97, 98, 99]);
    const id2 = IdInternal.create([97, 98, 99]);
    const id3 = IdInternal.create([97, 98, 100]);
    expect(id1.equals(id2)).toBe(true);
    expect(id1.equals(id3)).toBe(false);

Issues Fixed

Tasks

  1. Added toJSON and fromJSON and tests
  2. Added equals and tests
  3. Ensured that fromJSON can be used inside the reviver of JSON.parse

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

@tegefaulkes @joshuakarp. This is ready to be used and will be released as 3.3.0. You can replace any equality comparison in the src code with equals instead of converting to binary string.

Furthermore, the toJSON may not be used for now because you're using NodeIdEncoded, but this gives us the opportunity to change if we need to.

@CMCDragonkai
Copy link
Member Author

Some fixes for the previous PR too.

  1. fromString doesn't return undefined, any binary string is valid Id
  2. fromBuffer doesn't return undefined, any buffer is valid Id

This means some ! assertions may not be needed.

@tegefaulkes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants