Client: Reduce Simple Dependencies
Our client is up for some more serious production use cases with the rise of a stateless Ethereum world on the horizon and the need for lighter (browser) client (modules) coming along!
This is a warm-up task regarding bringing client dependencies to a bearable level, which is a necessary pre-requisite if we want the client being deployed and used in mission-critical environments!
There are likely various dependencies which can be easily removed and either replaced by some own code or "internalized" as we did in the past for other small libraries where the license permits it (e.g. MIT), e.g. crc32 in Common, see crt.ts.
On a rough first look these dependencies might be candidates (but definitely needs a second look/judgement, very quickly writing this together):
- bodyparser
- connect
- cors
- it-pipe
- jwt-simple
- qheap
(note that - for sure - yargs and winston will be candidates too to be handled, but these are more challening (might be solutions like the ability to replace the logger or something), and should not be part of this "warm-up session" 🙂 )
(another note: some dependencies (like level will likely/eventually also "fall away" coming with some client refactoring/modularization (so where e.g. a stateless client might not need a full level DB. But also something for later and more strucutred thoughts preparing for this 🙂 )
Ah, also to note: please rather to be tackled "one by one" (so: one dependency, one PR, rare exceptions on this)
Hey, @holgerd77, I was taking a quick look at this. Most of the time, it depends on external packages as well. How do you want to proceed: start internalizing those and add new external or everything in one PR? But I suppose some have a lot of nested and sub-nested packages.
Example :
-
connectuse 4 other externals -
body-parseruse 10 externals
Hey @foufrix, cool that you were having a look! 😄 Just to clarify: this was just a very quick write-up, I didn't have a closer look if removing the specifically cited dependencies is really an option respectively how we use these in the client code.
See the dependencies references rather as some "examples", this would still need a closer look one-by-one before doing anything (and it might very well turn out that the first look was too superficial and removing a certain dependency is not realistic).
Also note that this one is also really no short-term issue but a rather somewhat hard and strategic one, which will strain over months. We are generally cautious with adding dependencies, and most dependencies selected/used here will be here for some solid reason. So ease of removal will largely differ.
So generally we should go from simple to hard here.
Might be that you already picked some of the harder ones. Let me have a look.
Yeah, so connect seems already be a (too) hard one for now. I guess we should leave this aside for now.
This is deeply used within the code, not the "super core" part though but the RPC. This current issue comes along with an eventual option to further modularize the client, so that people can use selected parts (and with this skip certain dependencies). Also as some context. For RPC it might also be that we can offer this separately, and so also somewhat mitigate the need for people to draw too much dependencies in. Bigger topic though and something for some separate thought process.
For body-parser I have a lot more the impression that this is a viable direct candidate for replacement, "internalization" (copy the use code parts if possible by license an add a license header) or removal. This is only use in one very single place (and only for JSON parsing??) in src/util/rpc.ts and has an insanely large dependency list.
Hey @holgerd77, thanks for the quick reply,
Seems like cors is a good first candidate. It depends on:
- object-assign, which is only needed before node 4, and from what I see, we use node
>18so I can refactor and use directlyObject.assign() - vary which has no dependencies.
Should I add vary.ts and cors.ts in packages/client/src/util?
Cors is only used in rpc.ts, so it ticks all the checkboxes for a first move.
Can you validate what I said, then I'll start the integration 🙏
@foufrix Hi there, cool, yeah I would fully agree 🤩, great selection! Code is also small enough to not overload on our side, and it's also good to have this very old object-assign dependency out!
One "change request" is the comment I already gave here https://github.com/ethereumjs/ethereumjs-monorepo/pull/3451#issuecomment-2164958515 to Amir for the qheap internalization, so it would be good if you can place in a dedicated ext folder (guess it's ok to create in parallel, we'll for sure get this merged afterwards).
So I would think you can start here! 👍 If you want to be on the super-safe side you can wait for an additional comment from @acolytec3, he normally answers pretty quickly (will likely "come in" latest 2-4 hours after this comment) and I guess he has more insight in the code parts and sometimes he comes with some comments like "Oh, this (the cors) package is not so useful anyhow and I already wanted to remove" 😂 or something like that!
Guess chances here are only at 20% at most that this happens. So this should not hold you back if you would otherwise loose time you would want to spend on this!
Did someone call? :ninja:
So I have a thought about the whole set of deps we use in the rpc for middleware handling. Could we replace the whole set (connect, body-parser, cors) with something like ittyRouter, which has zero additional deps, is maintained (and modern and is written in Typescript), and looks like it has cors/json parsing middleware built in? I know that's a bigger project than just copy/paste cors to an internal folder but feels like it would be a bigger win in terms of shrinking our dependency footprint.
I'm not a master of API routing and how that would integrate with the jayson RPC module we use but it seems to be similar.
@g11tech any thoughts here?
Did someone call? 🥷
So I have a thought about the whole set of deps we use in the rpc for middleware handling. Could we replace the whole set (
connect,body-parser,cors) with something likeittyRouter, which has zero additional deps, is maintained (and modern and is written in Typescript), and looks like it has cors/json parsing middleware built in? I know that's a bigger project than just copy/pastecorsto an internal folder but feels like it would be a bigger win in terms of shrinking our dependency footprint.I'm not a master of API routing and how that would integrate with the
jaysonRPC module we use but it seems to be similar.@g11tech any thoughts here?
Yes
I can take a look at ittyRouter implementation, effectively it solve 3 packages in one go. and maybe start with only the file rpc.ts where I can change connect, body-parser, and cors. As connect is used heavily everywhere else we can do it in two time, first in rpc.ts then the rest?
Until confirmation, I've started to move jwt-simple that has no sub depencies and is only used rpc.ts
I think that's fine. I don't know for sure that ittyRouter is the solution here. Just did some googling for modern dependency free middleware providers and it came up and looks like it has the features we need.
I'll be honest that I'd much rather have one maintained, modern dependency (with no deps of its own) that takes care of all this super low level HTTP network layer stuff so we don't have to maintain it). Having to maintain two middleware modules to parse HTTP response bodies feels like work that we don't want to have to do (should the specs for them ever change).
To keep you updated, I've started to investigate how to add itty-router #3467, though I will not be able to continue before 9 July. If anyone wants to take a look before then, feel free! I will continue when I'm back.
Cool! 😃