Skip to content
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.

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

Make TextColorTo Generated Classes for .NET MAUI Controls Internal #791

Merged
merged 6 commits into from
Dec 6, 2022

Conversation

brminnick
Copy link
Collaborator

@brminnick brminnick commented Nov 29, 2022

Description of Change

This PR changes generated classes for .NET MAUI controls, i.e. code generated for Microsoft.Maui.Controls.* by TextColorTo, from public to internal.

The bug in CommunityToolkit/Maui.Markup#158 is caused by two matching public classes being generated in both the .NET MAUI library (eg net7.0-ios) and the referenced class library (e.g. net7.0).

📦MyMauiSolution
 ┣ 📂MyMauiApp
    - <TargetFrameworks>net7.0-android;net7.0-ios;net7.0-maccatalyst</TargetFrameworks>
 ┗ 📂MyLibrary
    - <TargetFramework>net7.0</TargetFramework>

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

@brminnick
Copy link
Collaborator Author

@loop8ack - I went ahead and started the PR based on our discussion: #756 (reply in thread)

Do have time this week to add some additional Unit Tests to this PR?

@loop8ack
Copy link
Contributor

loop8ack commented Nov 29, 2022

Your change would prevent the generated code for self-implemented controls from being public.

For the self-implemented controls, the access modifier should match the control so that the generated code for public controls can also be used outside the assembly.
In other projects this does not cause any problems, because there the controls are not implemented, so the code is not generated.

I would instead change the hardcoded public access modifier to internal in line 60 (foreach loop over mauiTextStyleImplementors).
Then only the code that is generated for Maui controls and therefore present in every project is internal.

Do have time this week to add some additional Unit Tests to this PR?

I have some time this weekend and could write some general tests.
But I have no knowledge of UnitTests for CodeGenerators, I need to learn the bascis first.

@brminnick
Copy link
Collaborator Author

Thanks @loop8ack!

I would instead change the hardcoded public access modifier to internal in line 60 (foreach loop over mauiTextStyleImplementors). Then only the code that is generated for Maui controls and therefore present in every project is internal.

Interesting idea! When you're playing around with the Unit Tests, try writing tests that break my current implementation, where all generated classes are internal, and see if implementing your change to make only the Microsoft.Maui.Controls classes internal fixes it. Hopefully that makes sense!

But I have no knowledge of UnitTests for CodeGenerators, I need to learn the basics first.

No worries at all! You'll be testing the generated code, not the source generator library. Check out the existing TextColorTo Unit Tests to get started.

@brminnick brminnick changed the title Make TextColorTo Generated Classes Internal Make TextColorTo Generated Classes for .NET MAUI Controls Internal Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CS0436 "conflicts with the imported type" when using community toolkit with a shared maui project
5 participants