Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ship ReactiveUI 5.0 #219

Merged
merged 427 commits into from
Jul 2, 2013
Merged

Ship ReactiveUI 5.0 #219

merged 427 commits into from
Jul 2, 2013

Conversation

anaisbetts
Copy link
Member

This branch is the Master branch for tracking all of the stuff that will eventually become ReactiveUI 5.0

What MUST HAPPEN to Ship

What WOULD BE AWESOME to Ship

  • Readonly reactive collections (tracked in Add ReadOnly versions of the reactive collection classes. #267)
  • Fix the XYZPlayground samples, people read them
  • Finish the PowerShell integration with RxUIViewModelGenerator
  • Move Xamarin.* to new unified profile that they're going to release soon (Dubious if they'll actually release them soon)

Notes

  • Preferably, don't commit to this branch directly unless it's something small, do it via a PR
  • Maintenance / Cleanup is definitely more important than New Features right now, since none of the uncompleted New Features require breaking changes. We can always ship them later. Maintenance will involve breaking changes.
  • Don't merge this to master yet until ReactiveUI 5.0 ships

@Martin-Andersen
Copy link

Integration with FluentValidation would be very nice. Catel has Catel.Extensions.FluentValidation maybe we can get some inspiration from them.

@Martin-Andersen
Copy link

A reference application that is not to simple to show what's possible with RxUI. It's very importing to always keep this application up to date with new features and best practiced patterns

@wendazhou wendazhou mentioned this pull request Mar 27, 2013
@jlaanstra
Copy link
Member

@xpaulbettsx how about using the CommonServiceLocator from the patterns and practices team For ReactiveUI and make people specify there own DI container?

@shiftkey
Copy link
Contributor

@jlaanstra for client applications I'd argue that you can get away with something simple (you don't need to resolve components insanely quickly like you would with a web application serving a non-trivial amount of requests).

There's things that I love from Autofac (readable registration code and being able to use ctor parameters like Func<T>, IEnumerable<T>, Lazy<T> etc without registering components explicitly) that might be able to be added ontop of Paul's proposal.

And with RxUI looking to move to PCL for much of it's core, it may constrain what containers people can use until upstream projects choose to support PCL. And would Mono* devs be able to use existing containers too on their projects?

@anaisbetts
Copy link
Member Author

how about using the CommonServiceLocator from the patterns and practices team For ReactiveUI and make people specify there own DI container?

The current design is CSL, in Delegate Form. It was a terrible idea to copy, many containers don't allow Register after initial setup, others don't support multiple registration per type - the design of CSL is just wrong for ReactiveUI.

@haacked
Copy link
Contributor

haacked commented Mar 29, 2013

Correction. The design of CSL is just wrong.

:trollface:

@anaisbetts
Copy link
Member Author

@haacked Well, I was trying to be nice, but yeah :)

@jlaanstra
Copy link
Member

I'm neutral about CSL. The thing is that it's already used a lot. How about including a DI container with RxUI in a similar fashion as Mvvmlight. We can pick an existing one or create our own. I'm just thinking of the best way to do this in a non-terrible way, without losing the flexibility to use your own DI container if you want to.

We could make DI an implementation detail and not expose it at all, but that means losing extensibility right?

@deavon
Copy link

deavon commented Mar 30, 2013

Paul,

I'd love to help out with ReactiveUI 5.0. Is there anything you'd like me to focus on?

Here's my work history:
http://www.linkedin.com/in/deavon

Thanks,
D

@wendazhou
Copy link
Member

I think that in terms of IOC, there are two distinct issues we need to solve.

On one hand, ReactiveUI internally needs a DI container to wire all the extensibility mechanisms up, and personally, I feel that MEF would be a perfect fit in this case. It exists in a portable library, and has a simple attributed model that makes everything very simple. I don't think there is any need that this be replaceable as a DI container, and I think that it is acceptable to force the application to use a given DI framework to extend the framework.

On the other hand, we need a way to plug into the existing DI container used by the application, for stuff like ResolveViewFor and other places where we need to instantiate application code in the framework. I think that, in this case, a common facade remains the best way so that people can choose their own DI container. However terrible the design of CSL might be, it would limit its scope to just the interface between RxUI and the client code.

@anaisbetts
Copy link
Member Author

I think that in terms of IOC, there are two distinct issues we need to solve.

@wendazhou This is a great point, MEF might certainly be worth looking into as a hard dependency. Though, to be honest, I think that the Gist that I linked above serves all of the purposes of MEF without the configuration ceremony. You can do:

  • Already instantiated singletons: locator.Register(typeof(IFoo), () => myFoo);
  • Lazy instantiations: locator.Register(typeof(IFoo), () => someLazy.Value);
  • Create on-demand: locator.Register(typeof(IFoo), () => new Foo())

You could add Extension methods to do all of these things easily, and the code is straightforward and doesn't involve scanning every type of an assembly like MEF does. On mobile platforms, that scan will be killer for startup performance and memory consumption (on a big'ish desktop app that I once worked on, MEF used ~10MB on startup, right off the bat)

@anaisbetts
Copy link
Member Author

I'm just thinking of the best way to do this in a non-terrible way, without losing the flexibility to use your own DI container if you want to.

@jlaanstra You kind of already have one in RxUI 4 (i.e. just call Register and GetService, they both work even if you never set anything up), but I agree - you should never have to set up a container

@anaisbetts
Copy link
Member Author

@deavon You don't need to be approved or added to anything before you can help out, no resumé necessary! If you wanted something easy to hack on, there's definitely some janitor-type things that can be done. Specifically, the usage of Code Contracts is pretty lax, and it'd be great if that was filled out for every method.

However, don't feel constrained that you have to work on that, if you have an idea, create a pull request! If you have trouble getting started with Git or GitHub, feel free to send an Email, paul@paulbetts.org.

@deavon
Copy link

deavon commented Mar 30, 2013

Thanks, Paul! Will do. I don't know if you've met Kyle Neath, but I worked with him at LEVEL Studios a few years ago.

@DamianReeves
Copy link

I got my wires crossed before. The EF6 model of dependency resolution uses chain of responsibility and seems like an interesting why to solve the question on how a framework can allow integration with DI containers and also manage the need of the framework to create its own objects and services.

I also happen to be a fan of MEF too, but it seems like the average developer has the impression that MEF is too hard to learn.

@wendazhou
Copy link
Member

@DamianReeves MEF has quite a lot of technical parts, but RxUI can set that all up itself, and then all the user has to do is add a Export(ITheContractInterface)] to their implementation. We could even provide special attribute classes for all the extension points that automatically export the correct type (and eventually metadata if needed).

Personally, I dislike the service locator pattern, because it makes code harder to test (you have to mock the Service Locator, providing the correct implementations etc.), especially compared to something like constructor injection that makes dependencies very obvious. However, it does seem to be the simplest model to abstract away, and this does have some advantages for a framework.

@jlaanstra
Copy link
Member

I did some work on RxApp in 8b94898 by simplifying unit test and design mode detection. Still a lot of work to do.

@anaisbetts
Copy link
Member Author

@jlaanstra That looks good, but if you have a hack at redoing it further, please start a PR first (even a dummy one like the CSS Selectors PR) so that we can discuss it first, that'll be a pretty big change

@jlaanstra
Copy link
Member

@xpaulbettsx sure! The only thing I need to do for portable is adapt MakeRelease.ps1 to build portable libraries and than all tasks for portable are done. I'll make a new PR for RxApp changes.

@johnnyelwailer
Copy link

Any thoughts on blendability especially with the new .Bind syntax? There should be done a lot to enable the tooling with design time data (maybe even something like a reactive design time data generator :)).

@DamianReeves
Copy link

@wendazhou it turns out that MEF has limited portable library support. Try adding System.ComponentModel.Composition and you will quickly find out that you can't include this and target Windows Phone 8 and Windows Store Apps.

I've implemented the type of dependency resolution I spoke of before in a project I hacked at this week. I'm going to give a go at implementing something similar in my reactiveui fork.

How do I go about updating my fork to grab the latest from here (haven't dealt with git in the fork and pull request workflow that much)?

@anaisbetts
Copy link
Member Author

@DamianReeves

git remote add upstream https://github.com/reactiveui/ReactiveUI.git
git fetch upstream
git checkout rxui5-master
git merge upstream/rxui5-master

@wendazhou
Copy link
Member

@DamianReeves you have to use the Microsoft.Composition Nuget package for WinRT, but it is true that it has no WP8 support, so I guess it wouldn't work.

@anaisbetts
Copy link
Member Author

Any thoughts on blendability especially with the new .Bind syntax? There should be done a lot to enable the tooling with design time data (maybe even something like a reactive design time data generator :)).

@johnnyelwailer RxUI5 bindings are actually Blendable in a way that's so simple you'll kick yourself, it's so much better than the Microsoft way, in almost every way. When RxUI5 releases, I'll blog about it.

@DamianReeves
Copy link

So since I'm stuck on .NET 4.0 at work, does this me I can't use 5.x alpha? Whats the plan for those of us who can't upgrade to .NET 4.5 yet?

@anaisbetts
Copy link
Member Author

Whats the plan for those of us who can't upgrade to .NET 4.5 yet?

"There was that law of life, so cruel and so just, that one must grow or else pay more for remaining the same." - Norman Mailer

@DamianReeves
Copy link

It would be great to at least get the RxApp refactorings for ReactiveUI "4.x".
I find the service location setup in 4.5 to be not great when you use a container that doesn't like registering stuff after the container is created (I have a working solution for this, but its no where as elegant as what @jlaanstra added for 5.x).
So yes I'm having feature envy.

@anaisbetts
Copy link
Member Author

It would be great to at least get the RxApp refactorings for ReactiveUI "4.x".

That'd be a breaking change for the 4.x folks, that's sadly not possible.

anaisbetts and others added 23 commits June 19, 2013 17:40
Conflicts:
	ReactiveUI/ReactiveList.cs
Adds protected virtual void methods to raise events and comply with .NET best practices around events through all of ReactiveUI.
If the developer doesn't want to use orientation-specifc views, try to
resolve the non-orientation specific view
Fall back to non orientation-specific views
Have CreateDerivedCollection return interface for maximum
flexibility going forward in RxUI5.

Makes IObservedChange and IReactiveNotifyPropertyChanged
covariant.
anaisbetts pushed a commit that referenced this pull request Jul 2, 2013
@anaisbetts anaisbetts merged commit a475965 into master Jul 2, 2013
@lock lock bot locked and limited conversation to collaborators Jun 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.