mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.51k stars 1.77k forks source link

Considerations for trees and filtering #62

Closed kmalakoff closed 8 years ago

kmalakoff commented 8 years ago

This looks very interesting!

I'm currently using redux and the connect decorator to build and filter a tree dynamically. Basically, I use chokidar in my electron app to watch a folder and build an in-memory representation of it. Then, I allow for text-based filtering plus some modes (like flatten or hierarchical) to build another tree that gets rendered reasonably optimally:

  1. maintain a flat representation watching a folder using chokidar and a normalizr-like representation
  2. depending on the search string and mode (flat or hierarchical) and tree node expanded flags, build a flat array to represent the tree (the depth is a numeric property rather than an actual tree)
  3. use connect on each tree node component to pull out a minimal representation into the properties so each component only renders if it actually changed

How would I do something similarly optimized with mobservable?

(basically, I haven't been able to find an example of mobservable for trees yet but think it could simplify my application logic by operating directly on a tree of nodes rather than jumping through hoops with normalr + redux, am wondering about how cascading transformations between different representations should be implemented, and am looking for more clarity on how @observer and @connect are similar in ensuring minimal re-renders given the transformation step).

mweststrate commented 8 years ago

Hi @kmalakoff

A few comments on how I would approach this.

I wouldn't introduce a flat representation unless this is really convenient for your algorithms. I always suggest to keep the mental model as close to the in memory model as possible as it will benefit the readability (especially for others) of your code base.

After that I would introduce reactive properties that express whether a node should be currently visible, something like:

const currentSearchFilter = observable({
  filter: ''
});

class Folder {
  @observable name;
  @observable fullPath;
  @observable children = [];
  @observable get isVisible() {
      return !currentSearchFilter.filter || 
                name === currentSearchFilter.filter || 
                this.children.filter(child => child.isVisible).length > 0
   }
}

After that just decorate your components with @observable and mobservable will figure out automatically when folders need to be shown / hidden by just changing the currentSearchFilter.filter property:

@observer
class TreeNode extends React.Component {
   render() {
      if (!this.props.folder.isVisible)
          return null;
      return <div><span>{this.props.folder.name</span>
           <ul>
                  {this.props.folder.children.map(child => <TreeNode folder={child} key={child.fullPath} />)}
           </ul>
      </div>
   } 
}

That's in the basis all you need as far as I could conduct from your description :)

Edit: node btw that you could also express the 'isVisible' as expression in your component. It probably depends on the rest of your architecture which approach is nicer

kmalakoff commented 8 years ago

@mweststrate thank you for the quick response! This makes a lot of sense and I like the clear separation between models and components...using stores as you suggest in the documentation is really clear and clean.

Unfortunately, the transformation step is necessary since the display mode can be logical / simplified (eg. removing folders or flattening and merging similar nodes) or raw as on disk, and I'm filtering the tree by tags, search terms, etc so the raw models are not really what I need to render.

How should I do the following with mobservables in a simple and performant way:

raw models -> representation to render -> rendering in components

I know it is a bit more complicated than usual and I'd love to avoid it, but it is the correct design for the problem. I feel like it is related to somehow configuring a reactive transformation pipeline, but not sure how to configure and propagate the observables themselves to avoid re-rendering.


For example, would keeping on creating new observables for the transformed nodes only trigger re-render if the underlying values, but not the observables themselves change?

class DiskItem { /* Folder from above */ }
class DisplayItem {
  @observable isExpanded;
  @observable canExpand;

  constructor(diskItem) {
    this.diskItem = diskItem;
  }

  @observable get isVisible() {
      return !currentSearchFilter.filter || 
                this.diskItem.name === currentSearchFilter.filter || 
                this.diskItem.children.filter(child => child.isVisible).length > 0
   }
}
@observer
class ItemComponent extends React.Component {
   render() { /* similar to TreeNode */ } 
}

The idea is that the DisplayItem's get dynamically generated whenever a searchFilter parameter changes or the DiskItem tree changes.

Currently, when using @connect and knowing that it does a shallow comparison on the values, I know that the ItemComponents only get re-rendered when needed; for example, if I change the active item, @connect will change the 'isActive' boolean on all ItemComponents, but only the maximum two that could have changes get re-rendered.

mweststrate commented 8 years ago

It is good to realize that observable functions of any kind purely observe the values they did actually use and will always update accordingly if something changes. So if an @observer component uses an value, like isActive during its render phase it will update as soon as it changes. What is even more fun; if you slam @observer on each component, a child will rerender when a value it has used but, unlike when using smart components, its parents will not update. And the inverse holds as well, parents can rerender without rerendering their children; @observer actually applies the pure render mixin as well :)

So a TreeRender component will rerender if either the name changes, or the lists of children. But it won't rerender when one of the children itself changes; because it doesn't access the properties of the children! That is done in the child components. In other words the map just picks elements from the array, which is registered as read for the parent treeNode. But the data inside each child is being read only inside the the child components that are being created, which have their own lifecycle.

To put it differently:

var a = mobservable(7);

var b = mobservable(function() { Math.pow(a(), 2); })

var c = mobservable(function() { b()  + 3 });

In this case c only depends on b, because it doesn't access a directly. Nonetheless c will re-evaluate if you change a to 8, because the changes cascade.

On the other hand, if you would update a to the value -7, b would re-evaluate, because a has changed. But c wouldn't re-evaluate, because b didn't change after re-evaluation.


So suppose you the following property on the Folder data structure, and print the whole json structure in an autorun:


class Folder {
  get toJSON() {
     return {
         name: this.name,
         children: this.children.map(child => child.toJSON)
    }
  }

// later

autorun(() => {
   console.dir(theRootFolder.toJSON)
})

From now on, on any change in any name or children list, the autorun will trigger and print the json. But not all toJSON functions will be called again! Only the the folder that changed, and its parent up to the root will re-evaluate their toJSONfunction. So each observable function acts as its own cache as long as the data it observers doesn't change.

... To be continued, have an appointment right now :)

mweststrate commented 8 years ago

... So what Folder.toJSON and React components have in common is that each time you render / read you get the same observable which make it really efficient.

So in your case (which sounds completely valid to me) I think you should just start modelling your intermediate state as a second state tree which has (almost) only derived properties as members. That should be pretty straightforward.

But the second important thing to do is that you reuse parts of this tree as much as possible, in essence, given a folder and your transformation function, you want to be able to reuse the same intermediate representation if your transformation function has been applied to he same folder earlier. So probably you need a cache for that mapping, or store reference(s) to the display item on the original folder (the latter is a bit easier as you don' t have to clean up, but maybe less flexible as well). Remember that the second time you apply the same transformation to the same folder you don't have to actually apply it! Your derivation is reactive so that will be done automatically for you.

So something like:

class DiskItem {
  @observable name;
  @observable fullPath;
  @observable children = [];
  @observable get isVisible() {
      return !currentSearchFilter.filter || 
                name === currentSearchFilter.filter || 
                this.children.filter(child => child.isVisible).length > 0
   }
}

class DisplayItem {
  @observable isExpanded;
  @observable canExpand;

  constructor(diskItem) {
    this.diskItem = diskItem;
  }

  @observable get chidren() {
         return this.diskItem.children.map(transform);
  }

  @observable get isVisible() {
      return !currentSearchFilter.filter || 
                this.diskItem.name === currentSearchFilter.filter || 
                this.children.filter(child => child.isVisible).length > 0
   }
}

// using a cache, a simpler approach would be to just store a reference on the diskItem
var displayItems = {}; // map with key: fullpath, value: display item

function transform(diskItem) {
     if (!displayItems[diskItem.fullPath])
          displayItems[diskItem.fullPath] = new DisplayItem(diskItem);
     return displayItems[diskItem.fullPath];
}

I have the strong impression that there is a generic pattern here, that can be applied to avoid the burden of needing to 'cache'.

Let me know whether this helps!

kmalakoff commented 8 years ago

@mweststrate this is exactly what I was looking for. It confirms my hunch that the intermediate representation needed to reuse the identical observables. Thank you!

FYI: my current implementation has a cache for ensuring the same items are looked up, but like you say, it is open to accumulating memory so I like your idea of storing them on the original nodes to allow them to be garbage collected. If I come up with a more generic transform solution or pattern, I'll let you know.

I'm going to run some sandbox experiments with my more complex use cases over the coming days by reimplementing parts of this app in mobservable. I've closed this for now and will reopen if needed.

I really like the simplicity of what you have done...fewer hoops to jump through and less code than redux and much simpler to learn!