search

Dresel / RouteLocalization

RouteLocalization is an MVC and Web API package that allows localization of your attribute routes.
MIT License
67 stars 13 forks source link

Some notes from the MVC team #62

Open rynowak opened 7 years ago

rynowak commented 7 years ago

Hi @Dresel - I promised I'd take a look at this and give my feedback and finally go to it. Sorry for the delay, it's been a madhouse trying to lock down 2.0.0 preview1.

I don't have much feedback because I think that all of this is pretty sensibly done.

At a high level, we want to eventually expose a fluent API (like your RouteTranslationBuilder) for application model. This isn't a high priority for us in 2.0.0 (it hasn't been asked for by many yet) so we're looking at 2.1.0 at the earliest. This might get you out of some boilerplate when/if we do it, but for now I think what you're doing here is great.

Consider taking this a step further and setting the current culture when you match a localized route. Our localization system does this, and use it as well, so this will play with all of the culture-sensitive features we have. The optimal way for you to do this would be to attach an IAsyncResourceFilter to the action model 👍

If you end up building any additional features that run after the routing stage, don't be afraid to use .Properties on the model classes to store data. We will copy this data to the action descriptor, and so it's cached forever without you having to maintain a store of it - and you have a way to retrieve all of the data (IActionDescriptorCollectionProvider) and a way to retrieve the data associated with the current request.

Another really small improvement we made in 2.0.0 is that the NullLogger and friends are provided in logging abstractions. These were copied frequently enough that decided to put them front and center.

Dresel commented 7 years ago

Much thanks for the feedback.

Great, didn't know 2.0 is already in the pipeline. Looking forward to any additional application model functionality. Once you are used to it, it's a great way for building extensions.

The resource filter is a good idea, that's definitely something for the next release.

I saw that, that's where I copied them from :)

Good to hear I'm not completely on the wrong track, will now work on finalizing (documentation, integration tests, ...) and integrating any additional feedback.

Dresel commented 6 years ago

At a high level, we want to eventually expose a fluent API (like your RouteTranslationBuilder) for application model. This isn't a high priority for us in 2.0.0 (it hasn't been asked for by many yet) so we're looking at 2.1.0 at the earliest. This might get you out of some boilerplate when/if we do it, but for now I think what you're doing here is great.

@rynowak Is there something there now that could be used for a future version of RouteLocalization?

rynowak commented 6 years ago

What did you have in mind? MVC's Application Model is a little complicated to use so we haven't built higher level APIs on top of it.

We're looking at changing the model a bit more in 3.0. You'd probably be interested in this: https://youtu.be/8k0OKeGsWTg?t=54m54s

Depending on your plans, that might help or not.

/cc @jamesnk @davidfowl

Dresel commented 6 years ago

Thanks for your answer. Just wanted to make sure I'm not missing something important.

Seems interesting, so instead of using the Application Model I might be able to work with the endpoints directly.

Dresel commented 6 years ago

Branch was moved to the RouteLocalization.AspNetCore repository. Your feedback was incorporated.

Let's wait and see what 3.0 will bring - it's a pity I have to spend most of my time with Xamarin now :)