Skip to content

Conversation

@smasher164
Copy link
Contributor

The DirectoryEntryConverter's implementation is in the System.DirectoryServices.Design namespace. However, the reference was mistakenly put in the System.DirectoryServices namespace. This was flagged by the new ApiCompat tool, where the type argument to the TypeConverter attribute was different (since they're in different namespaces).

The DirectoryEntryConverter's implementation is in the
System.DirectoryServices.Design namespace.
However, the reference was mistakenly put in the
System.DirectoryServices namespace. This was flagged by
the new ApiCompat tool, where the type argument to the
TypeConverter attribute was different (since they're in different
namespaces).
@ghost
Copy link

ghost commented Sep 8, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost assigned smasher164 Sep 8, 2022
@ghost
Copy link

ghost commented Sep 8, 2022

Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014
See info in area-owners.md if you want to be subscribed.

Issue Details

The DirectoryEntryConverter's implementation is in the System.DirectoryServices.Design namespace. However, the reference was mistakenly put in the System.DirectoryServices namespace. This was flagged by the new ApiCompat tool, where the type argument to the TypeConverter attribute was different (since they're in different namespaces).

Author: smasher164
Assignees: -
Labels:

area-System.DirectoryServices, new-api-needs-documentation

Milestone: -

@smasher164
Copy link
Contributor Author

I validated this against a version of the SDK that doesn't have the internal type argument change to ApiCompat.


namespace System.DirectoryServices.Design
{
internal sealed class DirectoryEntryConverter { }
Copy link
Member

Choose a reason for hiding this comment

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

@ericstj do you remember why the internal type is part of / exposed in the contract?

Copy link
Member

Choose a reason for hiding this comment

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

The XAML compiler records the TypeConverter at design time (even if it is internal). #27528

It would do this for any type, even internal types. All these type converters (internal as well) were exposed in the .NETFramework assemblies and this behavior was something WPF depended upon. There were cases we saw that relied on these internal converters. We brought it back for any type from .NETFramework that was exposed in .NET Core assemblies.

This behavior is specifically why we brought back TypeConverterAttributes into reference assemblies - it's why those became promoted to public surface area (including their internal targets).

@smasher164 smasher164 merged commit c89b7c7 into dotnet:main Sep 8, 2022
@smasher164 smasher164 deleted the fix-dirent-namespace branch September 8, 2022 21:54
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants