Move to Portable Libraries #217

Merged
merged 33 commits into from Apr 4, 2013

Conversation

Projects
None yet
4 participants
@paulcbetts
Owner

paulcbetts commented Mar 20, 2013

In ReactiveUI 5.0, support is being dropped for WP7.x and .NET 4.0, meaning that it makes more sense now to move to Portable Libraries.

Reorganize All The Things

Now that certain types have moved around in .NET 4.5 (most notably, ICommand is now in mscorlib), this becomes a more compelling proposition. Mainly:

  1. RxUI.Xaml now becomes more like RxUI.Cocoa or RxUI.Android - only providing platform-specific helpers.
  2. All of the commanding stuff moves into RxUI.Core
  3. ReactiveUI.Routing goes away, its stuff moves into RxUI.Core and RxUI.Xaml

TODO:

  • - Move ReactiveUI Core to PLib
  • - Reorganize everything
  • - Fix MakeRelease.ps1 to build Portable packages
@paulcbetts

This comment has been minimized.

Show comment Hide comment
@paulcbetts

paulcbetts Mar 20, 2013

Owner

@jlaanstra Can I rebase away these unrelated commits?

Owner

paulcbetts commented Mar 20, 2013

@jlaanstra Can I rebase away these unrelated commits?

@jlaanstra

This comment has been minimized.

Show comment Hide comment
@jlaanstra

jlaanstra Mar 20, 2013

Member

@xpaulbettsx you can certainly do that. Some commits are from changes I made myself to ReactiveUI. Although 353ae7d, dc29cfc and 2746e56 might be interesting to include.

Member

jlaanstra commented Mar 20, 2013

@xpaulbettsx you can certainly do that. Some commits are from changes I made myself to ReactiveUI. Although 353ae7d, dc29cfc and 2746e56 might be interesting to include.

@paulcbetts

This comment has been minimized.

Show comment Hide comment
@paulcbetts

paulcbetts Mar 21, 2013

Owner

@jlaanstra Agree, I'll cherry-pick them into a new PR

Owner

paulcbetts commented Mar 21, 2013

@jlaanstra Agree, I'll cherry-pick them into a new PR

jlaanstra added some commits Mar 19, 2013

Work in progress portable library.
ReactiveCommand and ReactiveAsyncCommand moved to RxUI-Core
Renamed ReactiveUI_Portable to ReactiveUI.
Added _Net40 to .Net 4.0 projects.
Moved Validation.cs into RxUI-Xaml.
Updated projects to reference the Portable Library.
Lazy in its own file.
Linked Command classes and Interfaces into RxUI-Xaml for .Net 4.0
Fixed compile errors.
Fixed InternalsVisibleTo.
Temporarily handle unit test detection by returning false.
Change the namespace for PropertyChanging classes to work around namespace collision on .Net45. Need to think about the best way to handle this.
@paulcbetts

This comment has been minimized.

Show comment Hide comment
@paulcbetts

paulcbetts Mar 21, 2013

Owner

Force-pushing away these commits, update your branch by running:

git checkout portable
git fetch
git reset origin/portable

For reference, the previous HEAD was 2823f2b

Owner

paulcbetts commented Mar 21, 2013

Force-pushing away these commits, update your branch by running:

git checkout portable
git fetch
git reset origin/portable

For reference, the previous HEAD was 2823f2b

@paulcbetts paulcbetts referenced this pull request Mar 21, 2013

Merged

Ship ReactiveUI 5.0 #219

4 of 8 tasks complete

jlaanstra added some commits Mar 21, 2013

Move RxUI-Routing into RxUI-Core and RxUI-Xaml.
Move Validation back to RxUI-Core.
Validation classes are not available on WP and therefore currently cause compile errors.
@jlaanstra

This comment has been minimized.

Show comment Hide comment
@jlaanstra

jlaanstra Mar 21, 2013

Member

Validation classes such as ValidationAttribute and ValidationContext are not available in Portable Class Libraries.
I would like to propose some sort of syntax instead, which looks like WhenAny:

