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

Exception for WinRT targets #563

Closed
jcmm33 opened this Issue Apr 24, 2014 · 11 comments

Comments

Projects
None yet
3 participants
@jcmm33
Copy link

jcmm33 commented Apr 24, 2014

Trying latest build 5.5.1 I've been getting exceptions for WinRT targets.

Using the source from github (master branch) and trying the sample app MobileSample-WinRT you also get this exception :

{"Value cannot be null.\r\nParameter name: type"}

The call stack in VS.NET 2013 shows the line raising the exception is

Line 97:  current = current.BaseType.GetTypeInfo(); 

in DependencyObjectObservableForProperty.cs

In this case the parameter values are TypeInfo =RoutedViewHost , propertyName="RouterProperty"

I suspect there needs to be a check as to whether current.BaseType is not null before calling GetTypeInfo (current has reached Object in this case).

Similarly line 110 in same file has the same behaviour.

Changes, I've made locally to make it so that the sample works

        PropertyInfo actuallyGetProperty(TypeInfo typeInfo, string propertyName)
        {
            var current = typeInfo;
            while (current != null) {
                var ret = typeInfo.GetDeclaredProperty(propertyName);
                if (ret != null && ret.IsStatic()) return ret;

                // no base type, break we didn't find the property
                if (current.BaseType == null) {
                    break;
                }
                current = current.BaseType.GetTypeInfo();
            }

            return null;
        }

        FieldInfo actuallyGetField(TypeInfo typeInfo, string propertyName)
        {
            var current = typeInfo;
            while (current != null) {
                var ret = typeInfo.GetDeclaredField(propertyName);
                if (ret != null && ret.IsStatic) return ret;

                // no base type, break we didn't find the property
                if (current.BaseType == null) {
                   break;
                }
                current = current.BaseType.GetTypeInfo();
            }

            return null;
        }
@paulcbetts

This comment has been minimized.

Copy link
Member

paulcbetts commented Apr 27, 2014

Using the source from github (master branch) and trying the sample app MobileSample-WinRT you also get this exception

Despite its name, "MobileSample" isn't actually a sample, it's just a scratch pad WinRT app - its contents are entirely arbitrary, don't rely on it to do anything :)

@paulcbetts

This comment has been minimized.

Copy link
Member

paulcbetts commented Apr 27, 2014

This code hasn't changed in awhile, I'm surprised that it's broken. Can you send a super-simple repro app?

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented Apr 28, 2014

I've forked from rxui6master branch and added a sample app for a universal windows & phone 8.1 app. The repo can be found here https://github.com/jcmm33/ReactiveUI/ . Issues vary depending upon which application type you launch.

With the Windows Store app, you get issues (once previously mentioned changes about checking BackType for null and deprecated method call has been changed) with

RxApp.MainThreadScheduler = new WaitForDispatcherScheduler(() => CoreDispatcherScheduler.Current);

at line 89 in Registrations.cs

This gives a null argument exception for the value of current.

When using the phone sample app, you get an exception whenever it hits the call to

if (ModeDetector.InUnitTestRunner()) 

in RxApp.cs.

This gives a 1st chance argument exception with additional information of 'Use of undefined keyword value 1 for event TaskScheduled.'

Apologies in advance if I've been doing something stupid.

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented May 1, 2014

I've looked at this some more, and I can get the windows 8.1 app to work (after including a new version of splat targeted for wpa 8.1), but the windows phone app , which references the same assemblies and source code seems to have random crashes.

@paulcbetts

This comment has been minimized.

Copy link
Member

paulcbetts commented May 1, 2014

@jcmm33 Can you send me your app?

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented May 1, 2014

its here https://github.com/jcmm33/ReactiveUI/tree/rxui6-master/Sample_UniversalApp - been fiddling with the references etc but you'll see that the source is 'somewhat trivial'.

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented May 2, 2014

I suspect have found what the issue is and it can be demonstrated with a simple application that doesn't involve reactiveui at all.

Specifically, if one creates a new universal app, a collection of code is generated for you in the app.xaml.cs file. One of these lines is a 'navigation' to the main page. If one replaces this with a rootFrame.Content = new MainPage() (which in effect is what setupDefaultSuspendResume does when _viewModelChanged) then one experiences the null exception

As to what the solution is....

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented May 2, 2014

So, from my minimal understanding of how reactiveUI works, one option would be to change the shape of the call to Locator.Current.GetService("InitialPage") to instead return a 'type' instead of an instance for the initialPage or, keep it as it is currently and get the type from the page variable e.g. something like

            _viewModelChanged.Subscribe(vm => {
                var page = default(IViewFor);

                var frame = Window.Current.Content as Frame;

                if (frame == null) {
                    frame = new Frame();
                    Window.Current.Content = frame;
                }

                page = Window.Current.Content as IViewFor;
                if (page == null) {
                    page = Locator.Current.GetService<IViewFor>("InitialPage");

                    // page is only used to get the type, Navigated event provides us with actual instance
                    // so assign viewmodel at this point
                    frame.Navigated += (sender, args) => ((IViewFor) args.Content).ViewModel = args.Parameter;
                    frame.Navigate(page.GetType(),vm);
                }

                Window.Current.Activate();
            });

As I said, my knowledge of ReactiveUI is quite minimal, so I don't know whether this 'breaks' other things like suspend, resume.

@jlaanstra

This comment has been minimized.

Copy link
Member

jlaanstra commented May 13, 2014

PR #589 is removing the automatic setup of routing for WinRT.

@jcmm33

This comment has been minimized.

Copy link

jcmm33 commented May 13, 2014

So are we similarly saying that routing is also removed for WP 8.1 universal apps then?

@jlaanstra

This comment has been minimized.

Copy link
Member

jlaanstra commented May 13, 2014

Routing will most likely still be available for WinRT, but we have to figure out a way to make it work. I should have made it clear that PR #589 only removes the automatic setup.

paulcbetts pushed a commit that referenced this issue May 25, 2014

Paul Betts
Merge pull request #619 from recumbent/acctuallyGet-actually-throws-o…
…n-WinRT

Actually get actually throws on WinRT should fix #563

@paulcbetts paulcbetts closed this May 25, 2014

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