Work on React Router support, request for comments

[gunnim]

I'm preparing to begin work on the implementation and just wanted to verify that i'm headed in the right direction.

Firstly, to open the project in VStudio 2017 i'm forced to upgrade all the xproj projects to csproj, I'm gonna assume you're fine with that.

Regarding the implementation, my first thought is to extend ReactComponent, enabling code reuse of many of the protected functions and variables there.
Then create a HtmlHelper extension method, Html.ReactRouter with similar parameters and functionality to Html.React + the "magic" that enables rendering a react router component with context.

So for the structure, I'm wondering if it would make sense to create a new project and alter the build to create a new NuGet pkg?
Seeing as this is addon functionality that not all users will need.

[gunnim]

@Daniel15 , just wanted to make sure you don't miss this. Still waiting for your confirmation so that i can begin work on the feature.

[jslatts]

I would be interested in this. I currently use React Router, including for pages rendered on the server. I set it up by passing Request.RawURL as a prop (in a base controller) to all my components. If I understand you right, this would accomplish the same thing using the helpers?

[gunnim]

This offers more, allowing the server to return custom http status codes determined by react router. f.x. react router receives the url, determines page not found and wants to return 404. This aims to achieve that, similar to what's shown here:
https://reacttraining.com/react-router/web/guides/server-rendering

[Daniel15]

Sorry I took a while to reply @gunnim, sometimes I get busy with other things and I'm a contributor on a bunch of other open-source projects so my free time is spread fairly thin.

Firstly, to open the project in VStudio 2017 i'm forced to upgrade all the xproj projects to csproj, I'm gonna assume you're fine with that.

This is fine, we just need to ensure that the build still works as expected. build.proj might need some changes. You may need to switch AppVeyor to a newer image too.

Regarding the implementation, my first thought is to extend ReactComponent, enabling code reuse of many of the protected functions and variables there.
Then create a HtmlHelper extension method, Html.ReactRouter with similar parameters and functionality to Html.React + the "magic" that enables rendering a react router component with context.

This seems reasonable.

So for the structure, I'm wondering if it would make sense to create a new project and alter the build to create a new NuGet pkg?

Yeah, perhaps something like a React.Router NuGet package.

[gunnim]

Of course and thanks.
Regarding the implementation, CreateComponent in ReactEnvironment instantiates the ReactComponent class, providing it with the environment configuration and then adding it to the list of components to render client side.
We however need it to create a ReactRouterComponent class..

I'm not particularly confident in my proposed solution but here goes:
My initial thought is to add a new method to ReactEnvironment, CreateCustomComponent which would in addition to the params in CreateComponent accept an IReactComponent object.

That I assume would mean we'd have to instantiate ReactRouterComponent with the environment object and configuration from the publicly accessible ReactEnvironment.Current and ReactSiteConfiguration.Configuration respectively.
It would also mean the instantiation has to happen elsewhere, in the HTMLHelper extension method or a method called therein.

Feels like some refactoring might be a better solution though?

[Daniel15]

By the way, I'm trying to convert from xproj to csproj today, and I'm hitting a bunch of issues. It won't convert the unit test project for some reason (might need to do that manually), and VS2017 keeps crashing when I try to load React.Core :/ I might just upgrade everything manually.

My initial thought is to add a new method to ReactEnvironment, CreateCustomComponent which would in addition to the params in CreateComponent accept an IReactComponent object.

This sounds reasonable. Maybe there could be two overrides for CreateComponent - One that takes a string (like the current one), and one that takes the IReactComponent itself:

public virtual IReactComponent CreateComponent<T>(string componentName, T props, string containerId = null, bool clientOnly = false)
public virtual IReactComponent CreateComponent<T>(IReactComponent component, T props, string containerId = null, bool clientOnly = false)

CreateRouterComponent could then be an extension method that calls CreateComponent(new RouterComponent(.....), .....)

Either that or we could embed ReactRouter support directly in React.Core, if it's too difficult to abstract it out into a separate package.

That I assume would mean we'd have to instantiate ReactRouterComponent with the environment object and configuration from the publicly accessible ReactEnvironment.Current and ReactSiteConfiguration.Configuration respectively.

You could make CreateRouterComponent as an extension method on the IReactEnvironment

[Daniel15]

Current status of switching from xproj to csproj: (╯°□°）╯︵ ┻━┻)

[gunnim]

I'm not sure i follow, a static createRouterComponent extension method on ReactEnvironment can't access the _config protected instance variable nor a ReactEnvironment instance without the aforementioned public static variables I assume?

and yeah i gave up on the xproj migration process after a while... some people who were having migration problems suggested creating a new solution and moving the code and project to this fresh base and go from there?

[Daniel15]

a static createRouterComponent extension method on ReactEnvironment can't access the _config protected instance variable

Hmmmm... yeah. You could get it directly out of the dependency injection container if you need to. It's not ideal, but it'll do.

nor a ReactEnvironment instance

An extension method would have the ReactEnvironment instance - It'd be the first argument (this IReactEnvironment environment)

some people who were having migration problems suggested creating a new solution and moving the code and project to this fresh base and go from there?

I already did that once for going to xproj and it was pretty painful 😛
41963d6

I'll play around with it for a bit and see if I can get the migration working. Otherwise, you can use VS2015 community edition. I still have both VS2015 and VS2017 installed.

[Daniel15]

React.AssemblyRegistration.Container.Resolve<IReactSiteConfiguration>() will get you the config.

[gunnim]

An extension method would have the ReactEnvironment instance - It'd be the first argument (this IReactEnvironment environment)

Doh! 😛

config solution sounds fine and thankfully i have VS2015 installed so good luck with the migration and I'll be in touch!

[Daniel15]

I've got a work-in-progress pull request to upgrade to VS2017 + the newer tooling: #406. It's not 100% complete, but seems to compile at least.

[Daniel15]

Code has been updated to use VS2017 + csproj: 9bc1388

[gunnim]

Pull request submitted