this.ValidationFor(x => x.SomeProperty, value => value != null, "error message");

What do you think?

Member

jlaanstra commented Mar 21, 2013

Validation classes such as ValidationAttribute and ValidationContext are not available in Portable Class Libraries.
I would like to propose some sort of syntax instead, which looks like WhenAny:

this.ValidationFor(x => x.SomeProperty, value => value != null, "error message");

What do you think?

@wendazhou

This comment has been minimized.

Show comment Hide comment
@wendazhou

wendazhou Mar 21, 2013

Member

Validation is delicate in any case. The WPF Validation class is very much coupled with the binding system, and replacing it means replacing it wholesale (with creating our own adorners etc.). For example try googling "validation without binding", most solutions involve creating a fake binding or something similar. Yuck.

Maybe we could try an extensible validation framework? Portable and extensible side in the portable library, plug in from ReactiveUI.Xaml / ReactiveUI.Cocoa / ReactiveUI.Android

Also, I do think that validation should be with the binding, maybe an extension of the binding syntax. After all, it does modify what the binding needs to do significantly (we don't want to set the value of the binding if it fails validation), though I still quite like your syntax. We would have to intercept the property setting in this case I think.

Member

wendazhou commented Mar 21, 2013

Validation is delicate in any case. The WPF Validation class is very much coupled with the binding system, and replacing it means replacing it wholesale (with creating our own adorners etc.). For example try googling "validation without binding", most solutions involve creating a fake binding or something similar. Yuck.

Maybe we could try an extensible validation framework? Portable and extensible side in the portable library, plug in from ReactiveUI.Xaml / ReactiveUI.Cocoa / ReactiveUI.Android

Also, I do think that validation should be with the binding, maybe an extension of the binding syntax. After all, it does modify what the binding needs to do significantly (we don't want to set the value of the binding if it fails validation), though I still quite like your syntax. We would have to intercept the property setting in this case I think.

@wendazhou

This comment has been minimized.

Show comment Hide comment
@wendazhou

wendazhou Mar 21, 2013

Member

Also, just thought about that, you really want validation to be expressed on ViewModel side rather than view side actually. System.ComponentMode.DataAnnotations is not available on portable class library for WP, roll our own?

Member

wendazhou commented Mar 21, 2013

Also, just thought about that, you really want validation to be expressed on ViewModel side rather than view side actually. System.ComponentMode.DataAnnotations is not available on portable class library for WP, roll our own?

@wendazhou

This comment has been minimized.

Show comment Hide comment
@wendazhou

wendazhou Mar 21, 2013

Member

And I just noticed that I said something inexact

After all, it does modify what the binding needs to do significantly

This is not true in the current IDataErrorInfo/INotifyDataErrorInfo framework that we have, as we set the property anyways and notify the error. Personally I think it would be better if the binding system didn't set the property in the case of a validation failure, but just reported the error so as to preserve the ViewModel invariants.

Member

wendazhou commented Mar 21, 2013

And I just noticed that I said something inexact

After all, it does modify what the binding needs to do significantly

This is not true in the current IDataErrorInfo/INotifyDataErrorInfo framework that we have, as we set the property anyways and notify the error. Personally I think it would be better if the binding system didn't set the property in the case of a validation failure, but just reported the error so as to preserve the ViewModel invariants.

@paulcbetts

This comment has been minimized.

Show comment Hide comment
@paulcbetts

paulcbetts Mar 21, 2013

Owner

Maybe we could try an extensible validation framework? Portable and extensible side in the portable library, plug in from ReactiveUI.Xaml / ReactiveUI.Cocoa / ReactiveUI.Android

Now this is the kind of thinking I can get behind :)

Also, I do think that validation should be with the binding, maybe an extension of the binding syntax.

I'll have to think about how to do this elegantly - the binder is already super complicated as-is with type conversion and binding hooks.

Owner

paulcbetts commented Mar 21, 2013

Maybe we could try an extensible validation framework? Portable and extensible side in the portable library, plug in from ReactiveUI.Xaml / ReactiveUI.Cocoa / ReactiveUI.Android

Now this is the kind of thinking I can get behind :)

