Add DateTimeOffset support to TypeScript source generator - #459
Open
aminparsa18 wants to merge 1 commit into
Open
Add DateTimeOffset support to TypeScript source generator#459aminparsa18 wants to merge 1 commit into
aminparsa18 wants to merge 1 commit into
Conversation
DateTimeOffset is not a Roslyn SpecialType (only DateTime is), and had no
dedicated check in TypeScriptMember.ConvertFromSymbol the way Guid does, so
any [MemoryPackable, GenerateTypeScript] type with a DateTimeOffset member
threw MEMPACK031 ("not supported type in typescript generation").
- TypeScriptMember.cs: map DateTimeOffset to TS `Date` via the already-cached
ReferenceSymbols.System_DateTimeOffset symbol (previously unused outside an
unrelated unmanaged-layout check), with BinaryOperationMethod = "DateTimeOffset"
so the existing templating (including the Nullable<T> unwrapping path) emits
writeDateTimeOffset/readDateTimeOffset (and nullable variants) automatically.
- TypeScriptRuntime.cs: add writeDateTimeOffset/readDateTimeOffset and their
nullable counterparts to the embedded runtime templates.
Wire format: MemoryPack registers DateTimeOffset via UnmanagedFormatter<T> (a
raw struct blit), so the byte layout is undocumented/implementation-defined.
I verified it empirically (throwaway console app dumping serialized bytes for
zero/positive/negative offsets, MinValue, MaxValue, sub-millisecond ticks) and
cross-checked it against this repo's own MemoryLayoutTest.DateTimeOffsetLayout
test: the layout is `int offsetMinutes` (4 bytes) + 4 bytes padding + the UTC
ticks with the top 2 DateTimeKind bits masked off (8 bytes) - the offset field
is a 32-bit int, not 16/64-bit as a first pass assumed, and it holds the *UTC*
ticks (not locally-adjusted ticks). Getting this wrong silently produces wrong
instants for non-UTC offsets, so I added a regression test
(MemoryLayoutTest.DateTimeOffsetWireFormatMatchesTypeScriptRuntimeAlgorithm)
that pins MemoryPackSerializer's actual byte output against the exact
algorithm the TS runtime uses, across zero/positive/negative offsets and
Min/Max/epoch values - this also guards against a runtime layout change the
same way DateTimeOffsetLayout already does for the raw struct.
Also added GenerateTypeScriptDateTimeOffset, proving a DateTimeOffset member no
longer throws MEMPACK031 and that the generated .ts calls
writeDateTimeOffset/readDateTimeOffset (and the nullable variants for
DateTimeOffset?).
Full suite (`dotnet build -c Release && dotnet test -c Release --no-build`,
matching CI) passes: 164/164.
Related: Cysharp#335 (DateOnly/TimeOnly request, same class of gap, went unanswered
and was closed by the stale-bot - this PR ships a tested fix instead of
another issue).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DateTimeis a RoslynSpecialType, soTypeScriptMember.ConvertFromSymbolspecial-cases it into aDate/writeDate/readDatemapping.DateTimeOffsetis not aSpecialType(Roslyn only special-casesDateTime), and unlikeGuidit has no dedicated named-type check. So any[MemoryPackable, GenerateTypeScript]type with aDateTimeOffsetmember currently throwsMEMPACK031("... is not supported type in typescript generation") the moment it's annotated.Related: #335 (DateOnly/TimeOnly support - same class of gap) went unanswered and was closed by the stale-bot. This PR ships a tested fix rather than another issue.
Fix
TypeScriptMember.cs: add aDateTimeOffsetbranch inConvertFromSymbol, right next to the existingGuidcheck, using theReferenceSymbols.System_DateTimeOffsetsymbol that's already cached but was only used once elsewhere (an unrelated unmanaged-struct-layout check). Maps to TSDatewithBinaryOperationMethod = "DateTimeOffset", so the existing templating - including theNullable<T>unwrapping path - automatically emitswriter.writeDateTimeOffset(...)/reader.readDateTimeOffset()(and the nullable variants forDateTimeOffset?), the same way every otherBinaryOperationMethodalready works.TypeScriptRuntime.cs: addwriteDateTimeOffset/readDateTimeOffsetand their nullable counterparts to the embedded runtime-template strings, next to the existingwriteDate/readDate.Wire format
MemoryPack registers
DateTimeOffsetviaUnmanagedFormatter<DateTimeOffset>(a raw struct blit - seeMemoryPackFormatterProvider.WellknownTypes.cs), so the byte layout isn't documented anywhere. I verified it empirically with a throwaway console app dumpingMemoryPackSerializer.Serializeoutput for zero/positive/negative offsets,MinValue,MaxValue, and sub-millisecond-tick values, and cross-checked the result against this repo's ownMemoryLayoutTest.DateTimeOffsetLayouttest.The actual layout is:
[0..4):offsetMinutesas a signed 32-bit int (little-endian)[4..8): padding[8..16): the UTC ticks (not locally-adjusted ticks), with the top 2DateTimeKindbits masked off - same trickwriteDate/readDatealready use forDateTimeMy first pass assumed a 64-bit offset field with locally-adjusted ticks, which happened to pass for zero/positive offsets but silently produced wrong instants for negative offsets - exactly the kind of bug that's easy to ship if this format is trusted from a template instead of measured. To make sure this can't regress silently, I added
MemoryLayoutTest.DateTimeOffsetWireFormatMatchesTypeScriptRuntimeAlgorithm, which pinsMemoryPackSerializer's actual byte output against the exact algorithm the TS runtime uses, across zero/positive/negative offsets and Min/Max/epoch values - this also guards against a runtime layout change the same wayDateTimeOffsetLayoutalready does for the raw struct.Testing
GeneratorDiagnosticsTest.TypeScript.cs: newGenerateTypeScriptDateTimeOffsettest proves aDateTimeOffsetmember no longer throwsMEMPACK031, and that the generated.tscontainswriteDateTimeOffset/readDateTimeOffsetcalls for a plain member and the nullable variants for aDateTimeOffset?member. The existingMEMPACK031test (usingSystem.Version, still unsupported) is untouched.MemoryLayoutTest.cs: newDateTimeOffsetWireFormatMatchesTypeScriptRuntimeAlgorithmtheory (6 cases) described above.🤖 Generated with Claude Code