Removed usage of InternalsVisibleTo in all components
This PR:
- Removes all usage of
InternalsVisibleToin components in this repo, closes #441. - Copies minor helpers such as
ArgumentNullExceptionExtensions, keeping them internal. - Makes public any internal code required to build, mostly around Animations references in Media and Extensions references in Animations.
For 3), it's worth noting that several API surfaces were marked as public which were previously internal. Only the areas that needed to be exposed for other toolkit packages to build on top were updated.
These changes were required for these components to be extended by other components in the toolkit. I'm in favor of publishing them as public for others to build on to the same extent we have, unless we have information on why these were originally made internal and good reason to keep it.
Tagging @Sergio0694 in on some of these questions, as a lot of the internal APIs are from his Animation library, so he may know more of why they were internal only.
I really dislike these changes. What is the actual issue that's caused by using
[InternalsVisibleTo]? Can we not only enable it for these two projects?
We're testing these changes for three reasons:
- https://github.com/CommunityToolkit/Windows/issues/169#issuecomment-2181688627
- https://github.com/CommunityToolkit/Windows/pull/404
- ~~https://github.com/CommunityToolkit/Tooling-Windows-Submodule/pull/195~~
The last reason is dismissed, https://github.com/CommunityToolkit/Tooling-Windows-Submodule/pull/195#issuecomment-2204276472 helped us verify that it was unrelated, and the fix offered by Mano and yourself @Sergio0694 double confirms that.
The first two are still a possible concern. If we can verify that the use of InternalsVisibleTo here isn't uncovering a platform bug (causing #169 or #404), we can dismiss that as well and close this PR without merging.
I really dislike these changes.
@Sergio0694 To move this along, could you elaborate on this?
The rationale here was to expose the same APIs to consumers that we ourselves are using to build controls.
Since this is code you wrote, we'd pinged you to gather your insights on why these were marked internal so we could align our implementation with our rationale.
"The rationale here was to expose the same APIs to consumers that we ourselves are using to build controls."
Those APIs are an internal implementation details of our other APIs, and were not meant to be made public. We should make APIs public because they make sense, not just because we need them to make things build. This PR should not be merged.