Also, I do think that validation should be with the binding, maybe an extension of the binding syntax.

I'll have to think about how to do this elegantly - the binder is already super complicated as-is with type conversion and binding hooks.

@jlaanstra

This comment has been minimized.

Show comment Hide comment
@jlaanstra

jlaanstra Mar 22, 2013

Member

The WPF Validation class seems to be pure UI based and not coupled to INotifyDataErrorInfo. Although it is of course the case that currently only Xaml Bindings work with INotifyDataErrorInfo.

Member

jlaanstra commented Mar 22, 2013

The WPF Validation class seems to be pure UI based and not coupled to INotifyDataErrorInfo. Although it is of course the case that currently only Xaml Bindings work with INotifyDataErrorInfo.

@paulcbetts

This comment has been minimized.

Show comment Hide comment
@paulcbetts

paulcbetts Mar 22, 2013

Owner

While I'd be perfectly happy to kill all validation with 🔥, other people are super excited about it. It probably belongs in the New™ ReactiveUI.Xaml.

Owner

paulcbetts commented Mar 22, 2013

While I'd be perfectly happy to kill all validation with 🔥, other people are super excited about it. It probably belongs in the New™ ReactiveUI.Xaml.

Validation framework rebuilt on top of INotifyDataErrorInfo.
TODO: Add hooks for ValidationAttribute on supported platforms.
TODO: Tests

@wendazhou wendazhou referenced this pull request Mar 27, 2013

Closed

Rxui5 validation #225

jlaanstra and others added some commits Mar 29, 2013

Removed backing field conventions.
Removed old WP7 stuff.
Support property changes in derived collections
Adds support for updating derived collections when items in the source
collection changes (INPC). Changes to source items can result in items
being added to or removed from the derived collection as well as items
changing places (due to new sort conditions).

In order to make this possible I've moved all of the synchronization logic
into the RDC class itself and the class has been split into two parts. The
RDC<T> class is exposed to callers of CreateDerivedCollection (for
backwards compatibility and sanity) and RDC<TSource,TValue> which contains
the magic ingredient TSource (the type of the source collection items).

In order to support all scenarios of value and reference types, orderers
and no orderers filters and what not RDC now maintains a map between the
item indices and their corresponding indices in the source collection
allowing it to track and map changes in the origin collection to their
projected counterparts regardless of type and value/reference type
changes.

Also supports items existing in multiple places in the source collection
whether or not the selector is an identity function or if they're mapped
to value types.

This is a breaking change for the following reasons:
  * RDC constructor no longer accepts an IDisposable
  * RDC is now explicitly read only (modifying a derived collection makes
    no sense and now it's enforced. All modifying methods throws and the
    IsReadOnly property is set to true.

TODO:
  * CreateCollection is currently broken. I don't think it will be hard to
    fix it but I want to get some feedback on this change first.
  * One possible performance optimization would be to maintain an inverse
    map of source indices to destination indices which would allow for
    quicker lookups. Needs to be measured and weighed against increase
    complexity. Could also enable binary search for reverse lookup.
  * Another possible perf gain is to use binary search for
    positionForNewItem.
Getting rid of awkward reset overload
ReactiveDerivedCollection needs to override the implementation of Reset
while still being able to use SuppressChangeNotification. My previous
hack to allow this needs to be killed with fire so I'm adding
publishResetNotification to ReactiveCollection. It's not ideal but at
least it's better than the previous version.
Make RDC base class readonly
Move all throwing read-only methods into RDC<T> and expose internal
read/write methods to implementing classes.
Re-enable CreateCollection
Moving logic into internal helper class based on
ReactiveDerivedCollection<T>.

niik and others added some commits Apr 1, 2013

paulcbetts pushed a commit that referenced this pull request Apr 4, 2013

Paul Betts
Merge pull request #217 from reactiveui/portable
Move to Portable Libraries

@paulcbetts paulcbetts merged commit d07b3fc into rxui5-master Apr 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment