Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

feat: add flow typedefs #79

Merged
merged 1 commit into from
Apr 4, 2019
Merged

feat: add flow typedefs #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .flowconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[ignore]

[include]

[libs]

[lints]

[options]

[strict]
32 changes: 32 additions & 0 deletions src/index.js.flow
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// @flow strict

export type Version = 0 | 1
export type Codec = string
export type Multihash = Buffer
export type BaseEncodedString = string

declare class CID<a> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gozala generics aren't a requirement for class defs are they? I don't see a being used in CID and am not sure how CID could be a generic container so does <a> mean something else here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gozala generics aren't a requirement for class defs are they?

Correct, they are not required

I don't see a being used in CID and am not sure how CID could be a generic container so does mean something else here?

On my phone so can’t provide links but this is related to our other discussions on decode / encode. CID<a> implies here that it’s address for node a which in turn would allow get(CID<a>):Promise<a> without type parameter a it wouldn’t be possible to express that type of relationship.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more specific: indeed a type param is not used here, but it’s there because users of this lib are expected to make use of that parameter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wow, that's pretty funky. So tools that use flow definitions would follow that all the way through such a call chain and enforce that, or is this simply for the sake of descriptive completeness?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flow type-checker will indeed catch and report any miss-matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW that’s also a weak point of TS (at least it was) as it throws away unused type parameters not used in the class / function definitions while flow keeps them for inference at call-sites. It may not seem that important but in practice it makes a huge difference

constructor(Version, Codec, Multihash): void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CID now takes an optional multibaseName string as the 4th param as of #77

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out @olizilla, I'll create a followup pull to add that param.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olizilla @mikeal Can I assume that there is a consensus on wanting to keep the typedefs though ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there's consensus, but I'm the lead maintainer and I'd like to keep them :)

Copy link
Contributor Author

@Gozala Gozala Apr 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmx I think @mikeal was just looking +1 from someone, and it sounds like we have one. Thanks

constructor(BaseEncodedString): void;
constructor(Buffer): void;

+codec: Codec;
+multihash: Multihash;
+buffer: Buffer;
+prefix: Buffer;

toV0(): CID<a>;
toV1(): CID<a>;
toBaseEncodedString(base?: string): BaseEncodedString;
toString(): BaseEncodedString;
toJSON(): { codec: Codec, version: Version, hash: Multihash };

equals(mixed): boolean;

static codecs: { [string]: Codec };
static isCID(mixed): boolean;
static validateCID(mixed): void;
}

export default CID
export type { CID }