dashed / todomvc-providence

http://dashed.github.io/todomvc-providence
1 stars 1 forks source link

orwell implementation #3

Open dashed opened 9 years ago

dashed commented 9 years ago

Tracking tasks:

dashed commented 9 years ago

@andrewluetgers If you're still interested, you may want to check out lib/orwell.js.

andrewluetgers commented 9 years ago

Sorry for the epic post: Looks very interesting, I am currently using something like this at work on a new project and has been working out quite well. I will share with you what I have now since it is similar to what you are doing but the approach I have taken is a little bit different. I am working on a sample application for it as well and will point you to it when that is available but for now I'll give a little peek at it perhaps it will give you some ideas.

You define a centralized state object (obviously) and in your components you describe cursors into that state object, either in the component or in the props via a cursors object.

var stuffModel = require('./state/stuff/stuff');

component("MyComponent", {
    cursors: {
        foo: "stuff.foo",
        bar: "otherStuff.baz.bar",
        thing: "thing"
    },

    changeFoo: function() {
        stuffModel.setFoo(this.foo + " updated");
    },

    render: function() {
        return (
                <div onClick={this.changeFoo}>foo is: {this.foo}</div>
                <div>bar is: {this.bar}</div>
                <div>thing is {this.thing}</div>
        );
    }
});

This component is declaring what parts of that state it cares about in the cursors object. Behind the scenes observers are set up on each of the parts of the state that are declared. When any of those parts of the state change the component is forced to render, bypassing the shouldComponentUpdate.

As a convention the cursors section defines keys that will become serialized state placed directly on the component. On every swap the entire immutable state object is converted to json, that json state is then used to populate this.foo, this.bar and this.thing.

In the component I mostly never want to actually deal with the immutable data-structures directly. The serialized state keeps me from needing to use immutable getters and for updates I put all the update code in another file, you can think of that as the model for the data. I also put any ajax code or computed value getters in here as well.

So this is working out nicely, I actually very rarely even use this.props or this.state. IMHO this approach has been much simpler to work with and think about. Obviously there are implications if you want to create reusable generic components. Being coupled to a specific state object and its structure will make the code less portable. I have not found this to be a problem because I'm not building generic components I'm building an app. I find that there is A LOT of time wasted making generic this and that when really nobody will ever use it again anyway. So I don't bother. If and when something does need to be made generic, You can all ways fall back to the more traditional React style.

But there is a problem here, in my app the only time there is a top down render is when the route changes, otherwise its purely just the components that have had their state changed that are forced to render. In practice I have not had to think much about this but its possible to mix the two paradigms and have problems if you are expecting to have a child render because the props changed but the parent did not get rendered.

dashed commented 9 years ago

@andrewluetgers Awesome! Looking forward to see it in the wild.


The main inspiration for orwell was from this article: https://medium.com/@dan_abramov/mixins-are-dead-long-live-higher-order-components-94a0d2f9e750 (if you haven't read it, do so; it's pretty insightful!)

I began with lifting the connectToStores code, and gradually refactoring for use with cursors instead of flux stores to eventually getting a decent development story.

With regards to props, I still use this.props, and I have no problems using destructuring: const { foo, bar thing } = this.props. I use const liberally whenever I can.


Looking at your usage example, one thing that stood out was usage of keypath strings with the dot as the delimiter.

I think arrays may work just as well to keep with the keypath convention of the immutable library; but it may be fine if keypath strings are an established convention for a specific project:

component("MyComponent", {
    cursors: {
        foo: ['stuff', 'foo'],
        bar: ['otherStuff', 'baz', 'bar'],
        thing: ['thing']
    },

    // ...
});

How do you deal with dynamic keypaths (e.g. rows/columns of a table)?


In the component I mostly never want to actually deal with the immutable data-structures directly.

I think it's still useful to have some access to them. What if ['stuff', 'foo'] points to an immutable map? Wouldn't this.foo be a map? Or is it resolved into some value type?

But there is a problem here, in my app the only time there is a top down render is when the route changes, otherwise its purely just the components that have had their state changed that are forced to render.

Top-down render is still definitely useful, especially for dumb components: https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0

