-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fundamental redesign to allow complete customisation #117
Conversation
43dff93
to
8498584
Compare
Fix duplicate test problem Add new diagnostic for duplicate assembly attributes
It doesn't really make sense that their not...
66b9502
to
d4fe612
Compare
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, |
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) |
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 @SeanFarrow<https://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 GitHub<#117 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AALDK7XCC47O265XXTJVLFTYEDFWNAVCNFSM6AAAAAA7HN4VBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGEZTAMJSHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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! |
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 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 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 🙂 |
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 @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. |
Thanks @nils-a!
Great idea!
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 🤷♂️
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 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 🙂 |
Good point. Probably "plugin creators" should maybe add some prefix or at least some relatively project-unique identifier to their templates to avoid collisions.
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.
Sounds perfect!
Sounds great. Some suggestions for NuGet tags might also help discoverability. |
Any ETA for having a stable version instead of a beta? I really like the lib. |
This PR implements the approach described in #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:Currently the library includes just four core templates:
Guid
int
long
string
Each of these templates implements the following features:
IComparable<T>
IEquatable<T>
IFormattable
ISpanFormattable
(.NET 6+)IParsable<T>
(.NET 7+)ISpanParsable<T>
(.NET 7+)System.ComponentModel.TypeConverter
System.Text.Json.Serialization.JsonConverter
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:
TEMPLATE
is the name of the templatePLACEHOLDERID
inside the template. This will be replaced with the ID name when generating the template.AdditionalFiles
orC# analyzer additional file
(depending on your IDE)For example, you could create a template called guid-efcore.typedid like this:
And then you can apply it to your IDs like this:
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
Template.Guid
etc)For example:
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: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 existIn the IDE, you can see the generator has marked this as an error:
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:
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:
StronglyTypedIdCodefix.webm
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:
guid-full
int-full
long-full
string-full
nullablestring-full
newid-full
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:guid-dapper
(Can be used in conjunction with the built-inTemplate.Guid
)guid-efcore
guid-newtonsoftjson
int-dapper
int-efcore
int-newtonsoftjson
long-dapper
long-efcore
long-newtonsoftjson
long-full
string-dapper
string-efcore
string-newtonsoftjson
string-full
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: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