Backports full integral type support to legacy frameworks via source generation#44
Conversation
Added support for all integral types in legacy frameworks.
…urce generation Introduces an incremental source generator for SqidsEncoder that adds overloads supporting all integral types on legacy frameworks. Updates README with usage details, adds new tests for legacy encoding/decoding, and wires up the code generation project in the solution and main project. This enables drop-in support for byte, sbyte, short, ushort, int, uint, long, and ulong encoding/decoding in .NET Standard 2.0 (.NET Framework 4.7.2+) and .NET 6 environments. Extreme care was taken to avoid structural and code changes to the SqidsEncoder class (even comments were kept where they were, as a diff will show) to make it easy to keep it up to date with the main Sqids project if a PR is not accepted.
|
This is nice. Thank you @loudenvier I like the idea, though. |
|
The tests failed due to the runner lacking NET6 and NET7 frameworks installed. All tests are passing locally.
I know the feeling! I only had time to do this because it was actually for my day job. I know pretty well how time is limited! Thank you for such nice library. It was much appreciated, |
Updated the GitHub Actions workflow to run on Windows and added support for multiple .NET versions.
|
@loudenvier The workflow you added seems to be failing, could you please take another look at it? I also just temporarily disabled this repository's approval requirement for running CI workflows in PRs to make it easier for you to test the result. |
I'm working on making it "work" and I'll merge it with the official workflow file (test.yaml). I'm only experimenting in this one. I'm not much acquainted with Github Worflows, that's why I didn't want to mess with the current one before having it working. Now I'm at my workhours here in Brazil, I'll take a look at it tonight! Don't bother with it, I'll handle it. |
Updated the .NET workflow to install multiple .NET versions and simplified the setup steps.
I've removed "my" extraneous workflow after it worked correctly, and added a workflow_dispatcher to the test.yaml one so I/we can trigger it manually, and updated it with the changes to make it also work. The changes were basically to manually install the missing frameworks and use versions v4 of the actions. Perhaps I'm doing more than is needed but the test is now completing successfully. I'm really not used to github workflows!!! I'm planning on adding a BenchmarkDotNet project that compares the original implementation with the newer one to see if any performance (in speed or memory) were actually lost. Do you think this benchmarking project belongs to this repo? Or would it be better to create it stand-alone? |
|
Excellent! Thanks again @loudenvier for the work! |
|
Sorry, I forgot to answer this question:
I do think it'd be valuable to have a benchmarking project as part of this repo, yes. |
|
Released in v3.2.0 — and here's the new NuGet package. |
2 participants
Rationale and Motivation
I was working on a multi-target project (NETSTANDARD 2.0 and NET 9.0) and needed to calculate
ulongSqids IDs but support forulongis only available on NET 7+ targets. I was faced with two options: break-down theulongvalue into twointnumbers or backport support for all integral types on legacy frameworks. Of course I choose the later (and harder) approach, why not?I soon realized that it was not straight-forward as pre-NET 7 generics constraints (or the lack of) prevents us from doing this "the clean way" (as you folks already realized by adding support only to NET 7+ targets). But I thought that I could live with custom
DecodeXXXmethods when targeting older frameworks (as this was the norm back then) and specific overloads forEncode.Goals and Strategy
I didn't want to deviate much from the current source of the
SqidsEncoderclass, and I didn't want to clutter it with boilerplate code. My initial approach was to make it a partial class and add overloads for the other integral types in another source file. While writing it I saw it was a perfect candidate for an incremental source generator. This resulted in a much cleaner solution.I also realized that I could refactor the encoding/decoding logic to use
ulongfor legacy targets with no performance hit. Refactoring it to a private method kept the library's façade still CLS-Compliant. A trick I used was to makeTan alias forulongand it helped a lot with my goal of keeping as faithful to the original code as possible.Extreme care was taken to keep the updated code as similar to the original as possible, even comments were kept where they belong so the diff only shows actual, meaningful changes. It has the side effect of making it easy for me to add any new features back to this updated code base if this PR is not accepted (which is not a problem at all!)
Implementation
It introduces an incremental source generator for SqidsEncoder that adds overloads for encoding, and custom, suffixed methods for decoding (
DecodeByte,DecodeSByte,DecodeULong, etc.), supporting all integral types on legacy frameworks.To ensure complete backward compatibility (making this a drop-in replacement), the standard
Decodemethods continue to returnint"arrays", so no changes are needed in existing codebases.Unit tests were added to ensure feature parity across all supported integral types, covering all the scenarios from the original test suite (plus additional edge cases specific to the new types).
Conclusion
This PR helps in bridging the gap between modern and legacy .NET frameworks for the Sqids library. By leveraging Source Generators, we achieve full support for all integral types (including
ulong) on .NET Standard 2.0 without sacrificing performance or code maintainability. This is a non-breaking change that significantly expands the library's utility for consumers on older runtimes while keeping the codebase clean and CLS-compliant. Next to no change was made to NET 7+ targets!I look forward to your feedback and am willing to iterate on the code to get this merged. Alternatively, if this falls outside the project's scope, would you have any objection to me publishing this as a separate NuGet package (e.g., Sqids.Legacy)?