xf00f / web3x

Ethereum TypeScript Client Library - for perfect types and tiny builds.
211 stars 27 forks source link

Make typescript a runtime dependency #36

Closed webmaster128 closed 5 years ago

webmaster128 commented 5 years ago

web3x-codegen requires typescript at runtime, but it is in devDependencies. So after installing web3x, web3x-codegen does not work out of the box.

webmaster128 commented 5 years ago

hmm.... looks like web3x-codegen is more like a development tool, in contrast to the web3x library. Maybe it is worth splitting them into two packages

ethanfrey commented 5 years ago

See issue #34 for previous discussion.

Maybe codegen, evmprovider, etc... could be considered "extra" tools outside the core library. But I agree keeping it all in one repo is far nicer than the dispersion of the web3 world.

xf00f commented 5 years ago

@webmaster128 @ethanfrey

Given https://github.com/xf00f/web3x/issues/38 and https://github.com/xf00f/web3x/issues/34 discussion it does provide a stronger case for lerna.

For this problem there are 3 solutions:

  1. Leave it as is, and assume that if you're in need of web3x-codegen that you're obviously going to have typescript as a dependency in whatever is using it.
  2. Include typescript as a prod dependency, not great as someone installing a node app/lib that depends on web3x now gets typescript hauled in as well when perhaps they don't need it.
  3. Give it its own package, probably using lerna. This does mean that developers will now need to install both web3x and web3x-codegen. Given there is no need for separate builds of web3x-codegen (only its commonjs build is deployed in both web3x and web3x-es) this isn't too bad. Arguably though if you have to install web3x-codegen separately, it also doesn't just work out the box either. You have to install two packages either way!

The inclusion of EvmProvider has now also produced https://github.com/xf00f/web3x/issues/38. This is solvable as suggested by using a bigint lib. I'd probably favour jsbi as it's a semantic match. That would mean web3x could continue to exist as single package (with two builds web3x and web3x-es) and still be used on Node 8 (I didn't consider the inclusion of bigint-buffer having that impact).

I'm on the fence as to go either the Lerna route, or go with option 1 above, and the jsbi route, removing bigint-buffer (which is something I wanted to do anyway throughout the whole library, getting rid of bn.js but will have impact on all users of lib).

With option 1 plus jsbi things are basically fine.

But with Lerna, (and assuming just a single commonjs build of EvmProvider as it's meant to be a dev tool as well), we could strip the following dependencies from web3x. bigint-buffer, got, level-js, levelup, merkle-patricia-tree, rustbn.js.

EvmProvider could then become web3x-evm and only support Node 10+. This is probably less effort in terms of code as I don't need to change anything.

I don't see any great expansion of web3x dependencies over what currently exists however (and, licensing permitting, there is some inlining that could be done), so is going the lerna route worth it? I'm unsure.

I suppose I would lean slightly towards option 1 (web3x-codegen just fails out the box without typescript added to users deps), port all BigInt, BN.js code to jsbi, removal of bigint-buffer in favour of custom code that does the same with jsbi, and then perhaps inlining some libs that could be useful modules in and of themselves in web3x such as merkle-patricia-tree.

Sorry for all the detail. I'm thinking "out loud".

webmaster128 commented 5 years ago

Hey @xf00f, thanks for all the insights and giving the community a chance to add their input.

I never used jsbi, but the idea looks great.

Regarding splitting the repo I'd follow the the zen of Python: complex is better than complicated. I like the idea of having web3x, web3x-codegen and web3x-evm in three packages that can be maintained and updated from a single repo. The ability to reduce dependencies is great.

xf00f commented 5 years ago

@webmaster128 @ethanfrey I have started restructuring work here. https://github.com/xf00f/web3x/tree/xf/v4.0.0

I opted not to use lerna but to simply manage the packages manually within the monorepo. As this is going to be v4.0.0 I may well try to port everything to use jsbi as well to group breaking changes, but it means it will take a couple of weeks before it gets merged.

I have started converting parts to MIT also.

ethanfrey commented 5 years ago

Sounds good.

One point that may bite you, and why I use lerna, is to depend on current version of the other code in your same repo/branch.

For example, I see https://github.com/xf00f/web3x/blob/xf/v4.0.0/web3x-codegen/package.json#L40 which links web3x-codegen^4.0.0 to a webx^3.0.6 dependency. Imagine you add a feature you want to webx in v4, you can't test it... You need to make sure to link the repos locally for dev work.

That is one thing lerna handles nice, alongside managing releases. But if you can get those two points working locally without lerna, then I agree no need for added complexity.

xf00f commented 5 years ago

I did try lerna. The issue immediately arose that it incorrectly linked to the top level of the web3x package, when really it's meant to link to web3x/dest, or web3x/dest-es. The fact there are two builds complicates things also. I quickly found myself trying to work around the limitation, and decided that wasn't the way to go.

It's possible I just don't know some of the deeper "tweaks" to lerna. But the way I've usually handled linking libs is to do the build, go to web3x/dest, type yarn link, and then go to whatever depends on it and do yarn link web3x. That process continues to work in this new structure and seems pretty simple.

ethanfrey commented 5 years ago

Sounds good. I would just add some docs on how you set up for development in the README (or other doc), to allow other devs to easily contribute.

I agree lerna can be confusing/complex, but it is documented. I think your simpler solution just needs some docs.

xf00f commented 5 years ago

web3x-codegen is now a separate package that can be added as a dev dependency.

ethanfrey commented 5 years ago

Awesome update. I will take another look now