ProductiveRage / Bridge.React

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

Wrong Render method appears to be called when two components share a props class #11

Closed DanTup closed 8 years ago

DanTup commented 8 years ago

I have two components which I (accidentally) had sharing the same props class (which is empty):

public class Content : PureComponent<Content.Props>
{
    public Content() : base(null) { }
    public override ReactElement Render() { return DOM.Div("Content"); }
    public class Props { }
}

public class Navigation : PureComponent<Content.Props> // ERROR IN GENERIC TYPE HERE
{
    public Navigation() : base(null) { }
    public override ReactElement Render() { return DOM.Div("Navigation"); }
    public class Props { }
}

When I render these components:

React.Render(
    DOM.Div(
        null,
        DOM.Div(new Attributes { Id = "page-nav" }, new Navigation()),
        DOM.Div(new Attributes { Id = "page-content" }, new Content())
    ),
    Document.Body
);

I unexpectedly get Content written out twice.

ProductiveRage commented 8 years ago

I have reproduced this and tested with the latest version of the bindings (1.8.5, which doesn't include anything specifically to address this issue, I just thought that it was worth a try).

I'll have to have a poke around and try to find out what's going on.

Incidentally, React recommends against rendering direct to the document body, the production version of React (which you should be using until you actually release) shows the following warning:

Warning: render(): Rendering components directly into document.body is discouraged, since its children are often manipulated by third-party scripts and browser extensions. This may lead to subtle reconciliation issues. Try rendering into a container element created for your app.warning

DanTup commented 8 years ago

React recommends against rendering direct to the document body

Yeah, I actually noticed that warning right after posting and changed this! I was sure there was a reason but couldn't remember why; now it's all clear!

ProductiveRage commented 8 years ago

It turns out that this down to a misunderstanding that I had in relation to when static fields are initialised on generic types. I think I know how to fix it, it shouldn't be too big of a deal.

ProductiveRage commented 8 years ago

I've pushed a new version of the library to NuGet (1.8.6) that includes this fix.