Fix for #314 - streamline lambda component args

This change removes the magic 'auto-lambda' feature that has some
unconvincing UX.

Also working around a razor bug where explicit expressions are lowered
incorrectly. This should make it possible to write code like:

<Foo Bar="@(e => { OnChanged(e); })" />
This commit is contained in:
Ryan Nowak 2018-03-21 12:31:36 -07:00
parent 635aa55d03
commit 808f741cdd
9 changed files with 148 additions and 70 deletions

View File

@ -425,30 +425,27 @@ namespace Microsoft.AspNetCore.Blazor.Razor
}
else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
{
// See the runtime version of this code for a thorough description of what we're doing here
// We always surround the expression with the delegate constructor. This makes type
// inference inside lambdas, and method group conversion do the right thing.
IntermediateToken token = null;
if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
{
// This is an escaped event handler
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.WriteLine();
WriteCSharpToken(context, ((IntermediateToken)cSharpNode.Children[0]));
context.CodeWriter.Write(");");
context.CodeWriter.WriteLine();
token = cSharpNode.Children[0] as IntermediateToken;
}
else
{
token = node.Children[0] as IntermediateToken;
}
if (token != null)
{
context.CodeWriter.Write(DesignTimeVariable);
context.CodeWriter.Write(" = ");
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.Write(node.BoundAttribute.GetDelegateSignature());
context.CodeWriter.Write(" => ");
WriteCSharpToken(context, ((IntermediateToken)node.Children[0]));
context.CodeWriter.WriteLine();
WriteCSharpToken(context, token);
context.CodeWriter.Write(");");
context.CodeWriter.WriteLine();
}

View File

@ -66,6 +66,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor
builder.Features.Add(new ConfigureBlazorCodeGenerationOptions());
builder.Features.Add(new ComponentDocumentClassifierPass());
builder.Features.Add(new ComplexAttributeContentPass());
builder.Features.Add(new ComponentLoweringPass());
builder.Features.Add(new ComponentTagHelperDescriptorProvider());

View File

@ -447,39 +447,24 @@ namespace Microsoft.AspNetCore.Blazor.Razor
}
else if (node.BoundAttribute?.IsDelegateProperty() ?? false)
{
// This is a UIEventHandler property. We do some special code generation for this
// case so that it's easier to write for common cases.
//
// Example:
// <MyComponent OnClick="Foo()"/>
// --> builder.AddAttribute(X, "OnClick", new UIEventHandler((e) => Foo()));
//
// The constructor is important because we want to put type inference into a state where
// we know the delegate's type should be UIEventHandler. AddAttribute has an overload that
// accepts object, so without the 'new UIEventHandler' things will get ugly.
//
// The escape for this behavior is to prefix the expression with @. This is similar to
// how escaping works for ModelExpression in MVC.
// Example:
// <MyComponent OnClick="@Foo"/>
// --> builder.AddAttribute(X, "OnClick", new UIEventHandler(Foo));
// We always surround the expression with the delegate constructor. This makes type
// inference inside lambdas, and method group conversion do the right thing.
IntermediateToken token = null;
if ((cSharpNode = node.Children[0] as CSharpExpressionIntermediateNode) != null)
{
// This is an escaped event handler;
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.Write(((IntermediateToken)cSharpNode.Children[0]).Content);
context.CodeWriter.Write(")");
token = cSharpNode.Children[0] as IntermediateToken;
}
else
{
token = node.Children[0] as IntermediateToken;
}
if (token != null)
{
context.CodeWriter.Write("new ");
context.CodeWriter.Write(node.BoundAttribute.TypeName);
context.CodeWriter.Write("(");
context.CodeWriter.Write(node.BoundAttribute.GetDelegateSignature());
context.CodeWriter.Write(" => ");
context.CodeWriter.Write(((IntermediateToken)node.Children[0]).Content);
context.CodeWriter.Write(token.Content);
context.CodeWriter.Write(")");
}
}

View File

