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

[Question]: What is the most performant way for dynamically adding atoms or focus atoms for a large tree? #447

Closed
sarmadsangi opened this issue Apr 24, 2021 · 21 comments

Comments

@sarmadsangi
Copy link

Hi folks,

First of all, great work done with Jotai. Good documentation too.

I have a use-case where i am require to maintain a component tree and its styles, which are to be rendered on a page. Each of this component's style can be modified. At the moment i use a single useState to update the tree whenever a component style is updated. I feel Jotai can help optimise for performance here as this tree can grow quite large.

Example tree:

const componentTree = {
  id: 'root',
  el: 'div',
  props: {
     style: {
       background: 'grey'
     }
  },
  children: [
    {
      id: '1dfsg2',
      el: 'p',
      props: {
        style: {
          fontSize: '16px'
        }
      },
      children: 'Heading'
     },
     {
      id: '38nsd7',
      el: 'div',
      props: {
        style: {
          marginTop: '10px'
        }
      },
      children: [
        {
          id: '32fss2d',
          el: 'p',
          props: {
            style: {
              fontSize: '12px'
            }
          },
          children: 'Sub text'
         }
      ]
     }
  ]
 }

My question is what is the best way to add state to this given we have dynamic tree. I saw an example here which deals with a similar problem using focusAtoms https://github.com/pmndrs/jotai/blob/master/docs/advanced-recipes/large-objects.md

However, in the above tree the only unique field i have is an id, which is auto generated whenever a component is added to the tree. I can probably use that to dynamically create an atom but issue with that approach is, how do i update the whole component tree itself.

Would appreciate some help/suggestions here on how to best use Jotai for my example.

Cheers :)

@dai-shi
Copy link
Member

dai-shi commented Apr 24, 2021

Assuming you don't need any serialization, making an atom for the object with the id is a primary approach, and probably performant.

I can probably use that to dynamically create an atom but issue with that approach is, how do i update the whole component tree itself.

So, basically this. But, nesting atoms in a tree might not be handy in your use case. (Because, you can only add an item to its parent.)
In such a case, using a big tree and focusAtom/splitAtom would be easier.

Creating id tree and using atomFamily can be another option. We don't have any examples with this approach, though.

Some previous discussion: #255

@yf-hk
Copy link

yf-hk commented Apr 26, 2021

I'm working on a project with a pretty similar use case. I do have the need to serialize it though.

What I'm currently doing is by creating the id using Symbol.for(id) so id field won't be serialized. And at the same time, I created a new object called idToPath:{[id: symbol]: string}, to map id to the path of the component. For example, {Symbol.for('32fss2d'): 'children.1.children.0'}.

So every time there's a change of order or add/remove of component, I manually update the map.
Then with the help of a focusAtom, I can use something like focusAtom(componentTreeAtom, optic => optic.path(path)) to update/read a specific component with the path info aquired in the idToPath map object, by just giving an id as parameter.

