andrewlock / StronglyTypedId

A Rosyln-powered generator for strongly-typed IDs
MIT License
1.5k stars 78 forks source link

Fundamental redesign to allow complete customisation #117

Closed andrewlock closed 8 months ago

andrewlock commented 8 months ago

This PR implements the approach described in https://github.com/andrewlock/StronglyTypedId/issues/102, namely it makes a fundamental redesign of the library.

Instead of an ever-increasing set of options for generating IDs, this moves the library to providing a small, minimal implementation of the core backing types by default, but gives the option for users to customise their generated IDs completely.

My hope is that this strikes a balance between a simple "getting-started" solution without any required configuration, but with an option to provide your own templates if you don't agree with my choices, or you want to add extra functionality.

Using the built-in templates

The quickest way to get started with the library is to use one of the built-in Template definitions. For example:

using StronglyTypedIds;

// set the default template, defaults to Guid if you don't add this assembly attribute
[assembly:StronglyTypedIdDefaults(Template.Int)] 

[StronglyTypedId] // use the default template
public partial struct DefaultId {}

[StronglyTypedId(Template.Guid)] // use the Guid template instead
public partial struct GuidId {}

Currently the library includes just four core templates:

Each of these templates implements the following features:

The intention with these templates is to be as widely applicable as possible. You should be able to use the templates in any of your libraries without issue. However, if you need additional converters, such as EF Core, Newtonsoft.Json or Dapper, then you'll need to use a custom template.

Using custom templates

In addition to the built-in templates, you can provide your own templates for use with strongly typed IDs. To do this, do the following:

For example, you could create a template called guid-efcore.typedid like this:

partial struct PLACEHOLDERID
{
    public class EfCoreValueConverter : global::Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<PLACEHOLDERID, global::System.Guid>
    {
        public EfCoreValueConverter() : this(null) { }
        public EfCoreValueConverter(global::Microsoft.EntityFrameworkCore.Storage.ValueConversion.ConverterMappingHints? mappingHints = null)
            : base(
                id => id.Value,
                value => new PLACEHOLDERID(value),
                mappingHints
            ) { }
    }
}

And then you can apply it to your IDs like this:

[StronglyTypedId(Template.Guid, "guid-efcore")] // Use the built-in Guid template and also the custom template
public partial struct GuidId {}

This shows another important feature: you can specify multiple templates to use when generating the ID.

Using multiple templates

When specifying the templates for an ID, you can specify

For example:

[StronglyTypedId] // Use whatever the default templates are!
public partial struct MyDefaultId {}

[StronglyTypedId(Template.Guid)] // Use a built-in template only
public partial struct MyId1 {}

[StronglyTypedId("my-guid")] // Use a custom-template only
public partial struct MyId2 {}

[StronglyTypedId(Template.Guid, "guid-efcore")] // Use a built-in template _and_ a custom template
public partial struct MyId3 {}

