search

reactivemarbles / ObservableEvents

MIT License
122 stars 10 forks source link

Static code shouldn't be generated #136

Closed MisinformedDNA closed 2 years ago

MisinformedDNA commented 2 years ago

The string in EventGenerator.cs is never altered, so why is this code being generated. Why not just create actual classes so they don't have to be generated?

glennawatson commented 2 years ago

Without it the entire generator wouldn't work.

It uses the code to be injected in as a 'hook' then relies on calls to the generic class in the class to produce the actual observable.

MisinformedDNA commented 2 years ago

I tried it and I didn't notice any problems. If you look at the .NET code base, their attributes aren't generated either. See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/Common/JsonSourceGenerationOptionsAttribute.cs and https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LoggerMessageAttribute.cs.

It's especially important for the attribute to be accessible for discoverability purposes. As it is now, if I paste [assembly: GenerateStaticEventObservables(typeof(StaticTest))] into my code, VS can't help me identify what using statement I'm missing. It's a small thing that helps developers fall into the pit of success.

glennawatson commented 2 years ago

Yeah, it's changing slightly how it is working in the next release but not from being injected by the source generator.

Mostly the next version which is Roslyn 4-based will avoid the use of namespaces.

System.text.Json and the Microsoft logging extensions have the luxury of an existing assembly.

glennawatson commented 2 years ago

The approach you've taken in your example won't work as a package fyi, due to it being a development dependency library and a analyzer.

Thanks for bringing up the suggestion regardless.

MisinformedDNA commented 2 years ago

Yeah, .NET does have that luxury. Follow-up question: Why does it have to be a development dependency? I searched NuGet. Some are and some aren't.

glennawatson commented 2 years ago

If you don't have it as a development dependency then every project that depends on this source generator will have the source generator running. We opt to have it for the user to opt in each time for each project by default.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.