I'm not sure if it's an ideal way of doing it, maintaining the map is a bit cumbersome (I currently rebuild it every time there's such a change) but otherwise it works alright so far.

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

It sounds like there could be a better way.

Can either/both of you create a smaller example to play/discuss with?
Do you still see difficulties with one-level-deep tree (aka list) or only with infinite-deep tree?

btw, serialization only might not a big issue, de-serialization is the hard part.

(This would become a good example in docs.)

@yf-hk
Copy link

yf-hk commented Apr 26, 2021

@dai-shi
Sure, here's my implementation. It's not very efficient though:
https://codesandbox.io/s/epic-mcclintock-ybjk4

With this implementation it barely works, and de-serialization would be super easy. But I still feel there are a lot of things can be improved, which I'm having a bit difficulty finding out how.

I tried to use focusAtom and atomFamily to simplify it a bit, but there are a few errors and I have no idea how to fix so I deleted the code.

Also, I kind of feel like it'd be very convinient if there is a getter atom which can take an extra argument as input. For example atom((get, id) => get(someAtom)[id])). I haven't found good way to do the same thing using atomFamily if id changes constantly, like from user's input

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

Thanks for the example! It's small enough to understand the structure.
It's still too big to give the complete rewrite from my side, so let me give some comments.

First of all, we have two basic approaches.

  1. big atom approach
  2. nested atom approach

what you are doing is 1 and it's fast for (de)serializing, but updating can be slower (still enough efficient). On the other hand, 2 is faster for updating, but slower for (de)serializing. I've only tried a simple one (todos app) for 2. If you go with 2, it will be the entire rewrite.

The biggest question in your current approach is whether you need to keep updating idToPathMap on your own. I guess you can make it a derived atom and the elementsAtom becomes the single source of truth.

After that, only remaining thing is to utilize focusAtom and splitAtom to render elementsAtom. At that point, I'm not sure if idToPathMap is necessary, as you don't need id to update an element.

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

I tried to use focusAtom and atomFamily to simplify it a bit,

atomFamily is another option, and that might fit with idToSomethingMap structure. I don't have much experience/thought on it.

a getter atom which can support an extra input argument.

Isn't it selectAtom? But, I guess it would be just fine with your current approach.

if id changes constantly, like from user's input

Yes, we'd need useMemo(() => atom(...), [...]) pattern.

@yf-hk
Copy link

yf-hk commented Apr 26, 2021

@dai-shi
Thanks for your comments. It's really helpful.

For my use case I need to seralize it every time I change it, for keeping a history so it can undo / redo changes. The reason why I created an idToPath map instead of a derived getter atom is that I think it doesn't always need to be updated if the modification is just the payload which happens very fraquently, instead of adding / removing / reordering elements which in my case are less fraquent. But still, I originally thought it's better to just update the changes, instead of traversing the entire tree again. It takes some time to write the logic though. And I'm always open for improvements.

I'll try to think a bit more about it and probably improve the code.

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

I need to seralize it every time I change it

I see.

it doesn't always need to be changed

That makes sense. We could try optimization later though and there might be some other ways to optimize.
But rather, my point is we might not need the map at all. (depends on where you update elements)

@yf-hk
Copy link

yf-hk commented Apr 26, 2021

But rather, my point is we might not need the map at all. (depends on where you update elements)

That'll be even better. It's just that I haven't found a way to do it with my limited experince of this module. But I'm happy to see better implementations

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

Have you seen splitAtom?
If you could make component tree structure the same as data tree structure, you could eliminate the map. That's the approach in https://docs.pmnd.rs/jotai/advanced-recipes/large-objects.

@yf-hk
Copy link

yf-hk commented Apr 26, 2021

Have you seen splitAtom?

Yes, I've noticed that. So let's say there is a feature that I'd like to select a component and change the order of it (move up and down relative to its siblings), or set a different parent component, or to delete some of the selected components. Is it doable using splitAtom?

@dai-shi
Copy link
Member

dai-shi commented Apr 26, 2021

Yes, it should! It's a relatively new function, so you might expect some bugs, but yeah that's the goal.

For reordering, your id is important which should be specified with keyExtractor.

@0xycvv
Copy link

0xycvv commented Apr 29, 2021

For reordering, your id is important which should be specified with keyExtractor.

I was trying to do reorder using splitAtom, don't know how, the splitAtom return only a remove function?

@dai-shi
Copy link
Member

dai-shi commented Apr 29, 2021

Oh, right. Yeah, I mean you reorder the original array atom and splitAtom works well with keyExtractor. To be honest, I haven't tried it. So, I'm happy to help your example.

@yf-hk
Copy link

yf-hk commented May 2, 2021

Yes, it should! It's a relatively new function, so you might expect some bugs, but yeah that's the goal.

Hi @dai-shi , could you please shed some light on how to implement similar logic with splitAtom and keyExtractor? After doing a bit of research, I still haven't found the correct way of doing it. Many thanks!

@dai-shi
Copy link
Member

dai-shi commented May 2, 2021

Sure.

So, I made this example.
https://codesandbox.io/s/react-typescript-forked-4dzl6?file=/src/App.tsx
...and found a bug. The very first time, if you move an item, it show two duplicated items. But, it seems this weird behavior is only for the first time. cc @Aslemammad

@Aslemammad
Copy link
Member

@dai-shi After 443, I'll get back to this one, but maybe @PuruVJ can figure this out if he's interested.

@yf-hk
Copy link

yf-hk commented May 7, 2021

Sorry if my comment is not appropriate to make here but after doing a bit of experiments, I found that mobx-keystone works perfectly well, in aspects like feature richness, easy of use / architecture of user's code and performance as well, when working with a large tree structure as state. Actually it's specifically designed and tailored to be working as a tree, and everything inside of its state is a node. Snapshot (serialization) support is also bult-in.

jotai is another top grade library too with wonderful community support and clever thinkers as contributors, but with my experince it isn't quite convinient when dealing with tree structures. I'd definately like to use jotai in projects that I don't need to maintain a tree though, it's so simple to use.

@dai-shi
Copy link
Member

dai-shi commented May 7, 2021

Yeah, we need more best practices and some utils to deal with tree structure effectively. For now, we are still working on a flat list and splitAtom. Jotai core is pretty solid. After all, it's simple global state modeled after useState. (Plus, derived state + suspense, which is big.) Then, things are built on top of it. Some are easy, and some are hard. I call this task/problem/process "jotai puzzle".

@dai-shi
Copy link
Member

dai-shi commented May 19, 2021

#447 (comment)

This was actually my bad. I forgot key={}. And, it's working fine with the latest version (v0.16.5). It's broken in v0.14.5.

https://codesandbox.io/s/react-typescript-forked-5cnu1?file=/src/App.tsx
It's working as expected.

@dai-shi
Copy link
Member

dai-shi commented May 31, 2021

This has been a great discussion. We've fixed some bugs.
The current recommendation for tree structure is using splitAtom and focusAtom.
However, we noticed some use cases should be handled more nicely.
We will continue to explore best practices and collect feedbacks.

Closing this now, and for further discussions, please open a new issue/discussion.

@dai-shi dai-shi closed this as completed May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants