ProductiveRage / Bridge.React

Bindings for Bridge.NET for React - write React applications in C#!
MIT License
74 stars 14 forks source link

Support Redux #9

Closed Zaid-Ajaj closed 8 years ago

Zaid-Ajaj commented 8 years ago

React + Redux would be pretty nice to have. We only need something like react-redux since I already gave the Redux library a try myself 😃 Take a look! The example project demonstrates how it is used. Redux middleware are supported as well and Thunk middleware is also included by default.

ProductiveRage commented 8 years ago

Sorry I've taken so long to reply - I had a very quick glance at this last week but haven't been able to look into it properly yet!

Zaid-Ajaj commented 8 years ago

Thats no problem! I look forward to hearing your thoughts on the Redux bindings and on how to approach the integration of the two libraries.

ProductiveRage commented 8 years ago

I've started looking through your example project code. I've not got to the point where I understand it all yet, partly because I need to go back and read up on Redux again!

I'm thinking that the best approach to relating your bindings with Bridge.React would be for the Redux bindings to be a separate NuGet package that has a dependency on Bridge.React - is that the sort of thing you had in mind?

Zaid-Ajaj commented 8 years ago

Yes, indeed. Having Bridge.Redux seperate is the logical thing to do as it is an independent library of it's own and knows nothing about React. This is the part where the react-redux comes into play. It is the library that lets React know how to talk to Redux by giving two things, first: the Provider component, this is a React component that wraps the main app. A connect decorator that tells some component of the app which part app state object it will handle.

Mechanically translating the Provider component looks not too difficult, maybe something like this:

ReactElement Provider(Store myStore, ReactElement mainApp) 
{
    return Script.Write<ReactElement>("React.createElement(ReactRedux.Provider, { store: myStore }, mainApp)");
}

As for the connect decorator part, since it is a decorator, I don't know how that translates into ES5 syntax.

Could you please elaborate on what exactly you found confusing in my code? I might be able to help explain things

ProductiveRage commented 8 years ago

I didn't find anything confusing in your code, per se, I'm just trying to get back up to speed with Redux (and how it varies from "classic" Flux) and what is required for the React Redux aspect of it.

The basics about stores, action and reducers are coming back to me from times that I've read about Redux in the past, having looked at your code. I'm just trying to work out how the Connector ties components back to the store, to allow state changes to be communicated. Like I said, I'm catching up on the Redux basics.. (starting with this article, which seems good: Writing a Basic App in Redux.. hopefully it all still applies since these technologies move so fast! One of the comments on the posts suggests otherwise..).

Ok. I've read that and now I've also read How to Build a Todo App Using React, Redux, and Immutable.js, which was written more recently (March 2016 rather than July 2015) and that seems to describe it differently!

The newer article still has a "Provider" at the top level, which is basically just a component that allows the top level / "container" component (the "TodoList") to be "store-aware", meaning that it has access to two functions "mapStateToProps" and "mapDispatchToProps". A "connect" method is used to make these available to the component but there doesn't seem to be any other concept of a "connector".