I ran into a similar issue with react-router (assuming you're using this); when I tried having the component to re-render in any other way besides new props from above. It may be the too-much magic of react-router; hopefully their v1.0 rewrite simplifies this in some way.


When any of those parts of the state change the component is forced to render, bypassing the shouldComponentUpdate.

I haven't adapted orwell to utilized this.forceUpdate() just yet; I've felt using it seemed to an anti-pattern. I tried to avoid using it when implementing todomvc. But I didn't want to restrict this flexibility; so it's on my radar to be able to use it.


When developing orwell, I realized I wanted components to update when observed cursor changed only when a validation step has passed. For example: https://github.com/Dashed/todomvc-providence/blob/gh-pages/src/components/all.js#L12-L27

I'm wondering how you would cover this use-case.


I'm uncomfortable with this: var stuffModel = require('./state/stuff/stuff'); Unfortunately, a singleton object does not work well in an isomorphic environment. :disappointed:

andrewluetgers commented 9 years ago

How do you deal with dynamic keypaths (e.g. rows/columns of a table)?

you can pass them in from the parent constructing the path string dynamically

return items.map((v, i)=> <MyComponent cursors={{thing: 'stuff.'+i}})/>)

however you may be better off passing the value itself in via params, MyComponent will be rendered so long as the parent has "stuff" in it's cursors section or otherwise has a parent that does.


What if ['stuff', 'foo'] points to an immutable map? Wouldn't this.foo be a map? Or is it resolved into some value type?

its whatever you get when you do state.cursor(['stuff', 'foo']).toJS(), or state.toJS().stuff.foo I have intentionally kept this very simple, my state starts out as plain old json and so there is no need to support more esoteric data-structures available in immutable. This is also why the string key-paths work just fine, numbers for arrays everything else is keying into plain objects.


I am using react router and doing basically the same as you, no longer using swap to tell the router to render.


I realized I wanted components to update when observed cursor changed only when a validation step has passed... how you would cover this use-case

I have not considered this, I would need to add some kind of validation hook to the code that calls forceUpdate, it may be an optimization too far for my implementation.


I was just going through the code and totally forgot that I actually switched up the update strategy about a month ago. I actually don't use references and cursors anymore LOL sorry (totally forgot), although the end result is almost exactly the same I now have one top level swap observer that does this...

appState.on('swap', function(n, o) {
    n = n.toJS();
    o = o.toJS();
    ops(operations.handlers, n, o, diff(o, n));
});

I'll try to explain the reason for this change without getting too into the weeds. Ops is essentially the top level application controller. It executes the "operations" defined for the application. These operations do pattern matching on the state given the new, old, and diff of the two and then decide weather or not to execute some code. For example, if the clientId URL parameter changes we kick off the ajax to fetch a different user, this happens before any rendering takes place. Other solutions to this such as fancy routers or putting everything in components has never satisfied me. Once all ops are done we take a look at that diff to determine what components need to be forced to update. I iterate over all the "cursors" (its kind of a meaningless word now) defined in the components and if any of those paths can be found in the diff we execute the listener, invariably this will cause some component to update. Whew! Ok so the reason for this refactor was the problem of having multiple paths for updates when the url updated it would sometimes cause changes to the state so that would lead to a weird situation where there would be a top down render and a differential render. I did't like that. This way I have more control over execution, i.e. If the url changes I call the render for the router after operations execute and after all the direct renders have happened.

dashed commented 9 years ago

Do you do a DFS diff check with diff(o, n)? This sounds computationally expensive.

andrewluetgers commented 9 years ago

Yes, depth first, its not cheap, but in practice the state object is quite small and simple and not too deep. Compared to the diffing of the virtual DOMs or rendering any actual changes I think this is not bad at all. I timed my current app and the average diff including the serialization/copy of the .toJS() was around 9ms over 20 swap operations including app init and some interactions. Max was 58ms min was .8ms, the ops call is similar.

andrewluetgers commented 9 years ago

Loading large datasets into state will affect this i'm sure but to what extent? I will find out in time as we progress with this app, I don't foresee it being any worse than the alternatives but I suppose it could perform poorly on some datasets and usage patterns.