angulardart / angular

Fast and productive web framework provided by Dart
https://pub.dev/packages/angular
MIT License
1.83k stars 232 forks source link

RoutePath.toUrl() returns a different URL as RouteDefintion.toUrl() #1314

Closed grundid closed 6 years ago

grundid commented 6 years ago

In the following example the first output is "/crises/2" and the second one is "/2". I would assume that RouteDefinition should create the same Url as RoutePath.

This way it is not possible to use the RouteDefinition in an routerLink attribute if the route has a parent.

final crises = new RoutePath(path: 'crises');

final crisis = new RoutePath(
  path: ':$idParam',
  parent: crises,
);

final _crisis = new RouteDefinition(
  routePath: crisis,
  component: cct.CrisisComponentNgFactory
);

showRoutes() {
  print(crisis.toUrl(parameters: {"id": "2"}));
  print(_crisis.toUrl({"id": "2"}));
}
leonsenft commented 6 years ago

The issue is that the same RouteDefinition could be used to define multiple routes throughout your app.

Consider the following a route hierarchy.

      root
     /   \
'/foo'   '/bar'
   |        |
'/baz'   '/baz'
final bazRouteDefinition = new RouteDefinition(
  routePath: bazRoutePath,
  component: ng.BazComponentNgFactory,
);

What then should bazRouteDefinition.toUrl() return? '/foo/baz' or '/bar/baz'? You could argue it should return whatever bazRoutePath defines as it's parent, but that may encourage creating multiple RouteDefinition instances instead of being able to reuse the same one. We don't want to couple a RouteDefinition with where it's used, since that defeats the purpose of it being reusable.

Instead I'd encourage creating multiple route path instances, and using them for their toUrl() method.

saibotma commented 6 years ago

The AngularDart Routing documentation is a bit misleading with RouteDefinition and RoutePath because it encourages to use RouteDefinition for [routerLink] in templates, but that wont work for child components. I am new to AngularDart and don´t know how i should actually handle routing after reading this and the documentation.

chalin commented 6 years ago

I've created https://github.com/dart-lang/site-webdev/issues/1595 as a reminder to take a look at the Router examples and docs (I'm in the middle of another sizeable change so I can't do it now). AFAIR, the docs follow the original angular_router 2 example, but I could be wrong. I'll take a look tomorrow. /cc @kwalrath

chalin commented 6 years ago

@leonsenft - ~in your example, what is the value of bazRoutePath?~ See below for followup questions.

chalin commented 6 years ago

Actually, you can ignore my previous question. Can you confirm that a RouteDefinition's path is actually just a "local" name for the definition as opposed to the full path?

In that sense, RouteDefinition.toUrl() constructs a string consisting of the "local" path + query parameters, where the local path is essentially what basename() would return if given the full path. Is that correct?

leonsenft commented 6 years ago

@chalin it's not equivalent to basename(), since a path can contain multiple path segments, whereas basename() would only return the last segment.

When constructing a RouteDefinition there are two ways to configure the path.

RouteDefinition(path:)

Nothing unexpected here. The path field will return a normalized copy of the path argument.

final routeDefinition = new RouteDefinition(path: '/foo/bar');
print(routeDefinition.path); // => /foo/bar

RouteDefinition(routePath:)

This is where the confusion occurs. The RouteDefinition.path field will be the same as the RoutePath.path field from the routePath argument. This doesn't include any part of the parent path.

final parentRoutePath = new RoutePath(path: '/foo/bar');
final childRoutePath = new RoutePath(path: '/baz/qux', parent: parentRoutePath);
final routeDefinition = new RouteDefinition(routePath: childRoutePath);
print(routeDefinition.path); // => /baz/qux

Only childRoutePath.toUrl() actually traverses the parent references to construct the URL. As you've remarked, routeDefinition.toUrl() only uses its own path to construct the URL.

Yet again it's an unfortunate design we're stuck with until we want to make a major breaking change, which we likely won't be doing for some time.

chalin commented 6 years ago

Thank you for the clear explanation!

chalin commented 6 years ago

The AngularDart Routing documentation is a bit misleading with RouteDefinition and RoutePath because it encourages to use RouteDefinition for [routerLink] in templates, but that wont work for child components.

Fixed. The ToH and Router docs have been updated to make use of RoutePath.toUrl() to initialize [routerLink] properties.