This seems like good news because the older article suggested that all components might have access to dispatch actions directly, via sharing references in React "context" (which is a bit voodoo magic and isn't supported by the Bridge React bindings).

So.. the Provider component barely even needs to be a class, I don't think. In fact, it doesn't even need to be a generic / re-usable component.. it could just be that you have an "TodoApp" top level component that has a props class that has properties that are the methods "mapStateToProps" and "mapDispatchToProps" and then the child components that it renders pass up actions that they want to dispatch.

Unless I'm missing the point, then, can React / Redux not be implemented as a pattern by using the Redux bindings that you've provided and the current React bindings here?

One other thought - that's barely related to this conversation - when you create actions for setting user name or user age, you use the same class "SimpleAction" but only use a subset of the properties for particular actions (eg. SetUserName doesn't need the Age property and SetUserAge doesn't need the Name property). Do you think that it would make more sense to have more specific action classes - a particular action class for setting user name (that only has the new name as a property) and a particular action class for setting user age (that has only has the new age as a property)? Instead of testing the ActionType enum, you could test the type of the class. Just a thought.. what you're doing currently looks like most of the examples "in the wild", but that's because they're for JavaScript which doesn't favour classes so much, whereas they would feel very natural (to me) to use in cases like this!

ProductiveRage commented 8 years ago

Oh, I meant to add one more thing - I got a bit confused when I first tried your sample because I tried to open the demo.html file in the "Bridge.Redux" project instead of the "Bridge.Redux.Examples" project!

When you add the Bridge NuGet package, it automatically creates the "Bridge" folder (with bridge.json, the "www" folder, etc..) but if you're creating a library project then you can add the "Bridge.Min" package instead and that will just add the Bridge translator and the runtime library dependencies but not the "Bridge" folder. Keeps your library project tidier! (If you need DOM references then also add the "Bridge.Html5" package).

Zaid-Ajaj commented 8 years ago

Ok. I've read that and now I've also read How to Build a Todo App Using React, Redux, and Immutable.js, which was written more recently (March 2016 rather than July 2015) and that seems to describe it differently!

Thanks for pointing this out as this article was helpful in explaining things. Good thing is that the "connecter" was not a decorator or some other magic.

Unless I'm missing the point, then, can React / Redux not be implemented as a pattern by using the Redux bindings that you've provided and the current React bindings here?

Correct, I wrote a one-to-one translation of the todo app example made in the article but I came across React errors that I didn't understand :( then I realized that a solution was to just forget about how the bindings work and just use what React gave me with the existing Redux code. It eneded up working!!! the only caveat is that it re-renders everything after every store dispatch which could be problematic, performance-wise. This is how it looks like:

var appReducer = Reducers.ItemsReducer();
var initialState = new TodoItem[] { };
var store = Redux.CreateStore(appReducer, initialState);
Action render = () =>
{
    var todoList = Components.TodoList(new TodoListProps
    {
        Todos = store.GetState(),
        AddTodo = text => store.Dispatch(Actions.AddTodo(text)),
        ToggleTodo = id => store.Dispatch(Actions.ToggleTodo(id))
    });
    var appContainer = Document.GetElementById("app");
    React.Render(todoList, appContainer);
};
// Invoke render with every state change
store.Subscribe(render);
render();

One other thought - that's barely related to this conversation - when you create actions for setting user name or user age, you use the same class "SimpleAction" but only use a subset of the properties for particular actions (eg. SetUserName doesn't need the Age property and SetUserAge doesn't need the Name property). Do you think that it would make more sense to have more specific action classes - a particular action class for setting user name (that only has the new name as a property) and a particular action class for setting user age (that has only has the new age as a property)? Instead of testing the ActionType enum, you could test the type of the class. Just a thought.. what you're doing currently looks like most of the examples "in the wild", but that's because they're for JavaScript which doesn't favour classes so much, whereas they would feel very natural (to me) to use in cases like this!

I agree, I haven't decided yet what design is the final way to go. C# Polymorfism does sound a good candidate for reducer construction where the execution of different code blocks of the if-else clauses can deferred to different descendants according to their type. It would be idiomatic C# and surely look more clean. Still thinking about how to implement that.

When you add the Bridge NuGet package, it automatically creates the "Bridge" folder (with bridge.json, the "www" folder, etc..) but if you're creating a library project then you can add the "Bridge.Min" package instead and that will just add the Bridge translator and the runtime library dependencies but not the "Bridge" folder. Keeps your library project tidier! (If you need DOM references then also add the "Bridge.Html5" package).

Wow, thats awesome. I didn't know that. Thanks man!

ProductiveRage commented 8 years ago

First off, I think that we can improve your re-render every time mechanism. In your TodoList implementation, you currently use functions that return a ReactElement as your component renderers. This is fine for "stateless components" where everything that you need to render is in the props reference and where you don't need any of the React component life cycle methods. 99% of your components should render only according to props and not require life cycle metods, so this is generally a good thing.

However, sometimes you want a component that has state as well as props because state has some magical properties. The most important one is that every time that you call SetState from within a component that has state, it will trigger a re-render of the component without you having to explicitly call the "Render" method again.

So I would create a "container" class (some guidelines specify a difference between "container" components and "presentation" components - the container only deals with tying state together and doesn't render anything directly while presentation components only render things directly from props and pass action up) like this:

public class TodoListContainer : Component<TodoListContainer.Props, IEnumerable<TodoItem>>
{
    public TodoListContainer(Store<IEnumerable<TodoItem>> store)
        : base(new Props { Store = store }) { }

    protected override void ComponentDidMount()
    {
        props.Store.Subscribe(() => SetState(props.Store.GetState()));
    }

    protected override IEnumerable<TodoItem> GetInitialState()
    {
        return props.Store.GetState();
    }

    public override ReactElement Render()
    {
        return new TodoList(new TodoListProps
        {
            Todos = state,
            AddTodo = text => props.Store.Dispatch(Actions.AddTodo(text)),
            ToggleTodo = id => props.Store.Dispatch(Actions.ToggleTodo(id))
        });
    }

    public class Props
    {
        public Store<IEnumerable<TodoItem>> Store;
    }
}

Then you could change you App.cs "Main" method to the following:

public static void Main()
{
    var appReducer = Reducers.ItemsReducer();
    var initialState = new TodoItem[] { };
    var store = Redux.CreateStore(appReducer, initialState);
    React.React.Render(
        new TodoListContainer(store),
        Document.GetElementById("app")
    );
}

This will still "fully re-render" every time that the state changes - however, it's only a React virtual DOM re-render, which is much cheaper than a real DOM re-render. So we probably don't have to worry about it; this is what React is designed to do.

However.. if you want to go a step further then you could help React out a bit. If you replace your static render methods with component class that are derived from PureComponent like this:

public class TodoList : PureComponent<TodoListProps>
{
    public TodoList(TodoListProps props) : base(props) { }

    public override ReactElement Render()
    {
        return DOM.Div(new Attributes { },
            DOM.Input(new InputAttributes
            {
                Type = Html5.InputType.Text,
                Placeholder = "Add todo",
                OnKeyDown = e =>
                {
                    var textInput = e.CurrentTarget.Value;
                    if (e.Which == 13 && textInput.Length > 0)
                    {
                        props.AddTodo(textInput);
                        e.CurrentTarget.Value = "";
                    }
                }
            }),
            DOM.UL(new Attributes { },
                props.Todos.Select(todo =>
                {
                    var attrs = new LIAttributes
                    {
                        Key = todo.Id,
                        OnClick = e => props.ToggleTodo(todo.Id)
                    };
                    return DOM.Li(attrs, new Todo(todo));
                })
            )
        );
    }
}

public class Todo : PureComponent<TodoItem>
{
    public Todo(TodoItem props) : base(props) { }

    public override ReactElement Render()
    {
        var attrs = new Attributes { Style = props.IsDone ? Styles.Done : Styles.NotDone };
        return DOM.Div(attrs, props.Text);
    }
}

.. then you get some under-the-hood optimisation magic applied. Any "PureComponent" will automatically get a "shouldComponentUpdate" implementation (it's not visible in the C# code but it's provided for React) that will return false if the new props reference has all of the same data as the current props reference. You can see this by adding the following line at the top of the "Todo" class' Render method:

Console.WriteLine("Todo[" + props.Id + "].Render");

Every time that a Todo item is render, it will write to the console. Note that as you add new items, you only get the Render method called for the new item and not for the existing items. If you want to see the difference then change "Todo" to be derived from StatelessComponent instead of PureComponent - now, every time that you add a new item every item in the list re-renders.

This works really well with simple properties like string, int and bool but it gets more complicated with collections or nested object graphs since the shouldComponentUpdate only does a "shallow comparison". If you really want to get the benefits of this then you want to move to using immutable data types. Conveniently, I have a Bridge library to help out with that :D See ProductiveRage.Immutable (also available on NuGet).

I don't know if you saw that I did a three part series about how I think that Bridge React apps should be written? Although it's three parts, each is quite long - but I think you might find it interesting, even if you end up using a slightly different Flux-like store for the code that you want to write. Part one is here: Writing React apps using Bridge.NET - The Dan Way (from first principles). It starts simple but moves on to talk about immutable data types for props, state and the store data and talks about Component vs StatelessComponent vs PureComponent. It also has a nice (imho!) demonstration of a way to describe dispatcher actions using specific classes and show a way to match them cleanly (which relates to what I was talking about yesterday).

ProductiveRage commented 8 years ago

One more note: It's not compulsory to create classes that derive from StatelessComponent or PureComponent when you want to create presentation components - in your example code, you were using the "StaticFunctions" class to create stateless components from simple render methods. This is a valid approach (and there is a "PureComponent" function to do similar for pure components) but I think that it's easier to understand using a class-based approach initially - once you're happy with that, we can always come back to look at the StaticFunctions.PureComponent version.

Zaid-Ajaj commented 8 years ago

Nice! Your container component worked perfectly as expected. However, at first glace, it seemed like a lot of code just to tie the react with redux part. Looking at the types and the how things are being transformed, I was able to abstract it a little. You only need these stuff:

You can guess what will happen just by looking at the types! To contruct the same TodoListContainer the code becomes the following, in our case TAppState = TProps = IEnumerable<TodoItem>

var todoListContainer = ReduxContainer.Create(new ContainerProps<IEnumerable<TodoItem>, IEnumerable<TodoItem>>
{
    Store = store,
    StateToPropsMapper = state => state,
    Renderer = props => Components.TodoList(new TodoListProps
    {
        Todos = props,
        AddTodo = text => store.Dispatch(Actions.AddTodo(text)),
        ToggleTodo = id => store.Dispatch(Actions.ToggleTodo(id))
    })
});

In my opinion the code looks cleaner and it is easier to read. What do you think?

public class Todo : PureComponent { public Todo(TodoItem props) : base(props) { }

public override ReactElement Render()
{
    var attrs = new Attributes { Style = props.IsDone ? Styles.Done : Styles.NotDone };
    return DOM.Div(attrs, props.Text);
}

}

These pure components look like pretty good fit for presentational components. I am still looking into your library on immutable data types and it works.

I don't know if you saw that I did a three part series about how I think that Bridge React apps should be written? Although it's three parts, each is quite long - but I think you might find it interesting, even if you end up using a slightly different Flux-like store for the code that you want to write. Part one is here: Writing React apps using Bridge.NET - The Dan Way (from first principles).

I actually I did come across your blog post a while ago. I was really impressed, I didn't think we were already able to do so much with C# on the browser. That said, I was also overwhelmed by the amount of information. for me it was "meant for a different crowd", thats natural considering my 0 experience with React and with Bridge by that time. Now I am reading it again, and since that I have written some examples with both React and Bridge, it became more accessible and even more interesting.

One more note: It's not compulsory to create classes that derive from StatelessComponent or PureComponent when you want to create presentation components - in your example code, you were using the "StaticFunctions" class to create stateless components from simple render methods. This is a valid approach (and there is a "PureComponent" function to do similar for pure components) but I think that it's easier to understand using a class-based approach initially - once you're happy with that, we can always come back to look at the StaticFunctions.PureComponent version.

After writing so much code with F#, my brain shifted a little. So when writing in C# I still tend to do things in terms of functions. Its not a "Functional is better" mentality, its just what I got used to write.

ProductiveRage commented 8 years ago

I think your abstraction for the container component (aka "Provider"..?) makes a lot of sense!

If you prefer the functional approach for generating components (which is understandable anyhow but especially if you're coming in with F# experience), then it's a simple change for Pure or Stateless components - instead of a component class such as:

public class Todo : PureComponent<TodoItem>
{
    public Todo(TodoItem props) : base(props) { }

    public override ReactElement Render()
    {
        Console.WriteLine("Todo[" + props.Id + "].Render");
        var attrs = new Attributes { Style = props.IsDone ? Styles.Done : Styles.NotDone };
        return DOM.Div(attrs, props.Text);
    }
}

.. which is rendered by creating a new instance..

return DOM.Li(attrs, new Todo(todo));

.. you would create a static class with a Render function -

public static class Todo
{
    [Name("Todo")]
    public static ReactElement Render(TodoItem props)
    {
        Console.WriteLine("Todo[" + props.Id + "].Render");
        var attrs = new Attributes { Style = props.IsDone ? Styles.Done : Styles.NotDone };
        return DOM.Div(attrs, props.Text);
    }
}

.. and then render it using the "StaticFunctions.Pure" method -

return DOM.Li(attrs, StaticComponent.Pure(Todo.Render, todo))

Actually, you could render the content by just calling the method directly and using the returned ReactElement. The "StaticComponent.Pure" has two benefits, though. Firstly, it automatically injects the "shouldComponentUpdate" implementation that can save React some work if the old and new props are the same. Secondly, it uses the value in the [Name] attribute to assign a display name to the component - this will be displayed if you use the React dev tools (a browser extension).

There is a (small) performance gain to defining components using "StaticComponent.Pure" instead of deriving from PureComponent - probably not enough to make much difference in real use but still nice to know if you prefer the functional approach already.

ProductiveRage commented 8 years ago

Just in case you've not used the React dev tool extension and don't know what I mean, this is what they look like (note the component names and the props references - can be helpful from time to time in working out why something's not doing what you expect). react dev tools

Zaid-Ajaj commented 8 years ago

I have installed React dev tools, but I still didn't get the (display) names right with [Name] :( Stateless components rendered with StaticComponent.Stateless have StatelessComponentas the display name. The pure ones, rendered with StaticComponent.Purehave display name No display name. Maybe its just a firefox thing (where I have the dev tools installed, chrome wasn't able to install React tools).

ProductiveRage commented 8 years ago

Can you include the code from your static render functions and the code that passes them through either "Pure" or "Stateless" calls?

Why wouldn't chrome install the extension, did it give a particular error?

Zaid-Ajaj commented 8 years ago

Ofcourse,

Renderer = props => StaticComponent.Stateless(Components.TodoList, new TodoListProps
{
    Todos = props,
    AddTodo = text => store.Dispatch(Actions.AddTodo(text)),
    ToggleTodo = id => store.Dispatch(Actions.ToggleTodo(id))
})

is translated to

setRenderer: function (props) {
                    return Bridge.React.StaticComponent.stateless(Object, Bridge.Redux.Examples.Components.TodoList, { todos: props, addTodo: function (text) {
                        store.dispatch(Bridge.Redux.Examples.Actions.addTodo(text));
                    }, toggleTodo: function (id) {
                        store.dispatch(Bridge.Redux.Examples.Actions.toggleTodo(id));
                    } });
                }

Using StaticComponent.Stateless above for TodoList, and StaticComponent.Pure for the following TodoItem

TodoItem: function (todo) {
    var attrs = { style: todo.isDone ? Bridge.Redux.Examples.Styles.done : Bridge.Redux.Examples.Styles.notDone };
    return React.DOM.div(attrs, todo.text);
},
TodoList: function (todoListProps) {
    return React.DOM.div({  }, React.DOM.input({ type: "text", placeholder: "Add todo", onKeyDown: function (e) {
        var textInput = e.currentTarget.value;
        if (e.which === 13 && textInput.length > 0) {
            todoListProps.addTodo(textInput);
            e.currentTarget.value = "";
        }
    } }), React.DOM.ul({  }, System.Linq.Enumerable.from(System.Linq.Enumerable.from(todoListProps.todos).select(function (todo) {
        var attrs = { key: todo.id, onClick: function (e) {
            todoListProps.toggleTodo(todo.id);
        } };

        return React.DOM.li(attrs, Bridge.React.StaticComponent.pure(Object, Bridge.Redux.Examples.Components.TodoItem, todo));
    })).toArray()));
}

The result: React dev tools

Chrome didn't give any errors, its just that nothing happened when I tried to install the tools. (I thought there should have been some react icon on toolbar but there weren't any) I haven't looked into it properly yet.

ProductiveRage commented 8 years ago

I think that I've had similar problems with Chrome - the new dev tools tab won't appear until you close it all down and start it again, sometimes.

It's weird that Firefox's tools behave differently - I've confirmed this on my computer; the names appear correctly in the React tools in Chrome but not in Firefox (see screenshot). I can't find any information about it through Google yet, though.

devtools

To get back to the original issue.. how do you feel about the Redux integration now? Are you happy to create a distinct NuGet package that adds the Redux bindings and helper classe (but that depends upon Bridge.React)? I don't know if you've created and published any NuGet packages in the past - if not, be assured that it's really easy! I can help out if you need any guidance. Are you happy for me to close this issue? (I'll raise a separate one about the Firefox Dev Tools).

Zaid-Ajaj commented 8 years ago

I think the end-solution is very sound and I like it a lot! Nothing is really needed on the React part of this feature so I will write a seperate library for this one. Thanks a lot for your help, really really appreciate it!

Oh yes, I have made some basic Nuget packages a while ago, a couple bindings libraries for Bridge :D nothing fancy haha so this won't be a problem.

As for this issue, we can close it since it is no longer an issue ;)