[PROPOSAL] Refactoring of token input screen in Send flow

[vkjr]

fixes #21468

Summary

An attempt to simplify the token input screen by moving all the internal calculations into subscriptions.
This PR is a place to discuss suggested approach, its pros and cons.

Review notes

• all calculations that were inside the view are now the subscriptions
• screen-specific subscriptions moved to a separate file
• state moved from view to rf-db for the sake of using in subscriptions
• controlled-input logic changed. Before we had 4 data pieces that were treated as token input - value of token input itself, upper limit, lower limit, error state. Now I tend to think that we need to store only token input value because error state should be a subscription and upper limit also. So new file for controlled input logic added. It is significantly overlap with already existing one, but I didn't want to refactor more in this PR and touch other views.

Testing notes

Platforms

• Android
• iOS

Areas that maybe impacted

Functional

• wallet / transactions

status: ready

[status-im-auto]

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	b7eaa85	#1	2024-10-28 11:34:54	~3 min	tests	📄log
✔️	b7eaa85	#1	2024-10-28 11:40:29	~9 min	android-e2e	🤖apk 📲
✔️	b7eaa85	#1	2024-10-28 11:40:54	~9 min	ios	📱ipa 📲
✔️	b7eaa85	#1	2024-10-28 11:41:11	~9 min	android	🤖apk 📲

[vkjr]

enabled-from-chain-ids and from-enabled-networks don't need to be an argument, they turned into subscriptions :send-input-amount-screen/enabled-from-chain-ids and :send-input-amount-screen/from-enabled-networks respectively.

[vkjr]

We never used this subscriptions with false argument, so I removed it and renamed

[clauxx]

just a reminder that this sub will change after this PR

[shivekkhurana]

This seems to be a big change so I have added the 2.32 label. Let's not merge this until 2.31 is out.

[vkjr]

This file duplicates a logic from controlled-input/utils.cljs because I didn't want to include that into refactoring.
Here all the functions operate only on input-value, not on state structure as in controlled-input/utils.cljs

[vkjr]

122 lines -> 9 lines

[vkjr]

Since we use upper limit as a subscription, it doesn't need to be updated

[alwx]

I do like the style and the general idea, would love to see more discussions around that and about weather we want to change other components to look like that or not. I would say it makes the proper separation between the "view" and the "controller" and helps writing cleaner code in UI components.

[clauxx]

Looks great 💯

A bit on the fence regarding "screen subscriptions" though. Could we use these subs more as "flow subscriptions", as we already have some for the send flow e.g. :wallet/send-amount, but by narrowing the ns -> :wallet.send/amount? My concern is that some of them might get reused (now or later) in other screens in the same flow (e.g. confirmation/approval/etc.), so tying them down to a component/screen might be too specific.

[vkjr]

@clauxx, I didnt touch other screens but pretty sure that some of this subscriptions can be elevated to a flow level. Totally agree with you on this!

[ilmotta]

Hey @vkjr, thanks for asking for my review and waiting as well.

I shared a few of my main concerns with the proposed solution, but I didn't add comments to a few other minor details.

I understand this PR is inheriting tech debt from the original code, but ignoring that, a few other points:

• There's way too much code without coverage. I think if we want to build something sustainable and with fewer bugs in the product we should all reject PRs that have too low test coverage (or nothing). QAs can't test the exponential number of test cases only we devs can simulate with unit tests.

• The subs and events are very tied to the UI. I see this as a significant violation of the re-frame architecture. I see you used the wallet/screens/ directory, but still doesn't seem right to me. If this type of solution is considered acceptable, where do we draw the line to separate UI and "business" responsibilities?

I do see the benefit of cleaning up the original view, but I think something similar could be achieved without coupling as much or at all the event and subscription layers.

Maybe I'm too attached to old ways or fixed ways of using re-frame, but my intuition tells me the app-db should be treated like a database and subs like derived computations without any UI concern if possible (sometimes we need to make exceptions and that's fine). We start to mix those things and many problems appear, some of which I see in the proposal.

