Two things I'm unsure about.

1. Currently this is only implementing on 'Route' and not, 'AsyncRoute' or 'Redirect'. Please let me know if we want to implement on either / both of those and I will update the code and tests.
2. Not sure on the status or workflow with Dart. This commit does not change any Dart code. Happy to update that as well.

Thanks @danielrasmuson!

Can you rebase this on master? I just landed a refactor to the router that should make this a bit easier.

I think this should be attached to the handler, and that'll make it agnostic for Route and AsyncRoute. I don't think we want it for Redirect.

If you get the tests passing for JS, I'll help make the Dart ones pass. No need to worry about that.

Sure thing Brian. I'll work on it.

Hey @btford
I rebased the commit on master and attached the data to the handler. The tests are passing for JS, but are still failing for Dart. :(
Happy to change / add anything.

@danielrasmuson– thanks! taking a look now. If you don't mind, I'll just amend your commit with the few fixes for Dart

This landed in master.

I made some changes – I removed the arbitrary type restriction, and extraneous class in favor of an opaque token for injection.

See ed81cb9 for details.

Thanks, @danielrasmuson!

@btford If I read this correctly, the only way to access the data is to use @Inject(ROUTE_DATA). Is there a particular reason for not having a RouteData service? That would be more coherent with the rest (and easier to understand) don't you think?

@cexbrayat I think the reason for that is, that ROUTE_DATA is really just a simple value. Introducing yet another service introduces also additional complexity (that is not necessarily needed), no?

@PascalPrecht Right. But don't you think that having to use @Inject is a bit surprising compared to RouteParams which gives us a service? I think this is a very rare case (the only one?) where we can't rely on Type annotation in an app for something provided by the framework.
And we obtain an object as you pointed out, whereas RouteParams provides a get method.

These differences are a bit surprising, I thought it was worth pointing out.

I have to agree with @cexbrayat . @Inject(ROUTE_DATA) feels out of sync with the rest of the framework.

@TommyM I get @cexbrayat point that it doesn't feel common that ROUTE_DATA isn't a service compared to RouteParams. However, using @Inject() is a very common pattern, if you have to inject a bound value. Simply because it doesn't have a Type.

Maybe @btford can shed some light on the reasoning behind it.

@PascalPrecht Yes you are right. I did mean to say that ROUTE_DATA being a bound value instead of a service feels out of sync, not necessarily the use of @Inject()

@PascalPrecht @TommyM I think we all agree. @Inject is useful, but not very common to inject something provided by the framework, because it's usually a service and not a value. It's furthermore surprising when you use RouteParams and ROUTE_DATA side by side.

Yes, let's lean back and wait for @btford 's thoughts on this. I'm sure he'll shed some light into darkness once he can find some time.

Using a @Inject means you can specify a value of any type for data.

@cexbrayat @TommyM @PascalPrecht I'm open to discussing this more in another issue.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}