Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client Modularization Part I: Client as a Library #3460

Open
4 tasks
holgerd77 opened this issue Jun 18, 2024 · 2 comments
Open
4 tasks

Client Modularization Part I: Client as a Library #3460

holgerd77 opened this issue Jun 18, 2024 · 2 comments

Comments

@holgerd77
Copy link
Member

This is a meta issue for a first round of work preparing for a more modular client world as discussed in the roadmap call, with the goal of having one or more client modules with a minimal dependency set which can then be instantiated in different security and application contexts (browser, verkle, eventually Portal,...).

First step here is to not think about modularization/refactoring yet but get to a cleaner separation between the core package as a library and a lightweight "packaging" or instantiation script. We have this separation already with the bin folder (instantiation of things like DB, logging, ...) and the src folder containing the "library" and at some later stage these things might go into separate packages, @ethereumjs/client-lib and @ethereumjs/client or @ethereumjs/client-el or however we reason about things, no need to decide yet and rather for a later stage.

So this issue is basically to get from 70% to 95% on a clean bin and src separation, following TODOs (not necessarily exhaustive):

  •  Adds things which feel "extra" and draw in extra dependencies as an option (e.g. Prometheus) (so that the dependency can later on be easily removed on a bin and src package separation
  • Decide what to do with the logger in a library context (how are other clients actually handling this? 🤔 I am pretty sure I stumbled upon examples here that other clients can run in "library mode"?), so what should happen with the logging output, is the current output even a good format or does this needs adoption on frequency, format and so on?
  • The former also leads to the case that the logger urgently needs to be replaceable generally, Winston draws in an awful lot of dependencies and it should minimally be possible to replace by a simple no-dependency logger (have a look at package.json linked, basically with Winston in we cannot responsibly provide anything other than a research client)
  •  There should be a closer look at the code in bin - this has substantially grown over the years - with the eye upon the question if some code is a) initialization code (then it can stay) or b) core logic code (then it should move over to src)

Guess there might be other things, feel free to add below. 🙂

@holgerd77
Copy link
Member Author

Also to add here, regarding the "looking at the bin code" TODO: this can actually also encompass to structure the remaining code better, so e.g. to split out some separate files in bin where/if it makes sense to make everything more readable and overall better structured.

@holgerd77
Copy link
Member Author

Update: I looked a bit into the Prometheus example. So this is actually already solidly separated to some greater extend, but not fully though (just search for Prometheusin the client package).

This opens the overall question again, how to cleanly keep typing things (in src) if the dependency is not there (or will be removed in the future). Atm we still have got things like import type * as promClient from 'prom-client' in the src folder (which should go away).

I think there must be solutions here, either using some more advanced TypeScript-specific features or also applying some more general design patterns on how to go further on abstraction.

Maybe something around the lines also mentioned by @roninjin10 here in #3446:

Class methods cannot treeshake

Problem: All class methods cannot be tree shaken
Solution: Consider moving class methods to a util function or seperate class. Especially if it's niche or unlikely to be used
Recommendation: I personally don't see problematic methods so I would skip this one

// this is what a maximally tree shakeable library like Viem or Tevm would do
// instead of class method seperate data and methods and pass data in as first argument
export const consensusAlgorithm = (common: Common): string | ConsensusAlgorithm {}

So to add this on instantiation in the form of a wrapped method or something, where the data is passed in? 🤔

Would be good if we get to some solid answers/solutions here, since we will need this pattern also for other things, also for things around tree shaking.

Maybe @gabrocheleau is a good candidate to take on this, Gabriel, since I have the impression you are relatively good with both these TypeScript specifics as well as these kind of structural questions?

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

No branches or pull requests

1 participant