Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Adding Source Generator #1714

Merged
merged 17 commits into from
Dec 31, 2021
Merged

Adding Source Generator #1714

merged 17 commits into from
Dec 31, 2021

Conversation

pictos
Copy link
Contributor

@pictos pictos commented Nov 11, 2021

Description of Bug

This PR adds a Source Generator in our project and nuget, with this we can call an Init method and hopefully avoid that fake XAML error.

I didn't test on Mac, so please try it on Mac before merging.

Issues Fixed

Behavioral Changes

This shouldn't change anything in the lib.

PR Checklist

  • Has a linked Issue, and the Issue has been approved
  • 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

Copy link
Contributor

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Cool! I know very little about Source Generators, so I have two questions:

  • How come it's in its own csproj, Xamarin.CommunityToolkit.SourceGenerator.csproj?
  • Do we need to commit Xamarin.CommunityToolkit.SourceGenerator.dll to the repo? Or is that auto-generated at build time?

@pictos
Copy link
Contributor Author

pictos commented Nov 11, 2021

@brminnick

How come it's in its own csproj, Xamarin.CommunityToolkit.SourceGenerator.csproj?

Not sure if I follow this question. But for Source Generators (SG) we need to have a separate project that targets netstandard2.0. Nuget handles SGs the same way as an analyzer.

Do we need to commit Xamarin.CommunityToolkit.SourceGenerator.dll to the repo? Or is that auto-generated at build time?

We can do both since we will not touch in SG too much (I believe that we will never change anything in there) I added the .dll to our repo and reference it. Now if we don't want to do that, we need to build the Xamarin.CommunityToolkit.SourceGenerator.csproj and then reference the result to Xamarin.CommunityToolkit.csproj.
Here's a sample of how that would be

@brminnick
Copy link
Contributor

Sounds good to me!

Is there anyway to make a Unit Test for the error fixed by this?

@pictos
Copy link
Contributor Author

pictos commented Nov 11, 2021

I'll investigate that.
In advance, I'm not sure if it's possible because we will need to have access to the VS errors and see if the XamlC error XFC0000 is on the list

@brminnick
Copy link
Contributor

Agreed. The only thing I can think of is to make a second sample app that doesn't reference any toolkit stuff in C# (only using XAML). Then we build that sample in our CI/CD pipeline to confirm there's no build errors.

It's not exactly a "Unit Test", but it'd accomplish the sample thing.

@brminnick brminnick added this to the 1.4 milestone Nov 24, 2021
@maxkoshevoi
Copy link
Contributor

  • Do we need to commit Xamarin.CommunityToolkit.SourceGenerator.dll to the repo? Or is that auto-generated at build time?

You don't need to do that. You can pack the dll inside the NuGet (same way it works with static analyzers). Actually, Source Generators and static analyzers are almost the same thing. You can think about Source Generators as static analyzers that can produce files.

Copy link
Contributor

@VladislavAntonyuk VladislavAntonyuk left a comment

Choose a reason for hiding this comment

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

Can we try not include dll as Max suggested?

@pictos
Copy link
Contributor Author

pictos commented Dec 29, 2021

@VladislavAntonyuk IMHO is better to use the dll instead of binding this project every time. This SG is just generating that code all the time, so we don't need to include it on our build process. Referencing the dll in release mode will be the best approach

@brminnick brminnick enabled auto-merge (squash) December 31, 2021 01:43
@brminnick brminnick merged commit e981bea into main Dec 31, 2021
@brminnick brminnick deleted the pj/source-generator branch December 31, 2021 01:55
@tranb3r
Copy link
Contributor

tranb3r commented Jan 12, 2022

I'm having a small issue with this change. The type Xamarin.CommunityToolkit.Initializer.XCTInitCaller is now present in each assembly of my app. This results in a 'duplicate type' warning when I merge these assemblies (obfuscation step during the release process of my app).
I think it would have been better to generate this class in a namespace that depends on the assembly (for example, XCT.[assembly name]). This would avoid potential conflicts when merging assemblies.
Do you think it's possible to modify this SG in the next version of XCT?

@brminnick
Copy link
Contributor

@pictos FYI 👆

Thanks @tranb3r! Could you report this bug in a new Issue and add a reproduction sample?

@pictos
Copy link
Contributor Author

pictos commented Jan 17, 2022

@tranb3r please open an issue and mark me. IF possible provide a small sample that I can use as reference

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]Error: : XamlC error XFC0000 : Cannot resolve type "Shield"
6 participants