// Use a built-in template _and_ multiple custom template
[StronglyTypedId(Template.Guid, "guid-efcore", "guid-dapper)]
public partial struct MyId4 {}

Similarly, for the optional [StronglyTypedIdDefaults] assembly attribute, which defines the default templates to use when you use the raw [StronglyTypedId] attribute you use a combination of built-in and/or custom templates. The only difference is you have to specify some template:

//⚠ You can only use _one_ of these in your project, I'm just showing them all here for comparison

[assembly:StronglyTypedIdDefaults(Template.Guid)] // Use a built-in template only

[assembly:StronglyTypedIdDefaults("my-guid")] // Use a custom-template only

[assembly:StronglyTypedIdDefaults(Template.Guid, "guid-efcore")] // Use a built-in template _and_ a custom template

// Use a built-in template _and_ multiple custom template
[assembly:StronglyTypedIdDefaults(Template.Guid, "guid-efcore", "guid-dapper)]

[StronglyTypedId] // Uses whatever templates were specified!
public partial struct MyDefaultId {}

The bad news is that this design requires a bit more effort from users to create your own templates. Luckily you have a little bit of help with that

Codefix for creating a template

As well as the source generator, the StronglyTypedId NuGet package now includes a CodeFix that looks for cases where you have specified a custom template that the source generator cannot find. For example, in the following code,the "some-int" template does not yet exist

[StronglyTypedId("some-int")] // does not exist
public partial struct MyStruct
{
}

In the IDE, you can see the generator has marked this as an error:

An error is shown when the template does not exist

The image above also shows that there's a CodeFix action available. Clicking the action reveals the possible action: Add some-int.typedid template to the project, and shows a preview of the file that will be added:

Showing the CodeFix in action, suggesting you can add a project

Choosing this option will add the template to your project. Unfortunately, due to limitations with the Roslyn APIs, it's not possible to add the new template with the required AdditionalFiles/C# analyzer additional file build action. Until you choose this option, the error will remain on your [StronglyTypedId] attribute.

Right-click the newly-added template, choose Properties, and change the Build Action to either C# analyzer additional file (Visual Studio 2022) or AdditionalFiles (JetBrains Rider). The source generator will then detect your template and the error will disappear.

You can see the complete flow in the following video:

The complete code-fix flow

Optional new StronglyTypedId.Templates packages

The ability to provide custom templates will (I hope) mean that people can get the most out of this library without running into my feelings about how strongly-typed-ids should be used.

If you need a new converter, no need to wait for it to be implemented in the project and for a new release. You can simply add your own custom template, optionally set it as the default for your ids, and carry on.

That said, one of the original advantages of this project was that you didn't have to write any of the converters yourself. With custom templates, you lose that benefit to an extent. To counteract that, I've decided to also publish a collection of "community templates" as a NuGet package, StronglyTypedId.Templates.

This package is entirely optional, but if you add it to your project, the templates it contains are automatically available as custom templates. Currently the package includes the "full" versions of all the ID implementations available in the previous version of the StronglyTypedId package, which contain all the converters and implementations previously available:

Additionally, specific "standalone" EF Core, Dapper, and Newtonsoft JSON converter templates are available to enhance the Guid/int/long/string built-in templates. The initial version of the package includes the following templates:

These are designed to be used in conjunction with the built-in templates. For example, if the built-in Guid template works for you, but you need an EF Core converter, you can use the following:

[StronglyTypedId(Template.Guid, "guid-efcore")]
public partial struct MyStruct { }

Summary

I've spent quite some time working on this re-architecture and I think it will solve the main problems I currently see with the library, namely that people want different things from it, and it's frankly become a burden on me! I hope everyone understands that, but if there's any obvious issues/recommendations/improvements we can make before I release the next version I would love to here them. Thanks all!

This also fixes/addresses/closes a variety of open issues: fixes #114 fixes #113 fixes #110 fixes #102 fixes #100 fixes #99 fixes #96 fixes #84 fixes #82 fixes #75 fixes #70 fixes #68 fixes #64 fixes #60 fixes #56 fixes #55 fixes #54 fixes #39 fixes #30 fixes #20 fixes #19 fixes #18

SeanFarrow commented 8 months ago

I like this redesign.

My only real question and yes, I know this might be a stretch, is around creating custom templates.

Let's say I create a custom template that requires referencing either a NuGet package or a project in my solution, can the current template syntax take account of that and automatically add the reference for me? It would be really good if this could be don and I'm happy to help you do this if needed before you release.

Thanks, Sean.

andrewlock commented 8 months ago

Thanks for the suggestion @SeanFarrow! πŸ™‚

Unfortunately, I'm not really sure how that would be possible πŸ€” The generator doesn't parse the templates it reads, and it's not really feasible for it to do so I don't think. We could potentially add a syntax to the top of templates that we could then parse, but it seems like a lot of hassle and somewhat tangential to the general approach here... On top of that, things could get complicated quickly, which version of the library do we add? What if there are conflicting version numbers in the templates. When do we do it?

Additionally, I think there's a question of whether we should do this anyway, I don't know if I would want a template to "secretly" add a package for me. The IDE will generally flag the issue anyway, which feels like a good enough solution to me (and is effectively what we currently have if you reference the Dapper or EF Core converters for example)

SeanFarrow commented 8 months ago

Very true. We need to make it clear in the documentation then that if you are referencing templates that use external packages, these will not be added for you. I think that would be a good solution.

Thanks, Sean.

From: Andrew Lock @.> Sent: Sunday, November 12, 2023 1:32 PM To: andrewlock/StronglyTypedId @.> Cc: Sean Farrow @.>; Mention @.> Subject: Re: [andrewlock/StronglyTypedId] Fundamental redesign to allow complete customisation (PR #117)

Thanks for the suggestion @SeanFarrowhttps://github.com/SeanFarrow! πŸ™‚

Unfortunately, I'm not really sure how that would be possible πŸ€” The generator doesn't parse the templates it reads, and it's not really feasible for it to do so I don't think. We could potentially add a syntax to the top of templates that we could then parse, but it seems like a lot of hassle and somewhat tangential to the general approach here... On top of that, things could get complicated quickly, which version of the library do we add? What if there are conflicting version numbers in the templates. When do we do it?

Additionally, I think there's a question of whether we should do this anyway, I don't know if I would want a template to "secretly" add a package for me. The IDE will generally flag the issue anyway, which feels like a good enough solution to me (and is effectively what we currently have if you reference the Dapper or EF Core converters for example)

β€” Reply to this email directly, view it on GitHubhttps://github.com/andrewlock/StronglyTypedId/pull/117#issuecomment-1807130128, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALDK7XCC47O265XXTJVLFTYEDFWNAVCNFSM6AAAAAA7HN4VBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGEZTAMJSHA. You are receiving this because you were mentioned.Message ID: @.***>

dzmitry-lahoda commented 8 months ago

super cool, thank you. just asking. could "templates" be just some limited C# cs files? :) i am ok both template and id compiled

andrewlock commented 8 months ago

super cool, thank you. just asking. could "templates" be just some limited C# cs files? :) i am ok both template and id compiled

That's pretty much how it works with this approach πŸ™‚ essentially you can create your type as a c# .C's file, where the type is called PLACEHOLDERID. You then rename the file to .typedid and wet the build action to Additional File, and it works!

arex388 commented 8 months ago

I have a suggestion when it comes to custom templates. Would it be possible as part of the library's documentation to add best practice recommendations over time as the community embraces and uses the templates?

My specific worry is that I end up needing to have a template for some edge case, and I write it, but what if the code is somehow bad or something I found somewhere on the internet and it opens the app to security, performance or other issues? I might recognize and fix such issues, or I might not.

I think that if the library's docs could show best practices for different standard and edge cases and becomes the single source of authority I personally would feel better that I'm not opening a can of worms with a custom template I make.

andrewlock commented 8 months ago

I have a suggestion when it comes to custom templates. Would it be possible as part of the library's documentation to add best practice recommendations over time as the community embraces and uses the templates?

My specific worry is that I end up needing to have a template for some edge case, and I write it, but what if the code is somehow bad or something I found somewhere on the internet and it opens the app to security, performance or other issues? I might recognize and fix such issues, or I might not.

I think that if the library's docs could show best practices for different standard and edge cases and becomes the single source of authority I personally would feel better that I'm not opening a can of worms with a custom template I make.

I think the answer to this one is both a yes and no...

I'm very much not interested in turning the StronglyTypedId documentation into an essay about how to use Span<T>, using Ordinal string comparisons, or thinking about null. Those are very much outside the scope of this library, and there's lots of great existing documentation about that around the internetπŸ™‚

On the other hand, I think it's totally reasonable for the library to help you get started with a template. That's the intention of the CodeFix provider I described (and shown in the video above). Currently, that codefix is semi-smart, in that it checks the name of the template you need to create. If it detects an int, long or string in the name, it generates a template based on those backing types, and includes all the built-in converters. The default is a Guid-based template.

By making the template a fully-fleshed out implementation with all the converters, hopefully it should mostly be a case of deleting things from there, adding extra features to the template, or changing some of the implementations. Which I think should be a much easier prospect than starting from scratch, and hopefully alleviates your concerns πŸ™‚

nils-a commented 8 months ago

Love it! Great work!

@SeanFarrow I think what you are seeking can be done in the form of a "plugin" - one could create a NuGet package that supplies new templates (very much like StronglyTypedIds.Templates does in this PR) and at the same time that package could reference some other, required package.

@andrewlock, have you already planned out a package-namespace for plugins? When this PR is merged, I'd love to provide what is now in #81 as a plugin, so I don't need to re-create my templates every time. My first idea was StronglyTypedIds.Plugins.* but that might sound too official for a 3rd-party plugin. (And it might lead to you getting issues for code that's not really yours.)

andrewlock commented 8 months ago

Thanks @nils-a!

one could create a NuGet package that supplies new templates (very much like StronglyTypedIds.Templates does in this PR) and at the same time that package could reference some other, required package.

Great idea!

have you already planned out a package-namespace for plugins? When this PR is merged, I'd love to provide what is now in https://github.com/andrewlock/StronglyTypedId/pull/81 as a plugin, so I don't need to re-create my templates every time.

I haven't, as the namespace doesn't really matter. As long as the package adds the files as AdditionalFiles, they'll be picked up. So namespaces don't really come into it πŸ™‚

One thing to be aware of that I haven't tackled yet is how to tackle "duplicate" templates. I think it's probably easiest to just add a diagnostic for them at the point of use.

Another minor concern I have is around the usability of the templates package. Currently it's not very easy to see which templates are being made available from inside your IDE. I tried various options for that, but didn't come up with a great solution πŸ€·β€β™‚οΈ

My first idea was StronglyTypedIds.Plugins.* but that might sound too official for a 3rd-party plugin. (And it might lead to you getting issues for code that's not really yours.)

Generally it's good practice to not use the first namespace like this, unless it's "official" (It's not even possible for some cases where the prefix has been reserved). So I would suggest something more like SomeNameHere.StronglyTypedId.Templates.* (or plugins or whatever πŸ™‚).

I'm also thinking it would be good to include a list of packages like that on the README, so that others can find them, but making it clear they're maintained by other people πŸ™‚

nils-a commented 8 months ago

One thing to be aware of that I haven't tackled yet is how to tackle "duplicate" templates.

Good point. Probably "plugin creators" should maybe add some prefix or at least some relatively project-unique identifier to their templates to avoid collisions.

Currently it's not very easy to see which templates are being made available from inside your IDE.

My first idea was to provide custom Attributes with the package, but as I'm not quite sure if that's a good idea (or even feasible) I'd most likely opt for providing constants with the package and see how well that works.

I would suggest something more like SomeNameHere.StronglyTypedId.Templates.*

Sounds perfect!

I'm also thinking it would be good to include a list of packages like that on the README, so that others can find them

Sounds great. Some suggestions for NuGet tags might also help discoverability.

artsemdziaryd commented 6 months ago

Any ETA for having a stable version instead of a beta? I really like the lib.