Skip to content

API review: Components SSR Forms #50078

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

Closed
SteveSandersonMS opened this issue Aug 15, 2023 · 5 comments
Closed

API review: Components SSR Forms #50078

SteveSandersonMS opened this issue Aug 15, 2023 · 5 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components feature-full-stack-web-ui Full stack web UI with Blazor
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Aug 15, 2023

Background and Motivation

These are APIs related to form handling with Blazor SSR. Ultimately it's all about making <form @onsubmit=...> and <EditForm> able to trigger actions on the server via an HTTP POST, map the incoming data via [SupplyParameterFromForm] properties on components, show up any mapping errors as validation errors, and preserve attempted values even if they are unparseable.

Proposed API

namespace Microsoft.AspNetCore.Components.Forms
{
    public class EditForm : ComponentBase
    {
+        /// <summary>
+        /// Gets or sets the form handler name. This is required for posting it to a server-side endpoint.
+        /// It is not used during interactive rendering.
+        /// </summary>
+        [Parameter] public string? FormName { get; set; }
    }

+    /// <summary>
+    /// Indicates that the value of the associated property should be supplied from
+    /// the form data for the form with the specified name.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
+    public sealed class SupplyParameterFromFormAttribute : CascadingParameterAttributeBase
+    {
+        /// <summary>
+        /// Gets or sets the name for the parameter. The name is used to determine
+        /// the prefix to use to match the form data and decide whether or not the
+        /// value needs to be bound.
+        /// </summary>
+        public override string? Name { get; set; }
+    
+        /// <summary>
+        /// Gets or sets the name for the handler. The name is used to match
+        /// the form data and decide whether or not the value needs to be bound.
+        /// </summary>
+        public string? Handler { get; set; }
+    }

     // Not required, but developers can optionally use <FormMappingScope> to define a named scope around descendant forms.
     // Then that name becomes part of how we route the POST event/data to the right form, avoiding name clashes if you
     // have to have multiple forms with the same name (e.g., multiple copies of a component that contains a form with a
     // fixed name). By default there's an empty-named scope above the root, which suffices in most cases.
+    /// <summary>
+    /// Defines the mapping scope for data received from form posts.
+    /// </summary>
+    public sealed class FormMappingScope : ICascadingValueSupplier, IComponent
+    {
+        public FormMappingScope() {}
+
+        /// <summary>
+        /// The mapping scope name.
+        /// </summary>
+        [Parameter, EditorRequired] public string Name { get; set; }
+
+        /// <summary>
+        /// Specifies the content to be rendered inside this <see cref="FormMappingScope"/>.
+        /// </summary>
+        [Parameter] public RenderFragment<FormMappingContext> ChildContent { get; set; }
+    }

     // The framework instantiates and populates this. Each mapping scope (defined by a <FormMappingScope> component)
     // instantiates one and, each time a [SupplyParameterFromForm] asks it for a value, it populates the FormMappingContext
     // with the attempted value and any mapping errors. Developers aren't expected to interact with this directly as it's
     // mainly a source of data for InputBase/EditContext/etc so they can show mapping errors as validation errors. If they
     // want, app developers can access it directly (received as a [CascadingParameter]) if they want to write custom code
     // that consumes the attempted values and mapping errors.
+    /// <summary>
+    /// The context associated with a given form mapping operation.
+    /// </summary>
+    public sealed class FormMappingContext
+    {
+        /// <summary>
+        /// The mapping scope name.
+        /// </summary>
+        public string MappingScopeName { get; }
+
+        /// <summary>
+        /// Retrieves the list of errors for a given model key.
+        /// </summary>
+        /// <param name="key">The key used to identify the specific part of the model.</param>
+        /// <param name="formName">Form name for a form under this context.</param>
+        /// <returns>The list of errors associated with that part of the model if any.</returns>
+        public FormMappingError? GetErrors(string formName, string key) {}
+
+        public FormMappingError? GetErrors(string key) {} // Across all forms
+
+        /// <summary>
+        /// Retrieves all the errors for the model.
+        /// </summary>
+        /// <param name="formName">Form name for a form under this context.</param>
+        /// <returns>The list of errors associated with the model if any.</returns>
+        public IEnumerable<FormMappingError> GetAllErrors(string formName) {}
+
+        public IEnumerable<FormMappingError> GetAllErrors() {} // Across all forms
+
+        /// <summary>
+        /// Retrieves the attempted value that failed to map for a given model key.
+        /// </summary>
+        /// <param name="formName">Form name for a form under this context.</param>
+        /// <param name="key">The key used to identify the specific part of the model.</param>
+        /// <returns>The attempted value associated with that part of the model if any.</returns>
+        public string? GetAttemptedValue(string formName, string key) {}
+
+        public string? GetAttemptedValue(string key) {} // Across all forms
+    }
}

+namespace Microsoft.AspNetCore.Components.Forms.Mapping
+{
     // Hosting models implement this. For example, M.A.C.Endpoints has HttpContextFormValueMapper that can
     // supply form data by reading it from the HttpContext associated with the current request. The abstraction
     // is only for layering reasons (Blazor's core does not know about HTTP concepts; it's a UI framework)
+    /// <summary>
+    /// Maps form data values to a model.
+    /// </summary>
+    public interface IFormValueMapper
+    {
+        /// <summary>
+        /// Determines whether the specified value type can be mapped.
+        /// </summary>
+        /// <param name="valueType">The <see cref="Type"/> for the value to map.</param>
+        /// <param name="scopeName">The name of the current <see cref="FormMappingScope"/>.</param>
+        /// <param name="formName">The form name, if values should only be provided for that form, or null to allow values from any form within the scope.</param>
+        /// <returns><c>true</c> if the value type can be mapped; otherwise, <c>false</c>.</returns>
+        bool CanMap(Type valueType, string scopeName, string? formName);
+
+        /// <summary>
+        /// Maps the form value with the specified name to a value of the specified type.
+        /// <param name="context">The <see cref="FormValueMappingContext"/>.</param>
+        /// </summary>
+        void Map(FormValueMappingContext context);
+    }
+
+    /// <summary>
+    /// Extension methods for configuring <see cref="SupplyParameterFromFormAttribute"/> within an <see cref="IServiceCollection"/>.
+    /// </summary>
+    public static class SupplyParameterFromFormServiceCollectionExtensions
+    {
         // App developers don't call this manually. M.A.C.Endpoints calls it when it's adding the Blazor DI services.
         // It makes [SupplyValueFromForm] work by registering an ICascadingValueSupplier that reads from the IFormValueMapper.
+        public static IServiceCollection AddSupplyValueFromFormProvider(this IServiceCollection serviceCollection) {}
+    }
+
+    /// <summary>
+    /// An error that occurred during the form mapping process.
+    /// </summary>
+    public class FormMappingError
+    {
+        /// <summary>
+        /// Gets the attempted value that failed to map (if any).
+        /// </summary>
+        public string? AttemptedValue { get; }
+
+        /// <summary>
+        /// Gets or sets the instance that contains the property or element that failed to map.
+        /// </summary>
+        /// <remarks>
+        /// For object models, this is the instance of the object that contains the property that failed to map.
+        /// For collection models, this is the collection instance that contains the element that failed to map.
+        /// For dictionaries, this is the dictionary instance that contains the element that failed to map.
+        /// </remarks>
+        public object Container { get; }
+
+        /// <summary>
+        /// Gets the list of error messages associated with the mapping errors for this field.
+        /// </summary>
+        public IReadOnlyList<FormattableString> ErrorMessages { get; }
+
+        /// <summary>
+        /// Gets or sets the name of the property or element that failed to map.
+        /// </summary>
+        public string Name { get; }
+
+        /// <summary>
+        /// Gets or sets the full path from the model root to the property or element that failed to map.
+        /// </summary>
+        public string Path { get; }
+    }
+
     // This is used by the framework to describe a request for some data item to be mapped, and to receive the result.
     // An alternative would be to have a FormValueMappingRequest and FormValueMappingResult pair. TBH I don't fully know why it's
     // done like this (with a callback for errors instead of the result data modelling errors). However this is not expected to
     // be used directly by typical app developers, and is public only for layering reasons - to expose it to hosting models.
+    /// <summary>
+    /// A context that tracks information about mapping a single value from form data.
+    /// </summary>
+    public class FormValueMappingContext
+    {
         // TODO: Why is this not internal? Why is the type not sealed?
+        public FormValueMappingContext(string acceptMappingScopeName, string? acceptFormName, Type valueType, string parameterName) {}
+
+        /// <summary>
+        /// Gets the name of <see cref="FormMappingScope"/> that is allowed to supply data in this context.
+        /// </summary>
+        public string AcceptMappingScopeName { get; }
+
+        /// <summary>
+        /// If set, indicates that the mapping should only receive values if the incoming form matches this name. If null, the mapping should receive data from any form in the mapping scope.
+        /// </summary>
+        public string? AcceptFormName { get; }
+
+        /// <summary>
+        /// Gets the name of the parameter to map data to.
+        /// </summary>
+        public string ParameterName { get; }
+
+        /// <summary>
+        /// Gets the <see cref="Type"/> of the value to map.
+        /// </summary>
+        public Type ValueType { get; }
+
+        /// <summary>
+        /// Gets the callback to invoke when an error occurs.
+        /// </summary>
+        public Action<string, FormattableString, string?>? OnError { get; set; }
+
+        /// <summary>
+        /// Maps a set of errors to a concrete containing instance.
+        /// </summary>
+        /// <remarks>
+        /// For example, maps errors for a given property in a class to the class instance.
+        /// This is required so that validation can work without the need of the full identifier.
+        /// </remarks>
+        public Action<string, object>? MapErrorToContainer { get; set; }
+
+        /// <summary>
+        /// Gets the result of the mapping operation.
+        /// </summary>
+        public object? Result { get; private set; }
+
         // NOTE: Public because it's called from a different layer (M.A.C.Endpoints)
+        /// <summary>
+        /// Sets the result of the mapping operation.
+        /// </summary>
+        /// <param name="result">The result of the mapping operation.</param>
+        /// <exception cref="InvalidOperationException">Thrown if the result has already been set.</exception>
+        public void SetResult(object? result) {}
+    }
+}
@SteveSandersonMS SteveSandersonMS added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 15, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Aug 15, 2023
@SteveSandersonMS SteveSandersonMS added this to the 8.0-rc2 milestone Aug 15, 2023
@SteveSandersonMS SteveSandersonMS self-assigned this Aug 15, 2023
@SteveSandersonMS SteveSandersonMS added the feature-full-stack-web-ui Full stack web UI with Blazor label Aug 15, 2023
@SteveSandersonMS
Copy link
Member Author

