PropertyChanged.SourceGenerator icon indicating copy to clipboard operation
PropertyChanged.SourceGenerator copied to clipboard

Generated code in latest version not compiling/missing OnPropertyChanged

Open VMelnalksnis opened this issue 1 year ago • 7 comments

Description After upgrading to 1.1.0, OnPropertyChanged either does not get called, or PropertyChanged.SourceGenerator.Internal.EventArgsCache.PropertyChanged_PropertyName gets passed instead of property name.

To Reproduce This only generated OnPropertyChanging, but not OnPropertyChanged.

public sealed partial class TestModel
{
	[Notify]
	private decimal _total;
}

Version Info

  • PropertyChanged.SourceGenerator version: 1.1.0
  • Roslyn Version: Compiler version: '4.5.0-6.23127.3 (e2bc27d2)'. Language version: 11.0.

Additional Info Upgrading to this version also caused various issues in Rider. I had to clear caches/restart it multiple times to even reproduce the simple case.

VMelnalksnis avatar Mar 15 '23 17:03 VMelnalksnis

Thanks! I've unlisted v1.1.0 until I get a chance to look into this

canton7 avatar Mar 15 '23 17:03 canton7

Maybe related, is this a typo on line 69? https://github.com/canton7/PropertyChanged.SourceGenerator/blob/94690bbaa71d8e7c6c1bc9e7e3b3f63b4b15dbc6/src/PropertyChanged.SourceGenerator/Generator.cs#L69

LeviGNMBS avatar Mar 05 '24 14:03 LeviGNMBS

When I cloned the repository (master branch) and manually packaged the project (dotnet pack) and used it in my project, everything works fine. There are no errors.

chjrom avatar Mar 16 '24 12:03 chjrom

Maybe related, is this a typo on line 69?

https://github.com/canton7/PropertyChanged.SourceGenerator/blob/94690bbaa71d8e7c6c1bc9e7e3b3f63b4b15dbc6/src/PropertyChanged.SourceGenerator/Generator.cs#L69

Good spot, thanks. It shouldn't be possible to actually hit that code path: we only generate an INotifyPropertyChanging implementation if the user explicitly adds that interface to their type, and in that case we don't need to add the interface to our generated type.

canton7 avatar Jul 20 '24 10:07 canton7

Good spot, thanks. It shouldn't be possible to actually hit that code path: we only generate an INotifyPropertyChanging implementation if the user explicitly adds that interface to their type, and in that case we don't need to add the interface to our generated type.

No problem but are you sure about that though? The example in the quick start doesn't add the interface. Isn't it supposed to be added by the source generator then?

LeviGNMBS avatar Jul 22 '24 08:07 LeviGNMBS

Good spot, thanks. It shouldn't be possible to actually hit that code path: we only generate an INotifyPropertyChanging implementation if the user explicitly adds that interface to their type, and in that case we don't need to add the interface to our generated type.

No problem but are you sure about that though? The example in the quick start doesn't add the interface. Isn't it supposed to be added by the source generator then?

See here:

Working with INotifyPropertyChanging is similar, with the caveat that your class or one of its base classes must implement INotifyPropertyChanging (not everone wants to implement this interface, so it's opt-in)

canton7 avatar Jul 22 '24 08:07 canton7

🤦 I misread changing for changed, typical MS to make such confusing names 😆

LeviGNMBS avatar Jul 22 '24 08:07 LeviGNMBS