{"action":"created","issue":{"url":"https://api.github.com/repos/decentraland/web3-ts/issues/2","repository_url":"https://api.github.com/repos/decentraland/web3-ts","labels_url":"https://api.github.com/repos/decentraland/web3-ts/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/decentraland/web3-ts/issues/2/comments","events_url":"https://api.github.com/repos/decentraland/web3-ts/issues/2/events","html_url":"https://github.com/decentraland/web3-ts/issues/2","id":327744702,"node_id":"MDU6SXNzdWUzMjc3NDQ3MDI=","number":2,"title":"Review (wip)","user":{"login":"mcortesi","id":656276,"node_id":"MDQ6VXNlcjY1NjI3Ng==","avatar_url":"https://avatars1.githubusercontent.com/u/656276?v=4","gravatar_id":"","url":"https://api.github.com/users/mcortesi","html_url":"https://github.com/mcortesi","followers_url":"https://api.github.com/users/mcortesi/followers","following_url":"https://api.github.com/users/mcortesi/following{/other_user}","gists_url":"https://api.github.com/users/mcortesi/gists{/gist_id}","starred_url":"https://api.github.com/users/mcortesi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mcortesi/subscriptions","organizations_url":"https://api.github.com/users/mcortesi/orgs","repos_url":"https://api.github.com/users/mcortesi/repos","events_url":"https://api.github.com/users/mcortesi/events{/privacy}","received_events_url":"https://api.github.com/users/mcortesi/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-05-30T14:00:28Z","updated_at":"2018-05-31T13:58:07Z","closed_at":null,"author_association":"NONE","body":"Agustin,\r\n\r\nAs promised, I began reading the code, so I can give you a proper review.\r\n\r\nFirst, let me say, awesome work! \r\nI haven't yet used decorators and I like how you used them... well in a sense.\r\n\r\nNow to the review...\r\n\r\n1) I think that even though the `@inject` and the `Method` is a nice dynamic way of adding behaviour, you are loosing type safety everywhere.\r\n\r\nYou could do something like:\r\n\r\n```ts\r\ntype Input<A> = (val: A) => any\r\ntype OutputFormatter<X> = (val: any) => X\r\n\r\nfunction newMethod<A1, X>(opts: {\r\n  name: string\r\n  inputs: [Input<A1>]\r\n  output: OutputFormatter<X>\r\n}): (rm: RequestManager) => (a1: A1) => Promise<X>\r\nfunction newMethod<A1, A2, X>(opts: {\r\n  name: string\r\n  inputs: [Input<A1>, Input<A2>]\r\n  output: OutputFormatter<X>\r\n}): (rm: RequestManager) => (a1: A1, a2: A2) => Promise<X>\r\nfunction newMethod<A1, A2, A3, X>(opts: {\r\n  name: string\r\n  inputs: [Input<A1>, Input<A2>, Input<A3>]\r\n  output: OutputFormatter<X>\r\n}): (rm: RequestManager) => (a1: A1, a2: A2, a3: A3) => Promise<X>\r\nfunction newMethod<A1, A2, A3, A4, X>(opts: {\r\n  name: string\r\n  inputs: [Input<A1>, Input<A2>, Input<A3>, Input<A4>]\r\n  output: OutputFormatter<X>\r\n}): (rm: RequestManager) => (a1: A1, a2: A2, a3: A3, a4: A4) => Promise<X>\r\nfunction newMethod(opts: {\r\n  name: string\r\n  inputs: Input<any>[]\r\n  output: OutputFormatter<any>\r\n}): (rm: RequestManager) => (...args: any[]) => Promise<any> {\r\n  return (rm: RequestManager) => async (...args) => {\r\n    const payload = {\r\n      method: opts.name,\r\n      params: opts.inputs.map((format, i) => format(args[i]))\r\n    }\r\n    const result = await rm.sendAsync(payload)\r\n    return opts.output(result)\r\n  }\r\n}\r\n\r\nexport const eth_getBalance = newMethod({\r\n  name: 'eth_getBalance',\r\n  inputs: [formatters.inputAddressFormatter, formatters.inputDefaultBlockNumberFormatter],\r\n  output: formatters.outputBigNumberFormatter\r\n})\r\n```\r\n\r\nThis way eth_getBalance returns a typed function at it will fail at compile time.\r\nI've just done this, but i also saw that `inputAddressFormatter` isn't typed either; but it could.\r\n\r\nThere a few places where parameters or return values are `any`, maybe because everything is a work in progress.\r\n\r\nFor now this is my 2 cents, happy to help if you want *code labor*.\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/decentraland/web3-ts/issues/comments/393539008","html_url":"https://github.com/decentraland/web3-ts/issues/2#issuecomment-393539008","issue_url":"https://api.github.com/repos/decentraland/web3-ts/issues/2","id":393539008,"node_id":"MDEyOklzc3VlQ29tbWVudDM5MzUzOTAwOA==","user":{"login":"menduz","id":260114,"node_id":"MDQ6VXNlcjI2MDExNA==","avatar_url":"https://avatars1.githubusercontent.com/u/260114?v=4","gravatar_id":"","url":"https://api.github.com/users/menduz","html_url":"https://github.com/menduz","followers_url":"https://api.github.com/users/menduz/followers","following_url":"https://api.github.com/users/menduz/following{/other_user}","gists_url":"https://api.github.com/users/menduz/gists{/gist_id}","starred_url":"https://api.github.com/users/menduz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/menduz/subscriptions","organizations_url":"https://api.github.com/users/menduz/orgs","repos_url":"https://api.github.com/users/menduz/repos","events_url":"https://api.github.com/users/menduz/events{/privacy}","received_events_url":"https://api.github.com/users/menduz/received_events","type":"User","site_admin":false},"created_at":"2018-05-31T13:58:07Z","updated_at":"2018-05-31T13:58:07Z","author_association":"MEMBER","body":"Telegram? my handle is @menduz"}}
{"actor":{"display_login":"menduz"}}