Usage examples

The other APIs are for layering (to allow M.A.C.Endpoints to supply form data from HttpContext, etc.), and to let the M.A.C.Forms components (e.g., InputBase and EditForm) to interact with mapping and any mapping errors it produces. We don't expect application code to use these APIs directly, though they could if they were implementing a custom hosting model or a custom form concept.

@halter73
Copy link
Member

API Review Notes:

  • Is FormName required?
    • Yes for SSR even if there is only one form or else the experience would be really bad when additional forms are added.
  • Rename SupplyParameterFromFormAttribute.Handler to FormName so it's more consistent with the component attribute.
  • Should FormMappingScope.Name be required?
    • It might be possible to generate one, but we think it would be too complicated.\
  • Should FormMappingScope.Name be called FormNamePrefix or something?
    • No. It's a 2-tuple, not a prefix.
  • Let's seal FormValueMappingContext and make its ctor internal.
  • Let's seal the other public classes too if possible.

API approved!

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Aug 16, 2023
@guardrex
Copy link
Contributor

❓ for documentation ...

Is FormName required?
Yes for SSR even if there is only one form or else the experience would be really bad when additional forms are added.

... but at least for Pre7 it doesn't seem to be required in the single form case.

Is this going to throw in the future? I wasn't placing it on the single-form examples, but it sounds like you want every doc example to show setting it, even in the single-form case.

@SteveSandersonMS
Copy link
Member Author

SteveSandersonMS commented Aug 17, 2023

It will be required on all forms that are submitted via SSR. That is:

  • It's not required for interactive rendering (existing WebAssembly/Server apps, or components marked with an interactive rendermode). So, this is not a breaking change.
  • It is only required at the point that a form is actually posted to a MapRazorComponents endpoint as a traditional HTTP POST request, i.e., an SSR form post. We don't throw at the point of rendering a form, but only at the point that an HTTP POST arrives and doesn't specify a form name.

In this SSR-form-post case, it is required on all such form posts that arrive, because otherwise when the POST request arrives we wouldn't know which form it's meant to be dispatched to. The "single form with no name" case would be dangerous and confusing because even if your app works with a single form today, someone might later add a 2nd form with no name somewhere else in the UI, and then the 1st form would become broken because now you have two forms with no name and we don't know which one to dispatch to.

Does that make sense?

@KennethHoff KennethHoff mentioned this issue Aug 19, 2023
1 task
@SteveSandersonMS
Copy link
Member Author

API review updates implemented in https://github.com/dotnet/aspnetcore/pull/50181/commits

@ghost ghost locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components feature-full-stack-web-ui Full stack web UI with Blazor
Projects
None yet
Development

No branches or pull requests

4 participants