Skip to content
This repository was archived by the owner on Aug 24, 2021. It is now read-only.

Add flow type annotations. #47

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions .aegir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"use strict"

module.exports = {
// Have not used webpack so just copied proposed setup from
// https://babeljs.io/docs/setup/#installation
webpack: {
module: {
rules: [
{
test: /\.js$/,
exclude: /node_modules/,
loader: "babel-loader"
}
]
}
}
}
34 changes: 34 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"env": {
"development": {
"sourceMaps": "inline",
"comments": false,
"presets": [
[
"env",
{
"targets": {
"node": "current"
}
}
],
"flow-node"
]
},
"umd": {
"comments": false,
"presets": [
[
"env",
{
"modules": false,
"targets": {
"browsers": "last 2 versions"
}
}
],
"flow-node"
]
}
}
}
9 changes: 9 additions & 0 deletions .flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[ignore]
.*/node_modules/documentation/*

[libs]

[include]

[options]
suppress_comment= \\(.\\|\n\\)*\\@FlowIgnore
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -35,3 +35,4 @@ node_modules

dist
docs
lib
4 changes: 4 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{

Choose a reason for hiding this comment

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

I don't think this file is meant to be in this PR.

"editor.formatOnSave": false,
"json.format.enable": false
}
1 change: 1 addition & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ install:
- npm install

test_script:
- npm run build:node
- npm run test:node

build: off
34 changes: 28 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
@@ -2,19 +2,30 @@
"name": "multihashes",
"version": "0.4.13",
"description": "multihash implementation",
"main": "src/index.js",
"main": "lib/index.js",
"scripts": {
"lint": "aegir lint",
"lint": "lint-staged && npm run type-check",
"test:browser": "aegir test --target browser",
"test:node": "aegir test --target node",
"build": "aegir build",
"test": "aegir test",
"type-check": "flow check",
"build": "npm run build:node && BABEL_ENV=umd aegir build",
"test": "npm run build:node && aegir test",
"docs": "aegir docs",
"release": "aegir release --docs",
"release-minor": "aegir release --type minor --docs",
"release-major": "aegir release --type major --docs",
"coverage": "aegir coverage",
"coverage-publish": "aegir coverage --provider coveralls"
"coverage-publish": "aegir coverage --provider coveralls",
"build:types": "flow-copy-source --verbose src lib",
"build:lib": "babel --out-dir lib src",
"build:node": "npm run build:types && npm run build:lib",
"start": "flow-copy-source --watch --verbose src lib & babel --watch --out-dir lib src"

Choose a reason for hiding this comment

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

Would it be possible to also add the type checking to the npm start command? It would be nice to be warned if there are any type errors introduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice there should be impossible to introduce type errors as that would fail In CI and there for won’t be merged.

Also given that initail run of type checking takes quite bit time I don’t think running that on start would be a good option.

},
"lint-staged": {
"*.js": [
"prettier --no-semi --write",
"git add"

Choose a reason for hiding this comment

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

I had bad experiences with a lint-staged configuration like this in the past. The main problem is that when you only stage patches of a file which get changed by prettier --write afterwards the whole file is staged.

]
},
"pre-commit": [
"lint",
@@ -43,10 +54,21 @@
},
"devDependencies": {
"aegir": "^12.3.0",
"babel-cli": "6.26.0",
"babel-core": "6.26.0",
"babel-loader": "7.1.2",
"babel-preset-flow-node": "2.0.1",
"buffer-equal": "1.0.0",
"chai": "^4.1.2",
"dirty-chai": "^2.0.1",
"pre-commit": "^1.2.2"
"flow-bin": "^0.66.0",
"flow-copy-source": "^1.2.0",
"lint-staged": "^6.0.0",
"pre-commit": "^1.2.2",
"prettier": "1.10.2",
"rollup": "0.56.1",

Choose a reason for hiding this comment

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

Why was rollup introduced / where does it get used?

Copy link
Member

Choose a reason for hiding this comment

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

I wondered the same. I'm in the process of pushing an updated version of this PR. There I don't use the rollup module and it still seems to work.

"rollup.config.flow": "1.0.0",
"source-map-support": "0.5.3"
},
"contributors": [
"Alan Shaw <alan@tableflip.io>",
1,353 changes: 679 additions & 674 deletions src/constants.js

Large diffs are not rendered by default.

68 changes: 40 additions & 28 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,38 @@
// @flow

/**
* Multihash implementation in JavaScript.
*
* @module multihash
*/
'use strict'

const bs58 = require('bs58')
import bs58 from 'bs58'
import varint from 'varint'
import {names, codes, defaultLengths, type Name, type Code} from "./constants"
Copy link
Member

Choose a reason for hiding this comment

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

