-
Notifications
You must be signed in to change notification settings - Fork 21
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
Support attributes on lambda expressions #984
Comments
So near and yet so far. A lot of object-oriented programming styles are so averse to using objects. [<HttpGet("/")>]
[<Id(0)>]
[<Name<"Play more!">>]
[<IsComplete(false)>]
let todo = object() Then everything is Overall MapAction seems uninteresting to F# because it's intrinsically annotation-based and has weird JSON dependencies which imply that everything is implicit not explicit. F# should not support this sort of thing. Intstead there should be an action creator with a genuine type signature, such that when given objects of the right types the code will work. |
We can use this to support static lambdas, if we decide to do it. |
We're planning to make the route pattern an explicit parameter again rather than take it from an attribute which should hopefully reduce "call site complication". dotnet/aspnetcore#30448 With this change, the F# example would look more like the following: type Todo = { Id: int; Name: string; IsComplete: bool }
app.MapPost("/", fun ([<FromBody>] todo) -> todo) |> ignore
app.MapGet("/", fun () -> { Id = 0; Name = "Play more!"; IsComplete = false }) |> ignore |
@halter73 An API that respects the .Net type system would avoid nongeneric classes like MapPost(string pattern, System.Func<System.Web.HttpRequest, System.Web.Mvc.ActionResult> poster)
MapGet(string pattern, System.Func<System.Web.Mvc.ActionResult> getter) |
Then we'd need infinity (hyberbole) overloads. Which makes things even more complex (any number of arguments, sync vs async, ValueTask vs Task) |
Sync/ValueTask/Task is 3. Why would you need "any number of arguments"? |
For convenient parameter binding. So instead of having If a developer also wants to get an id from the route (ex: "/api/todos/{id}"), they can add an int parameter instead of calling |
We can even natively support F# async without defining that overload. Because we want to support binding arguments from various sources:
The beauty of supporting any function type is that we can add support for new things in the future, without adding more overloads. |
@cartermp It occurs to me that this is just one of the many features we need to make this work smoothly on F#. The other is around supporting conversion from F# lambdas to Delegate without an explicit cast. |
So thanks to @davidfowl I'm going to leave some feedback here, but unfortunately I have many random thoughts which are not strictly limited to this specific issue. I appreciate that this is not the right place to log them all, but if it's ok with you I'll dumb my thoughts here for now and then move any actionable issue elsewhere with your guidance. Ok, so here we go... First I welcome the changes in ASP.NET Core to introduce a more flexible API which feels closer to the metal. At least this is what my impression is by looking at what you guys are working on and I think it's a great idea! Personally I think the current approach is not bold enough. It's just another "big" framework with a slightly different syntax. IMHO the approach taken tries to solve too many responsibilities at once. The new Houdini style framework tries to appease to too many developers, people like myself who want to have simple to use and flexible lower level API as well as to the "spoilt" developer who just wants a "plug & play" MVC-style API. I think trying to achieve both with a single framework will ultimately fail to deliver the right abstraction which can appeal to both groups and will only change the perception temporarily at best. If it was my decision then I'd suggest to create a very low level (almost Go like) HTTP API which is super flexible and truly bare bones which gives developers maximum control and yet a good enough convenience API which can empower the minimalist developer to feel extremely productive and not having to fight the tools. Using those low level APIs Microsoft could create a second higher level "plug & play" framework for the easy beginner friendly path. Currently this was meant to be ASP.NET Core and MVC, but I think they failed because the lines were not clearly drawn and many ASP.NET Core APIs were specifically built with MVC in mind and not thought of as a standalone library. I almost think that both these things should get built by different teams to avoid the same mistakes and not mud the waters. Now taking a bit more specifically about the low level API which I'd like to see. I don't think it needs (or should) have attributes at all. Declarative APIs are very high level. It means there is something else sitting below which expects certain idioms to be followed in order to carry out useful tasks. If something doesn't follow these idioms then things stop working or fall apart. At least things will not work entirely as advertised which is what will users give the feeling of fighting the tools. Regardless how well one thinks they have designed those idioms, you can only design what you anticipate for. Developers will always want to do something new, something unconventional, something which seemed silly one day but makes sense today and then a declarative API will quickly show its cracks. Therefore I suggest to keep it truly low. No attributes, no declarative model binding. Just have low level functions such as
Does it mean that things like authentication through a single middleware might not work or that another middleware won't be able to produce comprehensive Swagger docs? Yes, but that is ok. I would even suggest that concerns such as authentication and CORS should be moved closer to the endpoints where they are actually being required and not sit in a global middleware which therefore relies on a heavy declarative API in order to centrally control every single endpoint. Again, this only has to be true for the low level API. Move things closer to where it is required. Having something like Yes it's ugly, but it's low level beauty. People who will use that will build their own meaningful wrappers and APIs to build the app which they want. It empowers them and gives them 100% control and lets them create the beautiful APIs which they see fit. On top of these simple idioms you can have a separate attribute-driven API which gives you a more out of the box experience. Of course, in return this will force a user down a certain path but it's a trade-off that people will accept when they know that they can break out into a lower level world if they know that exists. The other thing which I would try to keep in mind is really the naming of things and how it affects the beauty of APIs. @davidfowl mentioned it on Twitter and I couldn't agree more. Beauty is important and in my opinion a big principle of beauty is to ensure that the name of a method or function must match the complexity underneath, otherwise people will think it's magic or too verbose. For example, mapping a single string to a route feels like a very trivial task. A method name like app.MapAction([HttpPost("/")] ([FromBody] Todo todo) : Todo => todo); Better: app.GET("/", ctx => someFunctionToHandle) However, if something is more complex then it is ok to have a slightly longer name. Bad: var model = ctx.Bind<T>() Good: var model = ctx.Request.Body.Bind<T>() Now coming back to F#.... Overall I think it would be a mistake to introduce Attributes as a core concept into F#. We already have them, but their use is very limited and often only in relation to C# interop, but such a change would make attributes a first class citizen which feels wrong if it's only needed in order to mould F# around a new .NET web framework which couldn't let go of a declarative API. EDIT: Until ASP.NET Core offers true low level APIs which don't need attributes or where endpoints must rely on centrally controlled authentication/CORS/etc middlewares then developers will always come and label the current framework as too heavy and too bloated because if they reject the existing API they are left with very little to no other options. Please believe me, I can guarantee you 100% that this will be the case because it's always been like that. |
That's really good feedback, feel free to close this issue. I see now that this isn't idiomatic at all in F# abs users are better served by something like giraffe. We'll definitely consider adding something like bind (though we do that already have some bring for JSON, and it's not clear that we need any more than that). As for the requireAuth(cors(...)) I'm not sure we'd add something like that since we already have an imperative way of adding metadata to endpoints. Again, this is good feedback, muchly appreciated |
F# supports direct conversions at the call site like so: open System
type C() =
member _.M(f: Func<int, int>) = ()
member _.M(f: Action<int>) = ()
let c = C()
c.M(fun x -> x + 1) // direct func interop
c.M(fun x -> printfn $"{x}") // action interop
let funcThing = fun x -> x + 1
let actionThing = fun x -> printfn $"{x}"
c.M(funcThing) // direct func interop
c.M(actionThing) // action interop But there can be some gaps, and it'd probably ease current interop patterns with ASP.NET Core. In the preview bits I played with for some reason I couldn't just interop directly like I can above, so that could be addressed separately. As for the general point @dustinmoris and @davidfowl I don't think there's much harm in F# adopting smoother .NET interop support for existing constructs like this. It is also a matter of orthogonality that is worth discussing - if you can put an attribute on an F# function, why not a lambda when they have near-identical correspondence? I think there are certainly some drawbacks to using it, and I would personally prefer not to use an API that requires lots of attributes. But that's also why we have wonderful choices like Giraffe, Saturn, Falco, Suave, etc. @davidfowl perhaps a better way to ease F# use of aspnetcore is to work out a few of the pain points with @dustinmoris and identify things where:
I don't have context on the former piece, but I'd see it as akin to MVC supporting routes that are written directly as F# async. When that was done, MVC apps didn't need to call |
FWIW these issues are, I believe, ironed out by https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1093-additional-conversions.md. However we should trial and check (it may not apply to overloaded cases for example).
I agree we should address this as it's a basic matter of interop-capability for F# and we'll inevitably encounter attribute-aware .NET frameworks |
I guess this got lost when this got turned into an issue. Here's the original email for context.
@cartermp Then contributed some PRs to the HoudiniPlaygroundFSharp repo that's gotten it closer to something that could theoretically work: halter73/HoudiniPlaygroundFSharp#1 and halter73/HoudiniPlaygroundFSharp#2, but Edit: The MapGet does work because there aren't any attributes required. |
Right I'd like the examples that @halter73 posted to work with F# as well, attributes aside |
We have a new design at dotnet/aspnetcore#30248 that will infer the sources for some parameters without attributes that follow conventions common to ASP.NET Core MVC. Because of these conventions, even the After the changes, the following will work. The type Todo = { Id: int; Name: string; IsComplete: bool }
app.MapPost("/", Func<Todo,Todo>(fun todo -> todo)) |> ignore
app.MapGet("/", Func<Todo>(fun () -> { Id = 0; Name = "Play more!"; IsComplete = false })) |> ignore Having to write out |
YES. And the low level API should be a type-safe one. The convenience API can be completely designed with typical C# devs in mind. @davidfowl and @halter73 were suggesting that for convenience we should lose static typing and use a dynamic convention-based system.
The ASP.Net team shouldn't need to bother about a convenience layer for F# since we should be able to use a type-safe layer directly or write our own convenience layer. The important thing is that all of this can be built on top of a type-safe core API. What is not acceptable is forcing people to use a dynamic API in .Net! Any dynamic API should be optional (nuget package or linker-compatible so it can get linked out if not used). |
Thanks @charlesroddie. We don't want to force anyone to use anything and want to expose the right building blocks so other libraries can build idiomatic layers. We already have a strongly typed core built around a strongly typed RequestDelegate (giraffe is built on top of it). We've also added APIs to read and write JSON from and to the request and response and will continue to do so as we push more features into the core as building blocks. That doesn't change what we're asking for here, but I think it's partially up to the F# community to decide if this is anti F# or not. The building blocks already exist and we will continue to build more of them but we also like the declarative model for C# as it can remove lots of boilerplate in lots of cases. This isn't zero sum. |
I think that's the crucial point here: |
app.MapPost("/", Func<Todo,Todo>(fun todo -> todo)) |> ignore I don't know what type let router = new Router()
router.GET("/", indexHandler)
...
// During web host creation:
app.Endpoints.MapRouter(router)
app.Run() Also the EDIT:
@davidfowl I didn't mean to put a downer on the Houdini API or suggest that this issue should get closed. Clearly the ASP.NET team sees a desire for a web framework which appeals more to the minimalist developer and I only wanted to share some thoughts of someone who considers themselves a minimalist developer with what I thought could be a problem. I think the developers who want to be productive with a higher level declarative API are already very well served with MVC and I think the new Houdini API doesn't necessarily have to compete with MVC but maybe could focus more on what MVC doesn't do well today. I don't know how much is already set in stone, but I'm happy to provide as much constructive feedback as you think is still helpful at this point :) |
The app.MapPost("/", Func<Todo,Todo>(fun todo -> todo)).RequireAuthorization() |> ignore I agree the If you're interested the docs are at https://docs.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-5.0#routing-concepts |
Right, I see.... Another option which I think could work nicely for C# and F# could be by making the handler method the final app.POST("/api/foo").RequiresAuth().EnableCors().Handle(fun todo -> todo) However, the problem only exists because your current framework requires those extra metadata functions in order to feed the information into a central auth middleware and other middlewares. If The API was "lighter" then none of that would even be an issue to begin with :) |
Also, re naming, see how my example reads so much simpler and quicker and allows a developer to comprehend what is happening than the version where everything is stretched much longer with app.MapPost("/", Func<Todo,Todo>(fun todo -> todo)).RequireAuthorization() |> ignore
vs
app.POST("/").RequiresAuth().Handle<Func<Todo,Todo>>(fun todo -> todo) The benefit of a final type TodoHandler = Func<Todo,Todo>
app.POST("/").RequiresAuth().Handle<TodoHandler>(fun todo -> todo) |
Personally, I think |
Also one more thing, I'm not even sure if type TodoHandler = Func<Todo,Todo>
app.POST("/").RequiresAuth(fun options ->
// configure auth options
).Handle<TodoHandler>(fun todo -> todo) Which leads to chains of builders within builders which are really not nice. They are not nice in production code and they are not nice in demos. Many layers of builders make C#/F# code look like a snake with a lot of indentation going in and out multiple times, a bit like a huge HTML file with tons of nested divs. Just the raw look of it makes things look "huge" and "bloated" and "complicated". This kinds of feeds back into the beauty of APIs again. The more the final code is just a set of simple top level statements the nicer it feels and reads. Just my own opinion. I'd suggest to stick with the first level of abstraction and just make better use of that like so: type TodoHandler = Func<Todo,Todo>
app.POST("/").BasicAuth().JWTAuth().SessionCookies().Handle<TodoHandler>(fun todo -> todo) |
Sure I understand that certain APIs have already been shipped and are set in stone, but if there is still an opportunity to make things nice across all of .NET then it's worth considering. After all Giraffe is not a different web framework, I never saw it that anyway. It's just a wrapper library to make ASP.NET Core work for F#. Honestly, I wish this wasn't required in the first place if you see what I mean. |
I completely agree we should make ASP.NET Core as usable out of the box as possible for F#. I appreciate the input. There's still a lot of open design space for the API, but the other existing overloads for Another API you might be interested in is our new "direct hosting" API dotnet/aspnetcore#30354. It's heavily inspired by Feather HTTP. |
Cool will take a look. Thanks for linking! |
That's good feedback about some of the fluent APIs and how it has the potential to complicate the code and make it look like a snake. In practice that doesn't happen because options aren't specified the way you describe. Also one big design goal is NOT to change the existing idioms we're already created. We're not building a new framework, we're refining what we have carefully so we don't break the existing patterns and can take advantage of all the code that is and the ecosystem has written over the last 5 years. That is an explicit design goal. |
PS: I also like the handle method at the end of the fluent chain of calls. We can propose an alternative API for building up a route, that'd be interesting. |
I still program mostly in C# for work and I gotta admit that I really like the look of something like what @dustinmoris suggested, for both F# and C#!
So I like the idea of looking into an alternative API for building up a route, for C# as much as for F# :)! P.S. I also support the idea of adding support for Attributes on lambda expressions in F#, if for nothing else than completeness/symmetry |
@lambdakris That's a weird hybrid of C# and F# there |
@Happypig375 the kind of weird hybrid that @cartermp code fixers will fix in F# editor 😄 |
Despite the minimal-api applicability, I feel like the |
I have tried searching for documentation on this, but failed - how does a typical C# library code search for attributes on arguments of lambdas? Ideally following a doc recommending how these things are typically written. Why I ask this: |
Any chance this will make its way into F# 8 ? |
No, it needs a formal approval, as well as F# 8 is frozen feature-wise. |
Wanted to share that there is a really cool library for cli based apps which has minimal api inspired design: And example is the topic is not working. type Todo = { Id: int; Name: string; IsComplete: bool }
[<HttpPost("/")>]
let echoTodo ([<FromBody>] todo) = todo
[<HttpGet("/")>]
let getTodo () = { Id = 0; Name = "Play more!"; IsComplete = false }
let oneMoreTest(webApp: WebApplication) =
webApp.Map("", Func<Todo,Todo>(echoTodo)) |> ignore
webApp.Map("", Func<Todo>(getTodo)) |> ignore results in (decompiled): public static void oneMoreTest(WebApplication webApp)
{
RouteHandlerBuilder routeHandlerBuilder = webApp.Map("", (Delegate) new Func<WebApplicationTests.Todo, WebApplicationTests.Todo>(WebApplicationTests.oneMoreTest@49.Invoke));
routeHandlerBuilder = webApp.Map("", (Delegate) new Func<WebApplicationTests.Todo>(WebApplicationTests.oneMoreTest@50-1.Invoke));
}
[CompilationMapping(SourceConstructFlags.Closure)]
[Serializable]
[SpecialName]
[StructLayout(LayoutKind.Auto, CharSet = CharSet.Auto)]
internal static class oneMoreTest@49
{
internal static WebApplicationTests.Todo Invoke(WebApplicationTests.Todo delegateArg0) => delegateArg0;
}
[CompilationMapping(SourceConstructFlags.Closure)]
[Serializable]
[SpecialName]
[StructLayout(LayoutKind.Auto, CharSet = CharSet.Auto)]
internal static class oneMoreTest@50-1
{
internal static WebApplicationTests.Todo Invoke() => new WebApplicationTests.Todo(0, "Play more!", false);
} As you see |
This is going to be in F# 9? |
This hasn't been approved, so not likely. |
I propose we support attributes on lambda expressions. A way this could get used is in the experimental aspnet Houdini project, which is experimenting ways to cut the cruft in aspnet core service development, including an alternate routing system than MVC. One of their approaches would be mapping endpoints with lambdas:
Note that the lambda itself has an atttribute. In F#, the above snippet would probably look something like this:
The existing way of approaching this problem in F#, using the above example, would be to pull out the lambda bodies into separate functions and pass them into a constructed
Func
:This isn't really a bad alternative though, the main thing is that it's annoying to have to construct a func. Instead, lobbying to get
MapAction
to support equivalentFSharpFunc
types as overloads and simply have those overloads construct the appropriateFunc
type could make this approach nice from an F# perspective.However, I still think it's worth considering (even if people decide it's not really useful for F#) since this is an approach to programming against an API that more library authors may take in the future.
Pros and Cons
The advantages of making this adjustment to F# are:
The disadvantages of making this adjustment to F# are:
Extra information
Estimated cost (XS, S, M, L, XL, XXL): M
Related suggestions: (put links to related suggestions here)
Affidavit (please submit!)
Please tick this by placing a cross in the box:
Please tick all that apply:
For Readers
If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.
The text was updated successfully, but these errors were encountered: