Skip to content

Commit

Permalink
Remove Routing with a Fully Qualified Type name (#5045)
Browse files Browse the repository at this point in the history
This behavior is not trim-compatible, since it just a random string "route name" and calls Type.GetType with it, and then Activator.CreateInstance. The trimmer might have removed the whole type, or even the constructor on the Type. This would cause a trimmed application to behave differently than an untrimmed application.

I do not know of anyone who relies on this behavior, and there are no tests for it. So removing it seems like the best path forward.

Contributes to #1962
  • Loading branch information
eerhardt committed Mar 4, 2022
1 parent 8f39d59 commit bf078f5
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 29 deletions.
22 changes: 3 additions & 19 deletions src/Controls/src/Core/Routing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,23 +158,6 @@ public static Element GetOrCreateContent(string route, IServiceProvider services
if (s_routes.TryGetValue(route, out var content))
result = content.GetOrCreate(services);

if (result == null)
{
// okay maybe its a type, we'll try that just to be nice to the user
var type = Type.GetType(route);
if (type != null)
{
if (services != null)
{
result = (services.GetService(type) ?? Activator.CreateInstance(type)) as Element;
}
else
{
result = Activator.CreateInstance(type) as Element;
}
}
}

if (result != null)
SetRoute(result, route);

Expand Down Expand Up @@ -284,9 +267,10 @@ public override Element GetOrCreate(IServiceProvider services)
{
if (services != null)
{
return (services.GetService(_type) ?? Activator.CreateInstance(_type)) as Element;
return (Element)(services.GetService(_type) ?? Activator.CreateInstance(_type));
}
return Activator.CreateInstance(_type) as Element;

return (Element)Activator.CreateInstance(_type);
}

public override bool Equals(object obj)
Expand Down
20 changes: 10 additions & 10 deletions src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -623,11 +623,11 @@ public async Task RouteWithGlobalPageRoute()
Assert.AreEqual("//animals/domestic/cats/catdetails", shell.CurrentState.Location.ToString());
}

[TestCase(typeof(PageWithDependency), typeof(PageWithDependency))]
[TestCase(typeof(PageWithDependencyAndMultipleConstructors), typeof(PageWithDependencyAndMultipleConstructors))]
[TestCase(typeof(PageWithDependency), typeof(Dependency))]
[TestCase(typeof(PageWithUnregisteredDependencyAndParameterlessConstructor), typeof(PageWithUnregisteredDependencyAndParameterlessConstructor))]
public async Task GlobalRouteWithDependencyResolution(Type typeForRouteName, Type type)
[TestCase(typeof(PageWithDependency))]
[TestCase(typeof(PageWithDependencyAndMultipleConstructors))]
[TestCase(typeof(PageWithDependency))]
[TestCase(typeof(PageWithUnregisteredDependencyAndParameterlessConstructor))]
public async Task GlobalRouteWithDependencyResolution(Type pageType)
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<Dependency>();
Expand All @@ -646,29 +646,29 @@ public async Task GlobalRouteWithDependencyResolution(Type typeForRouteName, Typ
Items = { flyoutItem }
};
shell.Parent.Handler = fakeHandler;
var routeName = typeForRouteName.AssemblyQualifiedName;
Routing.RegisterRoute(routeName, type);
var routeName = pageType.Name;
Routing.RegisterRoute(routeName, pageType);
await shell.GoToAsync(routeName);

Assert.IsNotNull(shell.Navigation);
Assert.IsNotNull(shell.Navigation.NavigationStack);
var page = shell.Navigation.NavigationStack[1];
Assert.That(page, Is.Not.Null);
if (type == typeof(PageWithDependency) || type == typeof(Dependency))
if (pageType == typeof(PageWithDependency) || pageType == typeof(Dependency))
{
Assert.IsInstanceOf<PageWithDependency>(page);
Assert.That((page as PageWithDependency).TestDependency, Is.Not.Null);
}

if (type == typeof(PageWithDependencyAndMultipleConstructors))
if (pageType == typeof(PageWithDependencyAndMultipleConstructors))
{
Assert.IsInstanceOf<PageWithDependencyAndMultipleConstructors>(page);
var testPage = page as PageWithDependencyAndMultipleConstructors;
Assert.That(testPage.TestDependency, Is.Not.Null);
Assert.That(testPage.OtherTestDependency, Is.Null);
}

if (type == typeof(PageWithUnregisteredDependencyAndParameterlessConstructor))
if (pageType == typeof(PageWithUnregisteredDependencyAndParameterlessConstructor))
{
Assert.IsInstanceOf<PageWithUnregisteredDependencyAndParameterlessConstructor>(page);
}
Expand Down

0 comments on commit bf078f5

Please sign in to comment.