Skip to content

Blazor API review changes: CompilerServices #11911

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 9, 2019
Merged

Blazor API review changes: CompilerServices #11911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d8437c
Blazor API review changes: CompilerServices
Jul 5, 2019
e464f9a
Remove obsolete test
Jul 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions src/Components/Blazor/Build/test/RenderingRazorIntegrationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,37 +290,6 @@ public void SupportsDataDashAttributes()
frame => AssertFrame.Attribute(frame, "data-def", "Expression value", 2));
}

[Fact]
public void SupportsAttributesWithEventHandlerValues()
{
// Arrange/Act
var component = CompileToComponent(
@"<elem attr=@MyHandleEvent />
@code {
public bool HandlerWasCalled { get; set; } = false;

void MyHandleEvent(Microsoft.AspNetCore.Components.UIEventArgs eventArgs)
{
HandlerWasCalled = true;
}
}");
Copy link
Member Author

Choose a reason for hiding this comment

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

@SteveSandersonMS - speakup if you have concerns. I'm specifically removing support for things like this (not using directive attributes). Should we do this with Action and Func<Task> as well?

Copy link
Member

@SteveSandersonMS SteveSandersonMS Jul 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I expect this is fine, including removing the Action/Func<Task> variants too. The ideal, if we have time for it, would be issuing a compile-time warning for people who put attributes named onX with delegate-type values when they fail to use the directive attribute.

I just checked that our extensibility for event tag helpers works nicely (it does), so people are free to define their own custom event type names and values. This is essential because the HTML spec continues to evolve, and new or nonstandard event names will keep appearing.

Given that the tag helper extensibility works well, there's no use case for people trying to define events handlers any other way. The only risk is that they don't know about how to do it, which is where a compiler warning (perhaps with a fwlink to docs) would be ideal. If we don't have time to add the warning now, it could be added in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we don't have time to add the warning now, it could be added in the future

I'm not certain how this would work in practice. We don't know in the Razor compiler what kind of value you're passing in when you set an attribute.

var handlerWasCalledProperty = component.GetType().GetProperty("HandlerWasCalled");

// Assert
Assert.False((bool)handlerWasCalledProperty.GetValue(component));
Assert.Collection(GetRenderTree(component),
frame => AssertFrame.Element(frame, "elem", 2, 0),
frame =>
{
Assert.Equal(RenderTreeFrameType.Attribute, frame.FrameType);
Assert.Equal(1, frame.Sequence);
Assert.NotNull(frame.AttributeValue);

((Action<UIEventArgs>)frame.AttributeValue)(null);
Assert.True((bool)handlerWasCalledProperty.GetValue(component));
});
}

[Fact]
public void SupportsUsingStatements()
{
Expand Down
41 changes: 7 additions & 34 deletions src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,6 @@ public BindInputElementAttribute(string type, string suffix, string valueAttribu
}
public static partial class BindMethods
{
public static Microsoft.AspNetCore.Components.EventCallback GetEventHandlerValue<T>(Microsoft.AspNetCore.Components.EventCallback value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static Microsoft.AspNetCore.Components.EventCallback<T> GetEventHandlerValue<T>(Microsoft.AspNetCore.Components.EventCallback<T> value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static System.MulticastDelegate GetEventHandlerValue<T>(System.Action value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static System.MulticastDelegate GetEventHandlerValue<T>(System.Action<T> value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static System.MulticastDelegate GetEventHandlerValue<T>(System.Func<System.Threading.Tasks.Task> value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static System.MulticastDelegate GetEventHandlerValue<T>(System.Func<T, System.Threading.Tasks.Task> value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static string GetEventHandlerValue<T>(string value) where T : Microsoft.AspNetCore.Components.UIEventArgs { throw null; }
public static string GetValue(System.DateTime value, string format) { throw null; }
public static T GetValue<T>(T value) { throw null; }
}
Expand Down Expand Up @@ -442,31 +435,6 @@ public partial class UIEventArgs
public UIEventArgs() { }
public string Type { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
}
public static partial class UIEventArgsRenderTreeBuilderExtensions
{
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIChangeEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIClipboardEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIDragEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIErrorEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIFocusEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIKeyboardEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIMouseEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIPointerEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIProgressEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UITouchEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIWheelEventArgs> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIChangeEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIClipboardEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIDragEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIErrorEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIFocusEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIKeyboardEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIMouseEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIPointerEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIProgressEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UITouchEventArgs, System.Threading.Tasks.Task> value) { }
public static void AddAttribute(this Microsoft.AspNetCore.Components.RenderTree.RenderTreeBuilder builder, int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIWheelEventArgs, System.Threading.Tasks.Task> value) { }
}
public partial class UIFocusEventArgs : Microsoft.AspNetCore.Components.UIEventArgs
{
public UIFocusEventArgs() { }
Expand Down Expand Up @@ -566,6 +534,13 @@ protected void SetAbsoluteUri(string uri) { }
protected void TriggerOnLocationChanged(bool isinterceptedLink) { }
}
}
namespace Microsoft.AspNetCore.Components.CompilerServices
{
public static partial class RuntimeHelpers
{
public static T TypeCheck<T>(T value) { throw null; }
}
}
namespace Microsoft.AspNetCore.Components.Forms
{
public sealed partial class EditContext
Expand Down Expand Up @@ -728,9 +703,7 @@ public RenderTreeBuilder(Microsoft.AspNetCore.Components.Rendering.Renderer rend
public void AddAttribute(int sequence, in Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame frame) { }
public void AddAttribute(int sequence, string name, Microsoft.AspNetCore.Components.EventCallback value) { }
public void AddAttribute(int sequence, string name, System.Action value) { }
public void AddAttribute(int sequence, string name, System.Action<Microsoft.AspNetCore.Components.UIEventArgs> value) { }
public void AddAttribute(int sequence, string name, bool value) { }
public void AddAttribute(int sequence, string name, System.Func<Microsoft.AspNetCore.Components.UIEventArgs, System.Threading.Tasks.Task> value) { }
public void AddAttribute(int sequence, string name, System.Func<System.Threading.Tasks.Task> value) { }
public void AddAttribute(int sequence, string name, System.MulticastDelegate value) { }
public void AddAttribute(int sequence, string name, object value) { }
Expand Down
62 changes: 0 additions & 62 deletions src/Components/Components/src/BindMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Globalization;
using System.Threading.Tasks;

namespace Microsoft.AspNetCore.Components
{
Expand All @@ -24,67 +23,6 @@ public static string GetValue(DateTime value, string format) =>
value == default ? null
: (format == null ? value.ToString() : value.ToString(format));

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static string GetEventHandlerValue<T>(string value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static MulticastDelegate GetEventHandlerValue<T>(Action value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static MulticastDelegate GetEventHandlerValue<T>(Func<Task> value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static MulticastDelegate GetEventHandlerValue<T>(Action<T> value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static MulticastDelegate GetEventHandlerValue<T>(Func<T, Task> value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static EventCallback GetEventHandlerValue<T>(EventCallback value)
where T : UIEventArgs
{
return value;
}

/// <summary>
/// Not intended to be used directly.
/// </summary>
public static EventCallback<T> GetEventHandlerValue<T>(EventCallback<T> value)
where T : UIEventArgs
{
return value;
}
}
}
20 changes: 20 additions & 0 deletions src/Components/Components/src/CompilerServices/RuntimeHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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.

namespace Microsoft.AspNetCore.Components.CompilerServices
{
/// <summary>
/// Used by generated code produced by the Components code generator. Not intended or supported
/// for use in application code.
/// </summary>
public static class RuntimeHelpers
{
/// <summary>
/// Not intended for use by application code.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="value"></param>
/// <returns></returns>
public static T TypeCheck<T>(T value) => value;
}
}
2 changes: 1 addition & 1 deletion src/Components/Components/src/Microsoft.AspNetCore.Components.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk.Razor">
<Project Sdk="Microsoft.NET.Sdk.Razor">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
Expand Down
42 changes: 0 additions & 42 deletions src/Components/Components/src/RenderTree/RenderTreeBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,23 +224,6 @@ public void AddAttribute(int sequence, string name, Action value)
AddAttribute(sequence, name, (MulticastDelegate)value);
}

/// <summary>
/// <para>
/// Appends a frame representing an <see cref="Action{UIEventArgs}"/>-valued attribute.
/// </para>
/// <para>
/// The attribute is associated with the most recently added element. If the value is <c>null</c> and the
/// current element is not a component, the frame will be omitted.
/// </para>
/// </summary>
/// <param name="sequence">An integer that represents the position of the instruction in the source code.</param>
/// <param name="name">The name of the attribute.</param>
/// <param name="value">The value of the attribute.</param>
public void AddAttribute(int sequence, string name, Action<UIEventArgs> value)
Copy link
Member Author

Choose a reason for hiding this comment

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

These are unused except for some unit tests. It's still possible to attach arbitrary delegates to the RTB - these methods are here to support method-group-to-delegate conversion and aren't really needed anymore.

Copy link
Member

Choose a reason for hiding this comment

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

It's still possible to attach arbitrary delegates to the RTB

But there's no use case for it, is there? Is this what we're discussing above with "should we remove the Action/Func<T> overloads of AddAttribute, or is this something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

But there's no use case for it, is there?

The only use case for it would be making it nicer for someone writing RTB code by hand. Let's look at this again during the RTB API review.

{
AddAttribute(sequence, name, (MulticastDelegate)value);
}

/// <summary>
/// <para>
/// Appends a frame representing a <see cref="Func{Task}"/>-valued attribute.
Expand All @@ -258,23 +241,6 @@ public void AddAttribute(int sequence, string name, Func<Task> value)
AddAttribute(sequence, name, (MulticastDelegate)value);
}

/// <summary>
/// <para>
/// Appends a frame representing a <see cref="Func{UIEventArgs, Task}"/>-valued attribute.
/// </para>
/// <para>
/// The attribute is associated with the most recently added element. If the value is <c>null</c> and the
/// current element is not a component, the frame will be omitted.
/// </para>
/// </summary>
/// <param name="sequence">An integer that represents the position of the instruction in the source code.</param>
/// <param name="name">The name of the attribute.</param>
/// <param name="value">The value of the attribute.</param>
public void AddAttribute(int sequence, string name, Func<UIEventArgs, Task> value)
{
AddAttribute(sequence, name, (MulticastDelegate)value);
}

/// <summary>
/// <para>
/// Appends a frame representing a delegate-valued attribute.
Expand All @@ -287,14 +253,6 @@ public void AddAttribute(int sequence, string name, Func<UIEventArgs, Task> valu
/// <param name="sequence">An integer that represents the position of the instruction in the source code.</param>
/// <param name="name">The name of the attribute.</param>
/// <param name="value">The value of the attribute.</param>
/// <remarks>
/// This method is provided for infrastructure purposes, and is used to be
/// <see cref="UIEventArgsRenderTreeBuilderExtensions"/> to provide support for delegates of specific
/// types. For a good programming experience when using a custom delegate type, define an
/// extension method similar to
/// <see cref="UIEventArgsRenderTreeBuilderExtensions.AddAttribute(RenderTreeBuilder, int, string, Action{UIChangeEventArgs})"/>
/// that calls this method.
/// </remarks>
public void AddAttribute(int sequence, string name, MulticastDelegate value)
{
AssertCanAddAttribute();
Expand Down
3 changes: 0 additions & 3 deletions src/Components/Components/src/RuntimeHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// 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 System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components.Forms;

namespace Microsoft.AspNetCore.Components
{
Expand Down
Loading