hippware / rn-chat

MIT License
5 stars 0 forks source link

[dev] re-integrate wocky-client into codebase #4888

Closed southerneer closed 4 years ago

southerneer commented 4 years ago

The technical piece of this is relatively straightforward (albeit tedious). The discussion is potentially more complicated.

I never really understood the need to separate wocky-client from the rest of the codebase. At the time that decision was made I was not knowledgeable enough on the server-client code to justify arguing against it. The only reasoning I can recall was that it would allow us to more easily attach a web version of the app later if we so desired. If that is indeed the only compelling reason then we should look to something like react-native-web to approach that problem later (if at all) and remove the complexity now.

There are a few reasons in favor of integrating the codebase:

  1. Simplicity
  2. As I mentioned here, we're already breaking the module/client abstraction by referring to LocationStore in wocky-client code.
  3. DRY up the codebase. We used to have 2 versions of logger.ts. This is because of the complexity involved in sharing a module between wocky-client and UI code. Making that transition involved: move the code to wocky-client, export it, re-export it at the package level, then import it on the client. It's a lot of moving parts.
  4. It's generally good to avoid solving problems you don't have.

cc @bengtan @aksonov

aksonov commented 4 years ago

Generally I agree.

bengtan commented 4 years ago

We talked about this in a meeting last night.

Does this ticket still need to be open?

aksonov commented 4 years ago

I will do conversion here

southerneer commented 4 years ago

Not QA-able