IComponent is missing designer attributes

[DustinCampbell]

In particular, IComponent is missing the following two attributes, which were defined on .NET Framework:

[Designer("System.ComponentModel.Design.ComponentDesigner, " + AssemblyRef.SystemDesign, typeof(IDesigner))]
[Designer("System.Windows.Forms.Design.ComponentDocumentDesigner, " + AssemblyRef.SystemDesign, typeof(IRootDesigner))]

This affects both designer and runtime IDesignerHost scenarios. cc @ericstj.

While we've worked around this in our designer by hard-coding knowledge of IComponent's missing attributes in our own IDesignerHost implementation, third-parties would be hard-pressed to do the same for runtime scenarios where the designer is hosted in an application at runtime. An example of such a scenario is DevExpress's Reporting library where they allow a report designer based on CodeDom serialization to embedded directly into an end user application. This is a very meaningful for certain line-of-business apps.

[ericstj]

We would have to push DesignerAttribute down from System.ComponentModel.TypeConverter into System.ComponentModel.Primitives (or lower, like System.ObjectModel) in order to do this. We don't normally make such changes in servicing. How critical is this? Should we be rushing to get this in 3.1 (it's effectively done) or do you have a workaround / not need this for 3.x?

[DustinCampbell]

I'm working around it in the WinForms designer, so we don't need it for 3.1. We'll need it eventually as runtime IDesignerHost scenarios start shaping up (.NET 5?). Otherwise, third-party scenarios won't work properly unless they add similar hacks.

[ericstj]

Got it, we can fix in 5.0. We should also push the EditorAttribute down (and potentially any other related attributes) so that we can address these and related issues.

[DustinCampbell]

I'm betting there's other DesignerAttributes missing too (e.g. Process?), but I only found this one as we've started working on the component tray.

[ericstj]

Have a look here to see if any of these look important: https://github.com/dotnet/corefx/blob/df40a0d4b44d1666971504c8e87379abe7242610/eng/DefaultGenApiDocIds.txt#L3-L8

[Tanya-Solyanik]

Serializers, Editor, and typeDescriptionProvider are definitely important if any runtime scenario is using property grid or design time serialization. For the toolbox we are using a different approach than what was used in the .NET designer, and I don't know about the runtime uses for it. Maybe devexpress reports use it.

[DustinCampbell]

DesignerSerializerAttribute is important, since it allows a component/control to provide custom CodeDom serialization. For example, Control provides ControlCodeDomSerializer to generate SuspendLayout()/ResumeLayout() calls in InitializeComponent().

I'm pretty sure that RootDesignerSerializerAttribute has been deprecated since .NET 2.0, so I can't see any reason to bother with it.

[ericstj]

@DustinCampbell is this still important or are you OK with your workarounds? Fixing this requires moving types around which is not something we like to do unless there is a very strong need.

[ericstj]

So the following attributes need to be applied to all the types that had them in desktop:

T:System.ComponentModel.DesignerAttribute
T:System.ComponentModel.Design.Serialization.DesignerSerializerAttribute
T:System.ComponentModel.EditorAttribute
T:System.ComponentModel.ToolboxItemAttribute
T:System.ComponentModel.TypeDescriptionProviderAttribute

These types should be pushed down as low as necessary to apply them in all cases.

In places where these attributes depend on other types, like here

runtime/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/DesignerAttribute.cs

Line 23 in 0e23ef4

DesignerBaseTypeName = typeof(IDesigner).FullName;

We should avoid that dependency by using type name, as is done here:

runtime/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ToolboxItemAttribute.cs

Line 40 in 0e23ef4

_toolboxItemTypeName = "System.Drawing.Design.ToolboxItem, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a";

cc @Anipik since this is related to work he's doing WRT attributes in references.

[DustinCampbell]

@safern, @ericstj: It looks like one of the [Designer] attributes was added incorrectly to IComponent. The attribute for ComponentDocumentDesigner should specify a second argument of type IRootDesigner to distinguish the two designers. Can we reopen and address this?

[safern]

@DustinCampbell -- thanks for the heads up. Will re-open, and will move to 5.0.x so that we can fix that on the first servicing release.

[safern]

Also, ComponentDesigner is wrong...

[DustinCampbell]

Thanks @safern!

Is ComponentDesigner wrong? It looks like DesignerAttribute uses IDesigner as a base type if the base type isn't specified.

[safern]

You're right... somehow I can't read anymore and read IRootDesigner 😆

[ericstj]

@DustinCampbell is this important to service in 5.0? Does it need to be serviced in the reference assemblies as well?

[DustinCampbell]

I do think it's important to service in 5.0, but I don't think it's required in the reference assemblies? Typically, the designer acquires information like attributes from the runtime types or component instances.

[ericstj]

Do you think you could share a repro that demonstrates the bug so that we can both validate the fix is working and determine if reference update is necessary?

I'm a bit surprised the designer has access to runtime assemblies, for instance in the case where the app is targeting net5.0 without a RID and tooling might be running on a different framework. Where would it find the runtime assembly?

[DustinCampbell]

Do you think you could share a repro that demonstrates the bug so that we can both validate the fix is working and determine if reference update is necessary?

The repro would be to create an IComponent instance and call TypeDescriptor.CreateDesigner(...) with the base type as typeof(IDesigner) and then again with typeof(IRootDesigner). Today, both calls will return ComponentDesigner, but the second should return ComponentDocumentDesigner.

This blocks designer scenarios where the user opens a component or control as a root designer. When the user drops some component on a form designer, the designer host will already had a root component. So, it'll ask to create a designer with a base type of IDesigner. However, if the user double-clicks a component or control in the Solution Explorer, it'll be opened as the root component. So, the designer host will ask to create a designer with a base type of IRootDesigner. Without this change, the latter case is broken. I can work around it in the new designer, but'll still be broken from runtime designer hosting scenarios.

https://github.com/dotnet/winforms/blob/6a4da4de1c03d3778c63b2b0195045d00c490fd5/src/System.Windows.Forms.Design/src/System/ComponentModel/Design/DesignSurface.cs#L283-L305

I'm a bit surprised the designer has access to runtime assemblies, for instance in the case where the app is targeting net5.0 without a RID and tooling might be running on a different framework. Where would it find the runtime assembly?

Everything is done via reflection using TypeDescriptor and PropertyDescriptor, typically on runtime objects and types. In the classic designer, there was a filtering done to hide runtime properties that didn't appear on reference assemblies. This was necessary to hide properties that were available on runtime objects that didn't appear in the TFM that the project was referencing, since VS typically ran on a different TFM. However, the designer has always worked on runtime types and object instances.

In the .NET Core designer, we construct a server process (much like WPF does) using a runtimeconfig.json and deps.json that matches up with the user's project. That way, the process runs using the runtime, NuGet references, etc. that the user's app targets. So, the designer will be running on the same RID that the user is targeting. So, the runtime assemblies that it needs are in the default AssemblyLoadContext.

[safern]

Opening to track porting it to 5.0

[safern]

Port is merged.