Quo component schemas with open vs. closed maps

[clauxx]

We had a discussion a while back in the #malli discord channel which we didn't finish. Best if we continue it in a more "centralized" place to collect our opinions and arrive at a decision that can go into our guidelines.

for open maps:

• allows more data to be passed to components than it uses i.e. usually from data coming from a subscription
• less rigid about prop requirements

for closed maps:

• ensures only the data needed by the component is passed on, avoiding unnecessary re-renders
• avoids bugs where props are misnamed or have typos

above is what I remember as the main points/arguments from the prior conversation

[J-Son89]

My vote is for Quo components is that we used closed by default, and we allow open maps for previous components that would take a lot of refactoring to clean up immediately 👍
It seems we generally want to avoid passing excess props along anyway so it's a better safeguard to have this imo

[J-Son89]

By Component schemas what does this mean exactly though, can we be more specific of where we want this?

[clauxx]

Quo components for now, as we dont write schemas for status-im components

[shivekkhurana]

Note: Quo Components Only

Utilizing closed maps implies foregoing the ability to drill props down (i.e., passing an object to children rather than explicitly passing keys).

Though it necessitates additional lines of code, I fully endorse closed maps. I advocate for their consistent use. It's acknowledged that implementing them mandates refactoring each component call when writing the schema.

[ilmotta]

I'm starting to be more in line with your arguments @ulisesmac. Just as we are trying to keep guidelines more flexible in the myriad of changes due to performance, we could do the same with schemas and start with the most flexible solution and learn from there.

[cammellos]

One thing though, I appreciate arguments both & cons, but

learn from there

We did learn from this, we worked on this project long enough to know the cost of not being on top of performance, but just last week I had to change the community code because it was re-rendering the header every time we:

1. Checked permissions
2. Anything happened on a community (message received, spectated, etc)

This was happening because of two reasons:

1. Wrong structure of the database
2. Components were taking too many props as parameters

Both have been a factor, if either was coded correctly, we would not have had this issue, and it produced tangible work, since now have created extra issues to be picked up to correct the implementation.
There's a difference between focusing on optimization (which we should not, unless necessary), and write code that is efficient, those are two different things, and the latter come at little to no cost most of the times.

With regards of open vs closed maps I think the way we should think about it is that:

"You must be this tall to use open maps."

Are we this tall? I think that's the question we want to focus on, and if we are, sure, but if we aren't, then it's either we need to get there somehow by being careful and building up experience, or we use guardrails (closed maps) until we are tall enough.

Or as Peter would put it:

[J-Son89]

Poor Uncle Ben 🥲

[ilmotta]

In the clojure.spec days nobody was tall enough to close specs because it was not possible by design. But I like to have the choice to close in Malli. Whatever is decided will be fine by me. I won't object to any decision. But as @cammellos just pointed out, it looks like we're not tall enough yet.

The nicest part of it all to me is the discussion behind close vs open, even if the final decision can be somewhat arbitrary because both camps have merit 👍🏼

[ulisesmac]

@cammellos

I had to change the community code because it was re-rendering the header every time
...
This was happening because of two reasons:

1. Wrong structure of the database
2. Components were taking too many props as parameters

Regarding to 2,
Reagent re-executes its render functions when their props change and, AFAIK, that's an extra optimization on top of React's default behavior since Reagent just generates the VDOM needed and React the whole VDOM.

So, in the worst case, are we rendering the same as plain react (ofc, with the cost of using hiccup)?
As I said before, let's see if the performance arguments for using closed maps are still valid after dropping reagent.

In the meantime, I completely share what @ilmotta said:

I won't object to any decision. But as @cammellos just pointed out, it looks like we're not tall enough yet.