@Gozala @vmx given that this enables us to do dynamic imports, does this mean that we finally can also do treeshaking? (ipfs/js-ipfs#951)



const cs = require('./constants')
export {names, codes, defaultLengths}

exports.names = cs.names
exports.codes = cs.codes
exports.defaultLengths = cs.defaultLengths

const varint = require('varint')
export type Multihash = Buffer
export type MultihashPrefix = Buffer
export type B58Buffer = Buffer
export type HexString = string
export type MultihashData = {
code: Code,
name: Name,
length: number,
digest: Buffer
}

/**
* Convert the given multihash to a hex encoded string.
*
* @param {Buffer} hash
* @returns {string}
*/
exports.toHexString = function toHexString (hash) {
export function toHexString (hash:Multihash):HexString {
if (!Buffer.isBuffer(hash)) {
throw new Error('must be passed a buffer')
}
@@ -35,17 +46,18 @@ exports.toHexString = function toHexString (hash) {
* @param {string} hash
* @returns {Buffer}
*/
exports.fromHexString = function fromHexString (hash) {
export function fromHexString (hash:HexString):Multihash {
return Buffer.from(hash, 'hex')
}

export type B58String = string
/**
* Convert the given multihash to a base58 encoded string.
*
* @param {Buffer} hash
* @returns {string}
*/
exports.toB58String = function toB58String (hash) {
export function toB58String (hash:Multihash):B58String {
if (!Buffer.isBuffer(hash)) {
throw new Error('must be passed a buffer')
}
@@ -59,7 +71,7 @@ exports.toB58String = function toB58String (hash) {
* @param {string|Buffer} hash
* @returns {Buffer}
*/
exports.fromB58String = function fromB58String (hash) {
export function fromB58String (hash:B58String|B58Buffer):Multihash {
let encoded = hash
if (Buffer.isBuffer(hash)) {
encoded = hash.toString()
@@ -74,7 +86,7 @@ exports.fromB58String = function fromB58String (hash) {
* @param {Buffer} buf
* @returns {{code: number, name: string, length: number, digest: Buffer}} result
*/
exports.decode = function decode (buf) {
export function decode (buf:Multihash):MultihashData {
if (!(Buffer.isBuffer(buf))) {
throw new Error('multihash must be a Buffer')
}
@@ -84,7 +96,7 @@ exports.decode = function decode (buf) {
}

let code = varint.decode(buf)
if (!exports.isValidCode(code)) {
if (!isValidCode(code)) {
throw new Error(`multihash unknown function code: 0x${code.toString(16)}`)
}
buf = buf.slice(varint.decode.bytes)
@@ -101,7 +113,7 @@ exports.decode = function decode (buf) {

return {
code: code,
name: cs.codes[code],
name: codes[code],
length: len,
digest: buf
}
@@ -117,13 +129,13 @@ exports.decode = function decode (buf) {
* @param {number} [length]
* @returns {Buffer}
*/
exports.encode = function encode (digest, code, length) {
export function encode (digest:Buffer, code:Code, length:number):Multihash {

Choose a reason for hiding this comment

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

I'm working on converting js-multihashing and I think that code parameter here should be code: Code | Name

export function encode (digest:Buffer, code:Code | Number, length:number):Multihash {

if (!digest || !code) {
throw new Error('multihash encode requires at least two args: digest, code')
}

// ensure it's a hashfunction code.
const hashfn = exports.coerceCode(code)
const hashfn = coerceCode(code)

if (!(Buffer.isBuffer(digest))) {
throw new Error('digest should be a Buffer')
@@ -150,21 +162,22 @@ exports.encode = function encode (digest, code, length) {
* @param {string|number} name
* @returns {number}
*/
exports.coerceCode = function coerceCode (name) {
// TODO: Sholud we just allow Name here instead ?
export function coerceCode (name:Name|Code):Code {
let code = name

if (typeof name === 'string') {
if (!cs.names[name]) {
if (!names[name]) {
throw new Error(`Unrecognized hash function named: ${name}`)
}
code = cs.names[name]
code = names[name]
}

if (typeof code !== 'number') {
throw new Error(`Hash function code should be a number. Got: ${code}`)
}

if (!cs.codes[code] && !exports.isAppCode(code)) {
if (!codes[code] && !exports.isAppCode(code)) {
throw new Error(`Unrecognized function code: ${code}`)
}

@@ -177,7 +190,7 @@ exports.coerceCode = function coerceCode (name) {
* @param {number} code
* @returns {boolean}
*/
exports.isAppCode = function appCode (code) {
export function isAppCode (code:number):boolean {
return code > 0 && code < 0x10
}

@@ -187,12 +200,12 @@ exports.isAppCode = function appCode (code) {
* @param {number} code
* @returns {boolean}
*/
exports.isValidCode = function validCode (code) {
if (exports.isAppCode(code)) {
export function isValidCode(code:number):boolean {
if (isAppCode(code)) {
return true
}

if (cs.codes[code]) {
if (codes[(code:any)]) {
return true
}

@@ -206,19 +219,18 @@ exports.isValidCode = function validCode (code) {
* @returns {undefined}
* @throws {Error}
*/
function validate (multihash) {
exports.decode(multihash) // throws if bad.
export function validate (multihash:Buffer) {
decode(multihash) // throws if bad.
}
exports.validate = validate

/**
* Returns a prefix from a valid multihash. Throws an error if it is not valid.
*
* @param {Buffer} multihash
* @returns {undefined}
* @returns {Buffer}
* @throws {Error}
*/
exports.prefix = function prefix (multihash) {
export function prefix (multihash:Multihash):MultihashPrefix {
validate(multihash)

return multihash.slice(0, 2)
4 changes: 2 additions & 2 deletions test/index.spec.js
Original file line number Diff line number Diff line change
@@ -9,8 +9,8 @@ const expect = chai.expect
const bufeq = require('buffer-equal')
const bs58 = require('bs58')

const mh = require('../src')
const constants = require('../src/constants')
const mh = require('../lib')
const constants = require('../lib/constants')
const validCases = require('./fixtures/valid')
const invalidCases = require('./fixtures/invalid')

4 changes: 4 additions & 0 deletions test/node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// TODO: Instead mocha should be passed additional arg
// `--require source-map-support/register` but as things stand now there is
// no way to do it without changes to aegir so we do this instead for now.
require("source-map-support/register")