@ -0,0 +1,101 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Intermediate;
namespace Microsoft.AspNetCore.Blazor.Razor
{
// We don't support 'complex' content for components (mixed C# and markup) right now.
// It's not clear yet if Blazor will have a good scenario to use these constructs.
//
// This is where a lot of the complexity in the Razor/TagHelpers model creeps in and we
// might be able to avoid it if these features aren't needed.
internal class ComplexAttributeContentPass : IntermediateNodePassBase, IRazorOptimizationPass
{
// Run before other Blazor passes
public override int Order => -1000;
protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode)
{
var nodes = documentNode.FindDescendantNodes<TagHelperIntermediateNode>();
for (var i = 0; i < nodes.Count; i++)
{
ProcessAttributes(nodes[i]);
}
}
private void ProcessAttributes(TagHelperIntermediateNode node)
{
for (var i = node.Children.Count - 1; i >= 0; i--)
{
if (node.Children[i] is TagHelperPropertyIntermediateNode propertyNode)
{
if (HasComplexChildContent(propertyNode))
{
node.Diagnostics.Add(BlazorDiagnosticFactory.Create_UnsupportedComplexContent(
propertyNode,
propertyNode.AttributeName));
node.Children.RemoveAt(i);
continue;
}
node.Children[i] = new ComponentAttributeExtensionNode(propertyNode);
}
else if (node.Children[i] is TagHelperHtmlAttributeIntermediateNode htmlNode)
{
if (HasComplexChildContent(htmlNode))
{
node.Diagnostics.Add(BlazorDiagnosticFactory.Create_UnsupportedComplexContent(
htmlNode,
htmlNode.AttributeName));
node.Children.RemoveAt(i);
continue;
}
}
}
}
private static bool HasComplexChildContent(IntermediateNode node)
{
if (node.Children.Count == 1 &&
node.Children[0] is HtmlAttributeIntermediateNode htmlNode &&
htmlNode.Children.Count > 1)
{
// This case can be hit for a 'string' attribute
return true;
}
else if (node.Children.Count == 1 &&
node.Children[0] is CSharpExpressionIntermediateNode cSharpNode &&
cSharpNode.Children.Count > 1)
{
// This case can be hit when the attribute has an explicit @ inside, which
// 'escapes' any special sugar we provide for codegen.
//
// There's a special case here for explicit expressions. See https://github.com/aspnet/Razor/issues/2203
// handling this case as a tactical matter since it's important for lambdas.
if (cSharpNode.Children.Count == 3 &&
cSharpNode.Children[0] is IntermediateToken token0 &&
cSharpNode.Children[2] is IntermediateToken token2 &&
token0.Content == "(" &&
token2.Content == ")")
{
cSharpNode.Children.RemoveAt(2);
cSharpNode.Children.RemoveAt(0);
// We were able to simplify it, all good.
return false;
}
return true;
}
else if (node.Children.Count > 1)
{
// This is the common case for 'mixed' content
return true;
}
return false;
}
}
}

View File

@ -114,11 +114,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor
if (property.kind == PropertyKind.Delegate)
{
var propertyType = (INamedTypeSymbol)property.property.Type;
var parameters = propertyType.DelegateInvokeMethod.Parameters;
var signature = "(" + string.Join(", ", parameters.Select(p => p.Name)) + ")";
pb.Metadata.Add(DelegateSignatureMetadata, signature);
pb.Metadata.Add(DelegateSignatureMetadata, bool.TrueString);
}
xml = property.property.GetDocumentationCommentXml();

View File

@ -16,19 +16,9 @@ namespace Microsoft.AspNetCore.Blazor.Razor
}
var key = ComponentTagHelperDescriptorProvider.DelegateSignatureMetadata;
return attribute.Metadata.TryGetValue(key, out var value);
}
public static string GetDelegateSignature(this BoundAttributeDescriptor attribute)
{
if (attribute == null)
{
throw new ArgumentNullException(nameof(attribute));
}
var key = ComponentTagHelperDescriptorProvider.DelegateSignatureMetadata;
attribute.Metadata.TryGetValue(key, out var value);
return value;
return
attribute.Metadata.TryGetValue(key, out var value) &&
string.Equals(value, bool.TrueString);
}
}
}

View File

@ -153,8 +153,15 @@ namespace Test
}
[Fact]
public void Render_ChildComponent_WithLambdaEventHandler()
[Theory]
[InlineData("e => Increment(e)")]
[InlineData("(e) => Increment(e)")]
[InlineData("@(e => Increment(e))")]
[InlineData("@(e => { Increment(e); })")]
[InlineData("Increment")]
[InlineData("@Increment")]
[InlineData("@(Increment)")]
public void Render_ChildComponent_WithEventHandler(string expression)
{
// Arrange
AdditionalSyntaxTrees.Add(CSharpSyntaxTree.ParseText(@"
@ -171,16 +178,17 @@ namespace Test
}
"));
var component = CompileToComponent(@"
var component = CompileToComponent($@"
@addTagHelper *, TestAssembly
<MyComponent OnClick=""Increment()""/>
@using Microsoft.AspNetCore.Blazor
<MyComponent OnClick=""{expression}""/>
@functions {
@functions {{
private int counter;
private void Increment() {
private void Increment(UIEventArgs e) {{
counter++;
}
}");
}}
}}");
// Act
var frames = GetRenderTree(component);

View File

@ -268,7 +268,7 @@ namespace Test
// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
<MyComponent OnClick=""Increment()""/>
<MyComponent OnClick=""@((e) => { Increment(); })""/>
@functions {
private int counter;
@ -308,9 +308,9 @@ global::System.Object __typeHelper = ""*, TestAssembly"";
{
base.BuildRenderTree(builder);
__o = new Microsoft.AspNetCore.Blazor.UIEventHandler((eventArgs) =>
__o = new Microsoft.AspNetCore.Blazor.UIEventHandler(
#line 2 ""x:\dir\subdir\Test\TestComponent.cshtml""
Increment()
(e) => { Increment(); }
#line default
#line hidden

View File

@ -248,7 +248,7 @@ namespace Test
// Act
var generated = CompileToCSharp(@"
@addTagHelper *, TestAssembly
<MyComponent OnClick=""Increment()""/>
<MyComponent OnClick=""@(e => { Increment(); })""/>
@functions {
private int counter;
@ -277,7 +277,7 @@ namespace Test
{
base.BuildRenderTree(builder);
builder.OpenComponent<Test.MyComponent>(0);
builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler((eventArgs) => Increment()));
builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler(e => { Increment(); }));
builder.CloseComponent();
builder.AddContent(2, ""\n\n"");
}