But overall, I have to say you invested a lot into this refactor, so I don't want to reject it, but it's not a direction I think we should go, but again, I might be wrong and everything will be fine.

[ilmotta]

money/bignumber seems to always return a valid instance or nil, so I think we don't need the extra check with money/bignumber?. The code below also removes the duplicated decoding of input-value. Same idea can be applied to lower-limit-exceeded?.

(defn upper-limit-exceeded?
  [input-value upper-limit]
  (and upper-limit
       (when-let [input-bignum (money/bignumber input-value)]
         (money/greater-than input-bignum (money/bignumber upper-limit)))))

[ilmotta]

subs throws when the argument is nil, as most string functions do.

[ilmotta]

parse-double is dangerous to use for us because it throws if the input is not a string. Nil will throw as well. The functions relying on parse-double can now all throw with nil values (e.g. value-numberic).

Behind the scenes parse-double uses js/parseFloat, which we also use in utils.number/parse-float. Our utility function is more useful for us because it's guaranteed to not throw and never return NaN, which parse-double can still return. Our utility also allows a default value, which is zero by default.

If you see any other places using parse-double, please consider changing that.

[ilmotta]

The -logic suffix in the namespace seems unnecessary. Maybe controlled-input-utils?

[ilmotta]

This level of nesting I find wild and really hard to optimize for re-frame. I know this has been discussed a few times, but it's an unfortunate pattern that started some time ago when the Wallet team was first created and we forgot to align between teams, and now we have two models to structure the app-db.

I remember an experienced dev tried to optimize some parts of the wallet subs a few months ago and just gave up. It's really easy in this style to mutate parts of the app-db and cause re-computation in many unwanted subs because the state is modeled as a tree. In this deeply nested style, anything we update in the app-db that's not a leaf can be quite inefficient for re-frame.

I'm obviously not suggestion a change in the way the app-db is modeled now, but this is a problem and I think with well named root keys it's just as easy to understand the app-db and more flexible to combine subs and optimize them. For example, the root key could be :wallet.screens.input-amount-screen/input-value, then, writing to it would cause minimal overhead to re-frame.

It's also a bit of a smell that we are naming state with UI concerns, like input-amount-screen. Treating the app-db like a database of sorts seems better, at least in my experience.

Would be good for us all to understand these problems and be on the same page. Could be a good point for a conversation in our DX calls. Tagging a few folks who also thought about this @clauxx @ulisesmac @smohamedjavid @seanstrom

[ulisesmac]

@ilmotta A good point to discuss in our DX for sure.

I don't agree on that this pattern is inefficient, if subs are properly structured, I think this is the way re-frame is optimized to work: through a graph (tree) of signals (subs) and mounting only the subs being used by views.

Declaring everything as root values and compose keywords for every value stored sounds, to me, as a wrong pattern for re-frame 🤔

In terms of performance, if we store everything as root subs, each change in app-db will trigger a comparison on every stored value. This giant amount of comparisons won't happen if we nest values and write subs for the subnodes. So I don't see why it'd be worse to nest data. - We already discussed this in Montenegro and I remember an experienced dev said they thought re-frame compared by checking references, but it's not the case.

It's also a bit of a smell that we are naming state with UI concerns, like input-amount-screen. Treating the app-db like a database of sorts seems better, at least in my experience.

I've seen both approaches in production, I personally prefer not lift up component's state to re-frame, but it's not that bad, as long as it's properly implemented (e.g. by using dispatch-sync when handling inputs).

I agree with you on this point, and, to me, status doesn't use this pattern of handling the UI state in re-frame. I think good results can be achieved by using reagent and it's tools (like cursors).

[ilmotta]

Declaring everything as root values and compose keywords for every value stored sounds, to me, as a wrong pattern for re-frame 🤔

@ulisesmac, not everything. I wouldn't suggest always using root keys, of course there's a balance and only simple extractors should be closer to the root.

