Skip to content
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

Add types for Minimal API #35

Merged
merged 7 commits into from
Feb 24, 2022
Merged

Add types for Minimal API #35

merged 7 commits into from
Feb 24, 2022

Conversation

Swimburger
Copy link
Contributor

@Swimburger Swimburger commented Jan 28, 2022

ASP.NET Core 6 introduced a new way to build web applications using Minimal API.
This doesn't use controllers or action results, but uses the new IResult type.
It is suggested to add extension methods to IResultExtensions so it can be invoked using Results.Extensions.YourExtension.
Here's the guidance I found: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis?view=aspnetcore-6.0#customizing-results

To get this to work, I had to make a couple of decisions which should be up for discussion:

  • The types only seemed to be recognized when targeting net6.0. This is probably not acceptable to maintain support for older versions of .NET.
  • The types are in a separate namespace Twilio.AspNet.Core.MinimalApi to prevent name collisions.
  • I don't see much value in adding the extension to IResultExtensions, it'll make it so the extension method is available at Results.Extensions.TwiML, but we could just create our own static type TwilioResults.TwiML or something similar.

This is how the new code would be used:

app.MapGet("/voice", () =>
{
    var response = new VoiceResponse();
    response.Say("Hello World!");
    return Results.Extensions.TwiML(response);
});
app.MapGet("/message", () =>
{
    var response = new MessagingResponse();
    response.Message("Hello World!");
    return Results.Extensions.TwiML(response);
});

What do y'all think?

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

The msbuild commands would not correctly compile Twilio.AspNet.Core using NET6.0, but the dotnet CLI does equivalent commands does.
@Swimburger
Copy link
Contributor Author

Couple of updates:

  • I learned how to use C# preprocessor conditionals, so we can include these new features while still supporting older versions of .NET.
  • For multi-targeting to work with .NET 6, the build script needed to be updated to use the dotnet CLI more
  • We'll keep using Results.Extensions.TwiML as it is the recommended way
  • Version has been bumped to 5.71.0
  • @dprothero is helping me onboard on this project

@Swimburger
Copy link
Contributor Author

@dprothero what are the next steps to get this towards a mergable state?

@dprothero
Copy link
Collaborator

@Swimburger I think it's there!

@Swimburger Swimburger mentioned this pull request Feb 24, 2022
1 task
@Swimburger Swimburger marked this pull request as ready for review February 24, 2022 19:48
@Swimburger Swimburger requested a review from dprothero February 24, 2022 19:48
@Swimburger
Copy link
Contributor Author

@dprothero this is ready for review, would you like to review it or should I ask someone else?

Copy link
Collaborator

@dprothero dprothero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Lots of yak shaving to be done, but it looks good.

@dprothero dprothero merged commit 525c867 into twilio-labs:main Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants