effect-js icon indicating copy to clipboard operation
effect-js copied to clipboard

SDK v2 refactor

Open Jeffrieh opened this issue 1 year ago • 2 comments

  • implements simple caching system for IPFS (#99)
  • decouples & refactors the config
  • decouples tightly coupled client dependencies (services)
  • refactors the architecture into stateless functions pattern, this is to enable tree-shaking and promote a more functional and composable approach.
  • implements zustand store for store management inside the client.
  • remove non null asserts on vaccount & session
  • removes webpack, instead compile with pure tsc. (bundlers should be deferred to clients)
  • removes all tests as they were broken (lets focus on testing when v2 is stable)
  • adds ESLINT for linting & prettier as formatter, using the default prettier config.
  • prunes a lot of unused files & unused deps

Jeffrieh avatar Apr 02 '24 02:04 Jeffrieh

Thanks @Jeffrieh. I really like the stateless refactor, formatting style, and new build system, excellent work. I also understand your point to remove the tests, we can build a new test system once we finalize on the new architecture.

There's a couple of questions I want to look into, mainly around the external dependencies added:

1) zustand store I'm not familiar with this library - but it appears to be quite opinionated on state management and geared towards React. As a generic SDK, could we work without this dependency and move state management down to the applications?

2) IPFS cache system This is a point where we do require some state management in the SDK. I have a question about the idb-keyval dependency used, but will take the discussion to https://github.com/effectai/effect-js/issues/99

These 2 points are minor and should not hold up this refactor much. I will do some more testing, also to check if the SDK still works in React Native and NodeJS.

jeisses avatar Apr 03 '24 10:04 jeisses

Thanks @Jeffrieh. I really like the stateless refactor, formatting style, and new build system, excellent work. I also understand your point to remove the tests, we can build a new test system once we finalize on the new architecture.

There's a couple of questions I want to look into, mainly around the external dependencies added:

1) zustand store I'm not familiar with this library - but it appears to be quite opinionated on state management and geared towards React. As a generic SDK, could we work without this dependency and move state management down to the applications?

2) IPFS cache system This is a point where we do require some state management in the SDK. I have a question about the idb-keyval dependency used, but will take the discussion to #99

These 2 points are minor and should not hold up this refactor much. I will do some more testing, also to check if the SDK still works in React Native and NodeJS.

Hi @jeisses

  1. Yes this one is a bit confusing, Zustand is an unopinionated bearbones state-management solution that is very populair in the react ecosystem, it can however also be used without react We import the vanilla version of zustand which only has about a 1kb footprint. It provides us a very easy API and scalable way to manage and subscribe to state changes, for example the watchSession action - which triggers a callback whenever the session changes - makes use of this. I recommend keeping it in as it keeps us flexible, but we could also write some kind of observable/reactive logic ourselves.

  2. Nice catch, i assumed it was compatible with node aswell but indexedDB is indeed a browser only API. will do some more research to find a replacement for node :)

Jeffrieh avatar Apr 03 '24 12:04 Jeffrieh

I've tested and reviewed this again, excellent work @Jeffrieh! Will merge this now

jeisses avatar Apr 09 '24 19:04 jeisses