I think until a use case is built the discussion won't advance much. If I could find some time I'd love to dive deeper to make the discussion more practical, but unfortunately I don't have much else to say.

For now, my intuition and a some experience tells me that modeling re-frame state is not much different than modeling a database and that a database that's not reasonably well normalized is quite hard to optimize/change. Normalization is generally good for many reasons. There's a reason people suffered with the likes of MongoDB or Firestore when nesting collections and couldn't easily write efficient queries, to give an example.

[ulisesmac]

There's a reason people suffered with the likes of MongoDB or Firestore when nesting collections and couldn't easily write efficient queries, to give an example.

I completely agree since I've used both.

I think until a use case is built the discussion won't advance much.

Yeah, having real examples of storing and querying data with both approaches compared would be great to properly compare what's the best for us.

[ilmotta]

Same as another comment, rf/sub can't be used inside sub handlers. I think there are other instances of this problem.

[ilmotta]

As a general good practice, I always thought the subs should return data for the view layer, but not formatted data because that's a UI concern. One reason is that subs that return formatted data are usually not reusable. A formatted number can't be easily reused to compute something.

[ulisesmac]

Not sure about this one @ilmotta

Some node subs can't be reusable and that's fine, there can be some subs that are leafs that are screen-specific. The previous subs node (before formatting) can be made to be reused.

Otherwise, we would need to handle formatting logic in the view and it's not needed. Subs are cached and we aren't using reagent form-2 components, if we wanted to avoid the re-computation in the view we'd need to use a hook, which leads to something equal or more complex.

[ilmotta]

@ulisesmac There's nothing inherently wrong with using subs to return formatted data, but in my experience it does come with some problems because the subs IMO should be more independent, let's say, more stable than views.

Translations, number formatting, date formatting, string concatenations for presentation, etc those things I personally prefer to leave to the view layer. I've seen subs returning props for view components (even anonymous functions), just one example where bringing responsibilities to the sub layer can lead to sub-optimal solutions. But that's why I said "As a general practice, I always thought" so it's more of a personal preference.

Though I have to say this practice of formatting only at the view layer is as old as software, or at least I remember from pre-2010 era where backend controllers would return only data to the view and the view would use its own utilities to format numbers, etc. This architecture made the controllers simpler, more stable, improved reusability, easier to test, similar idea I see with subs.

Performance of these final presentation transformations shouldn't be a problem, but sometimes they might be I agree. For example, translations are memoized. I (personally) don't like the approach of caching all transformations in the view layer as we started to do since we migrated to hooks everywhere. I'm gonna guess almost nobody is profiling (myself included). I've seen many PRs caching stuff that is literally instant in CLJS, like the result of a simple case macro. So you know, worrying about caching formatting logic seems unnecessary unless profiling proves otherwise.

[ulisesmac]

Thanks for your reply, I agree on your POV.

[ilmotta]

merge is one of the slowest functions in Clojure, so it's good to avoid it. Here it looks like you can use cond-> and assoc.

[ilmotta]

Bignums are not cheap and here it's re-created on every iteration. A common approach I've seen in other project is to declare the zero value as a static var inside money and declare it as a ^:const.

[ilmotta]

These coercions using clj->js, or money/bignumber inside the subs seem like a smell. Many functions in the PR have some sort of coercion, which makes the code fragile. Wouldn't it be possible to decode the values exactly once, and write to the state?

Maybe this is just the result of too much data mangling for the UI at the wrong sub layer?

[ulisesmac]

Actually, if we pay attention to this piece of code (inherited). It's a bit weird because we transform to JS and wrap it on an or 🤔 , also what is token-symbol expected to be that we want it on JS?

I think this is something that we wanted to be (name token-symbol) if it is a keyword.

@vkjr What do you think token-symbol is?

[vkjr]

@ulisesmac, I remember trying to figure purpose of this conversion and as far as I understood token-symbol is a keyword. And cljs->js used here instead of name because name throws an error when argument is nil. So my guess is that one day developer ran into a problem with thrown exception because token-symbol appeared to be nil and created this workaround.

[ilmotta]

My own lessons learned from the past are to decode/coerce data as early as possible if performance allows (it should usually because some code will decode eventually/soon). This way, the correct instances flow in the system, not primitive strings representing numbers. In terms of performance these type transformations in subs cause unnecessary work over and over, and there's a good chance other parts of the code base are doing the same transformations on the primitive types.

Just one idea: with malli we can describe transformers to decode any data https://github.com/metosin/malli?tab=readme-ov-file#value-transformation We used that in another CLJS project quite successfully.

The idea would be more or less like:

1. A response from an RPC request or signal arrives. The response contains primitive values representing dates and big numbers.
2. We need to store in the app-db as clj data.
3. We define a schema and call decode from the response.
4. We persist the result in the app-db, making sure all data types implemented the necessary interfaces, like what we did with BigNumber in money.cljs. This will allow re-frame to do equality checks correctly without any extra work from devs.

The only thing worth measuring is the performance aspect. I don't remember now if malli decoding is roughly as fast as hand-rolled code.

[ulisesmac]

I like the idea @ilmotta

This will also work as documentation we can always check.
We need to check the performance aspect, but sounds great.

[ulisesmac]

BTW, this clj->js transformation is not related to a late data tranformation

[vkjr]

@ilmotta, thanks for all the comments and opinions on this PR, they are very valuable.
There are bunch of different surfaces touched and I will prepare more answers to your particular concerns and will post them here.
I also want it to be clear that this PR is not intended to be commited "as is". Instead it is a convenient field for discussions because when we have a code at hands it is easier to spot the problems. So all critical comments are very welcome!

[flexsurfer]

could you please elaborate on why such subscription is needed?

[vkjr]

It is an umbrella for screen states that are kept in rf db. Currently used in :send-input-amount-screen/state.
I see it a convenient way of separation screen state that kept in rf-db from other data. You can see structure in wallet/db.cljs.

[flexsurfer]

i'm not sure about specify namespace for specific screen, subscription in most cases should be general and could be used in any part of the app

[briansztamfater]

I'm not sure either, it looks a bit odd to me. Maybe a combination of general subscriptions and utility functions used in the UI layer could make the code more consistent with the rest of the codebase and improve readability.

[vkjr]

subscription in most cases should be general and could be used in any part of the app

@flexsurfer, would you mind providing some reasoning for this statement? Because I tend to disagree here, the main purpose of re-frame subscriptions is to provide a way for a view to re-render when data changes. So when you have a data for specific screen, you can have a screen-specific subscription.

But of course I agree that in this file not all subscriptions should be tied to particular screen, some are related to the whole send flow.

[ilmotta]

I'm also weary of this approach because it's a departure from what we've been doing for years and which has worked well, that is, subscriptions provide some computation on the state, but they don't care about which screen they are being used.

Going on the direction of @briansztamfater's feedback, subscriptions for views are a new concept. These subs will change much more often because they will be more tied to screens, contrary to our current subs, which tend to be quite stable because they mostly need to change if the global state changes.

We could have a local file named subs.cljs in the same directory of the view.cljs file. Our agreement/convention would then be that the subscriptions in this localized subs file would not be reused by other views or other view subs, otherwise I believe things would get messy quickly. I'm not a fan of this approach, but I think it's better than mixing specific screens concerns with the general subs in status-im.subs.

My main concern is related to the complexity of the subs graph, which would explode in the future with lots of layer-3 subs depending on other layer-3 subs. The problem is that some subs will "pull the world" and small changes in subs will cause rippling effects in unwanted places. This has been a problem with large subs pulling from too many inputs and from other layer-3 subs, it's certainly going to happen with this approach, although nobody can predict by which extent. Maybe it will be manageable, but I'm still weary @vkjr

[vkjr]

