Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Typescript migration #71

Closed
alcuadrado opened this issue Jul 23, 2019 · 5 comments · Fixed by #72
Closed

Typescript migration #71

alcuadrado opened this issue Jul 23, 2019 · 5 comments · Fixed by #72

Comments

@alcuadrado
Copy link
Member

Not having types in this library was by far the major source of bugs I had when working with the VM. I think we should consider migrating it.

@holgerd77
Copy link
Member

Yes, this is one of the last ones missing. @s1na not sure, did you once say that you already did some work on that?

@s1na
Copy link
Contributor

s1na commented Jul 23, 2019

Agreed. Well, when we first started "The Great Migration" (:smile:) I spent an hour or so on migrating this library, but stopped soon because it depended on other libraries and they didn't have typedefs yet. So no, I don't have anything useful, we'll have to start from scratch.

@alcuadrado
Copy link
Member Author

I'm working on this migration. It's a little more complicated than I expected, but not terrible.

One thing that I'm wondering is if we should migrate and keep these files:

  1. https://github.com/ethereumjs/ethereumjs-block/blob/2c3908e5bbb6c9c94aeb5547a779295433f42545/from-rpc.js
  2. https://github.com/ethereumjs/ethereumjs-block/blob/2c3908e5bbb6c9c94aeb5547a779295433f42545/header-from-rpc.js

They seem super out of place to me. None of the other main libraries has anything to do with the RPC format.

@s1na
Copy link
Contributor

s1na commented Jul 26, 2019

My guess is that those functions are in use by MetaMask. Let's migrate them along but create an issue to discuss their deprecation.

@alcuadrado
Copy link
Member Author

alcuadrado commented Jul 26, 2019

Makes sense, I'll do that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants