Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add encoding and decoding methods to IdInternal #11

Merged
merged 1 commit into from
Jan 21, 2022
Merged

Conversation

tegefaulkes
Copy link
Contributor

@tegefaulkes tegefaulkes commented Jan 20, 2022

Description

This adds some QOL features to the Id and IdInternal.

Issues Fixed

Tasks

  1. Add type conversion methods to InternalId
  2. add support for working with and converting to opaque types of Id
  3. expand testing to test new features.

Final checklist

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

@tegefaulkes tegefaulkes added the development Standard development label Jan 20, 2022
@tegefaulkes tegefaulkes self-assigned this Jan 20, 2022
tests/Id.test.ts Outdated Show resolved Hide resolved
tests/Id.test.ts Outdated Show resolved Hide resolved
src/Id.ts Outdated Show resolved Hide resolved
tests/Id.test.ts Outdated Show resolved Hide resolved
@tegefaulkes
Copy link
Contributor Author

I can't quite get the encode/decode feature to work nicely so we're stuck with to/fromMultibase and wrapping them.

But I have added this neat feature were we can create an Opaque type directly when converting by doing

// where
type NodeId = Opaque<'nodeId', Id>;

// we can do
const nodeId = internalId.fromBuffer<NodeId>(buffer);
// or
const nodeid2: NodeId = InternalId.fromBuffer(buffer);

Any method that returns an Id type can support this. so the generators like IdRandom can return our opaque type directly aswell. This just saves us having to use id as OpaqueId.

@tegefaulkes
Copy link
Contributor Author

This is still a WIP. I'm still making changes.

@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Jan 20, 2022

It may be possible to extend InternalId and set a Multibase format with;

type NodeId = InternalNodeid & number;
class internalNodeId extends InternalId {
    public readonly base: MultibaseFormats = 'base58btc';
}

The encode and decode functions can be built into InternalNodeId that way. Unfortunately I can't use the class generic types for the static methods so we'd still have to do InternalNodeId.fromBuffer<NodeId> to get it to return a NodeId. or we'd have to override all of the static methods to return a NodeId.

The alternative is to just have the utility functions for each domain to encode/decode.

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jan 20, 2022

It's not necessary, it would just be a nice to have. We can proceed to merge this with just the relevant instance and static methods added in here.

We keep using nodesUtils and vaultsUtils for encoding/decoding relevant to the domains.

@CMCDragonkai CMCDragonkai changed the title QOL conversion features for Id Add encoding and decoding methods to IdInternal Jan 20, 2022
@CMCDragonkai
Copy link
Member

@tegefaulkes Changed PR title to be more clearer. You should try to be more descriptive on those PR names.

tests/Id.test.ts Outdated Show resolved Hide resolved
@tegefaulkes
Copy link
Contributor Author

Examples of new features.

All of the type coverters are in one place now in the IdInternal class.

IdInternal.fromBuffer();
IdInternal.fromString();
IdInternal.fromUUID();
IdInternal.fromMultibase();

const id = IdInternal.create();

id.toBuffer();
id.toString();
id.toUUID();
id.toMultibase('someRandomBase');

Any constructors for a Id can create an opaque type directly now by using

type OpaqueId<'opaqueId', Id>
const opaqueId = IdInternal.create<OpaqueId>();
const opaqueId: OpaqueId = idInternal.create();

// Same for conversions from other types
const opaqueId = IdInternal.fromBuffer<OpaqueId>();
const opaqueId: OpaqueId = IdInternal.fromString();

// The original way still works 
const id = IdInternal.create() as OpaqueId;

The id generators can do the same thing.

type OpaqueId = Opaque<'opaque', Id>;
const idGen = new IdRandom<OpaqueId>();

// ids has type OpaqueId[]
 const ids = [...utils.take(idGen, 10)];

@tegefaulkes
Copy link
Contributor Author

All review comments are resolved. should be good to merge.

@CMCDragonkai CMCDragonkai merged commit d94c473 into master Jan 21, 2022
@CMCDragonkai CMCDragonkai deleted the type_methods branch January 21, 2022 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

Successfully merging this pull request may close these issues.

2 participants