My main concern is related to the complexity of the subs graph, which would explode in the future with lots of layer-3 subs depending on other layer-3 subs.

The problem is that some subs will "pull the world" and small changes in subs will cause rippling effects in unwanted places.

@ilmotta, all the screen-specific subs that I've created were extracted from huge let bindings inside the view. Before extracting they were local computations that still dependent on non-screen-specific subs and on each other. So the rippling effect of changes in high-level subs is still there. Also all these computations were made every time when any value of the subs changed. By moving them to subscriptions in different views we reduce the amount of recomputations but rippling effect stays intact - it was here, it is still here. But maybe I'm getting you incorrect and you mean something else by "rippling effect"? Wdyt?

I'm also weary of this approach because it's a departure from what we've been doing for years and which has worked well

I would like to opine here - this approach also led us to 160-line let statement with tons of local computations :)
For example we have a screen-specific state - value of input field. Also we have a screen-specific state - error flag if input value is higher than amount of tokens user has. Implementing this error flag with screen-specific subscription is trivial and moves logic out of the view. Implementing this without subscription require us to inflate view code with more logic.

But of course this depends on the approach we are comfortable with. I'd be happy to see the views more dumb and contain only basic logic related to displaying with all the other logic moved out)

[vkjr]

@ilmotta, thanks for the detailed explanation, now I see your point more clearly 🙏 And tend to think that we are trying to reach the same goal but having different paths in mind.

As we discussed on DevX call, our code lacks some clear layering and interfaces between layers that aren't crossed freely.

You talk about fixing the underlying problem by direct approaching the root. For example if we have a non-optimal data-model then we need to create a layer somewhere that prepares the optimal ones. And if we start doing this we should simultaneously change all the views to new data-model which should make them more simple. This approach might work but I suspect it to be more complex because it requires many changes at once, plus it is harder to see what optimal data model should be.

What I had in mind while but never articulated in comments to this PR (my bad) is approaching the problem from leafs and doing this incrementally because huge changes are hard :(

Since I never articulated this, there is no surprise that you can view this PR as fire-and-forget solution that doesn't really solve much. But it is not the case and I encourage you to read the imagined steps I written below to have an impression of how I see the evolution of this idea in next PRs 🙏
@briansztamfater @ulisesmac @flexsurfer @alwx @clauxx, @shivekkhurana, I would be glad if you also take a look.

Right now we have a lot of logic inside some view (input_amount particularly). How I see the path from here:

Step 1 (This PR). We can clean the views by squeezing the logic out of views to subscriptions and having screen-specific subscriptions.

Step 2 At this point we can see all the subscriptions that belong to screen, analyse them and groom. It is very convenient because we clearly see dependencies of subscriptions:

Example 1:
We see a subscription:

(rf/reg-sub :send-input-amount-screen/unsupported-token-in-receiver?
 :<- [:wallet/wallet-send-receiver-network-values]
 :<- [:wallet/wallet-send-tx-type]
 (fn [[receiver-network-values
       tx-type]]
   (and (not= tx-type :tx/bridge)
        (->> receiver-network-values
             (remove #(= (:type %) :add))
             (every? #(= (:type %) :not-available))))))

From the input of subscription we can tell that there is nothing screen-specific. So as a bare minimum this subscription should be moved to the upper layer and belong not to a screen but to a :wallet/wallet-send... cluster.
Second, the content of this subscription can be moved to a pure function, probably under send/utils.cljs with tests added. And here we can make a mental or physical note that probably that complicated way of checking if token is unsupported indicates us that the structure we received is too complex. But we will save it for later.

Example 2:

(rf/reg-sub :send-input-amount-screen/token-decimals
 :<- [:wallet/wallet-send-token]
 (fn [token]
   (-> token
       utils/token-usd-price
       utils/one-cent-value
       utils/calc-max-crypto-decimals)))

Same as before - doesn't rely on any screen state, so shouldn't be screen-specific.
Can be a function inside comon/utils.cljs.
But we also can spot one thing - despite the fact that we pass around token, we never actually use many of this content, we only digging for price. Making a mental note because we know that in our re-frame db we have a place with all the prices and we don't really need to duplicate them within token structure.

Example 3:

(rf/reg-sub :send-input-amount-screen/conversion-rate
 :<- [:wallet/wallet-send-token]
 :<- [:profile/currency]
 (fn [[token
       currency]]
   (-> token
       :market-values-per-currency
       currency
       :price)))

Again - shouldn't be a sub belonged to the screen. Can be extracted to some utils. And again we see that token is not used but instead we only look for a token price. Again making a mental note.

Step 3
Now we have cleaned view and cleaned subs. Now we can analyse all our utils files and and spot the problematic patterns.
At this point we already can think about more logical layers. Easy way to distinguish them is to ask yourself "would this function survive "as is" if we decide to rewrite our application to be just a command-line interface?". That will allow us to split the real business-logic and ui-related elements that we use only because of our current stack.

Example 1 in `wallet/common/utils.cljs:

(defn get-standard-crypto-format
  "For full details: https://github.com/status-im/status-mobile/issues/18225"
  [token token-units]
  (let [token-price-in-usd (token-usd-price token)
        {:keys [zero-value? usd-cent-value standardized-decimals-count]}
        (analyze-token-amount-for-price token-price-in-usd token-units)]
        ...
    )))

Let's ask ourselves, would that function be useful in command-line interface? In general - yes, because even in console ui we would need to print out crypto amounts. But we already spotted that we don't need to keep price within token, we have a separate place where we store all the price. So it is better to pass price as an argument here instead of extracting it from token.
Also if this function creates standard crypto format, probably it won't be used only in wallet, but also in other parts of application, so function can go somwhere in common/business-logic/utils.cljs

Example 2 in `wallet/common/utils.cljs:

(defn make-network-item
  "This function generates props for quo/category component item"
  [{:keys [network-name color on-change networks state label-props type blur?]}]
  (cond-> {:title        (string/capitalize (name network-name))
           :image        :icon-avatar
           :image-props  {:icon (resources/get-network network-name)
                          :size :size-20}
                          ; ... more code here
           }}

    label-props
    (assoc :label       :text
           :label-props label-props)))

Will this function useful in imaginable command-line interface? No! That means it doesn't belong to business logic and we can move it somewhere to `wallet/ui/utils.cljs' and in case of redesign simply drop out.

Step 4
By analyzing usage of data in different layers, for example in that related to ui we can have a better sense of what data shape would be best for our UI and business-logic. And create a data layer that transforms what we receive from status-go into what we need.

Conclusion:
@ilmotta, I very much like your metaphor of "sweeping dirt under the rug" because this is exactly what I do in this PR. I want to sweep all dirt under the rug so it is localised and as a next step we can start cleaning under the rug 😄

[ilmotta]

@vkjr, thanks a lot for your thoughtful comment explaning there's in fact a plan and the end goal is not to simply move logic out of views into subs.

Speaking of deeper problems, at some point in the history of the wallet source we started to solve problems in a certain way and never discussed as a larger group. For example, there's plenty of state in the app-db related to the UI in [:wallet :ui ...]. This coupling with the view, given enough time, is also responsible I believe for the complexities we see in some parts of the wallet. View concerns tend to spread like fire. My take on re-frame (which is obviously not universal) is that the re-frame layer should model data independently from the view layer. Like you said with the example of a CLI. This decoupling leads to superior code design in my experience. We now have events like :send-input-amount-screen/token-input-add-character and many subs violating this architectural principle.

Executing the plan you outlined will take a significant amount of time and we will need CCs to be well aligned in order to not just put more dirt under the rug. Might be a good idea to do some synchronous catch-ups with CCs every now and then to make sure we are on track and we all understand where we want to be. There's a considerable risk that we will stop mid-air and end-up with a mixed solution. I appreciate the less risky approach you want to take, this is key to keep mobile releasable 👍

My impression is that we are still not aligned on the problems that led the code to be what it is and that there's a high probability that we will make the same mistakes. The lack of involvement from more CCs in this PR is perhaps evidence that we should approach such complex discussions differently (e.g. RFC doc plus sync calls). It'd be good to add all CCs as reviewers in PRs that can change how we are solving problems at a fundamental level.

[vkjr]

@ilmotta, I agree that we need to formulate some architecture goal toward which we are going to move as a team and engage more CCs to assess this goal 👍

I think that significant problem with this particular PR is that I decided to keep local screen state like input value inside the rf db which is probably overkill and I'm totally okay with giving up on this idea in favor of react hooks for example.

And I agree with your take that the re-frame layer should model data independently from the view layer but to some extent. For example we have send flow that consists of few screens and we need to save the choices made on those screens - what token we are going to send, to what address and what amount. And I see no better choice than to store that in rf-db. But don't think we should store as much as we do now. For example we can save token symbol :ETH as a user choice and don't need to copy full token structure under [:wallet :ui :send] because there is another part of rf-db where that token can be found by symbol. Similarly we probably don't need to store route data on ui level of rf-db because that leads to UI containing logic on how to operate with that route structure while it can be done on lower levels.

So, if instead of squeezing the ugliness out of UI you would prefer to start with squeezing the ugliness out of data layer - I'm in :) I think that when we start to apply some rules at any level, other levels will also get better.

As I see it, we probably need to formulate requirements that can be applied to our data. And to formulate them we need to pose, discuss and answer bunch of questions. For example:

• what we want and do not want to store in rf-db?
• where local screen states should be stored?
• if we have flow that consists of few screens, where do we want to store the state of the flow?
• where navigation state should be stored?
• do we want the data received from status-go to be shaped before storing or we are storing it as-is and convert later?
• how we want to structure rf-db? plain? nested? What top-level keys or namespaces we can strictly assign to specific purposes?

wdyt?

[ilmotta]

These are excellent questions to start with @vkjr.

Because there can be many answers to each question and even personal preferences, in my experience I saw that it helps if we also try to build/have guiding principles. We could call constraints, or even architecture depending on how we do. More than guidelines, they would play a role similar to the strategy being built for Status right now. Without that, answers can feel arbitrary or biased towards those that scream louder.

For example, if one of our constraints is that we should model the app-db more like a relational database doing schema on write, then that constraint by itself already guides us in answering a few questions:

• Less nested (not necessarily plain).
• Decode and transform when writing to the app-db. Consider malli schemas to represent some parts of the db (schema on write).
• Supporting normalization to facilitate efficient queries (subs) from nodes in the tree.
• Orthogonal concerns would be persisted (like navigation), but not local input state. Things in the middle could use a React Context. State traveling too many components could also use a React Context, but not necessarily re-frame (here the answer is probably "it depends" for a lot of cases).

I tend to like this theory of constraints or something of that kind to reduce the solution space and help establish coherent solutions. And we can have competing constraints that we can benchmark and challenge. Some might prefer a global state that behaves more like MongoDB, and so on.

It would be great if we could start with the problems and have a document to guide our progress towards these architectural discussions and eventually decisions. We could then resurrect doc/decisions/ once we reach consensus. We might also want to have a small focus group of people that will drive this kind of initiative (somewhat like a crew (?), short-lived).

[vkjr]

@ilmotta, yeah, we definitely need to raise and discuss all these questions. Because my preference is to keep everything in rf-db including flow states and probably even ui states and doing this in a highly hierarchical way :) Along with restrictions that some parts of code can access only some subset of subscriptions.
I see this approach more convenient because within reagent components re-frame machinery like subscriptions look much nicer than react hooks. And it is much more convenient for developer to see all app state in rf-db.
So probably along with answering questions we will need also some experiments, like performance measurements to support point of view :)