Skip to content

fix(core): serialize format: date as a calendar date - #7

Merged
barney-ws merged 3 commits into
mainfrom
fix/date-format-serialization
Aug 17, 2026
Merged

barney-ws merged 3 commits into
mainfrom
fix/date-format-serialization

Conversation

@barney-ws

Copy link
Copy Markdown
Collaborator

Problem

format: date maps to DateTime, the same as format: date-time, and json_serializable then encodes it with toIso8601String(). So a field the spec declares as a calendar date goes out as a full timestamp:

fromDate:
  type: string
  format: date
  pattern: '^\d{4}-\d{2}-\d{2}$'   # what the server actually validates
// generated
'fromDate': instance.fromDate.toIso8601String(),   // "2026-05-12T00:00:00.000"

The server rejects it as an invalid ISO date. We hit this on a PDF export endpoint whose fromDate / toDate query parameters are format: date; the same spec has date fields on request bodies and an array of them in a payload object, so it is not limited to query strings.

Change

Date properties and date query parameters get a _DateOnlyConverter, emitted into the files that need one:

class _DateOnlyConverter implements JsonConverter<DateTime, String> {
  const _DateOnlyConverter();

  @override
  DateTime fromJson(String json) => DateTime.parse(json);

  @override
  String toJson(DateTime date) =>
      '${date.year.toString().padLeft(4, '0')}-'
      '${date.month.toString().padLeft(2, '0')}-'
      '${date.day.toString().padLeft(2, '0')}';
}

json_serializable applies a JsonConverter to nullable fields and to lists by itself, so this one covers DateTime, DateTime? and List<DateTime> — I checked the generated .g.dart rather than assuming it.

DateTime stays the Dart type either way, so this only changes the encoding — nothing that reads these fields today has to change. date-time is untouched.

One plumbing fix went with it: query parameters lost their schema format on the way into the params generator, so the annotation could never have reached them.

Verification

Generated a spec covering required / nullable / list format: date, a date-time control and a format: date query parameter, then ran build_runner over the output and encoded it for real:

model  -> {"date":"2026-05-12","maybeDate":"2026-01-02","dates":["2026-03-04"],"createdAt":"2026-05-12T09:30:00.000Z"}
params -> {"fromDate":"2026-05-12","at":"2026-05-12T09:30:00.000Z"}
decode -> 2026-05-12 00:00:00.000 | [2026-03-04 00:00:00.000]

Dates come out as YYYY-MM-DD, date-time keeps its timestamp, and decoding still works.

5 tests added in models-date-format.test.ts, covering the annotation on required / nullable / list properties, date-time staying unannotated, no converter emitted for models without date fields, and the query-parameter path. packages/core: 40 files / 413 tests pass.

Notes

This is the third defect of the same kind, after numeric enums (#4) and boolean enums (#6): the Dart type is right, but the spec's format is lost by the time the value is serialized. All three are invisible to dart analyze — the generated code compiles and sends the wrong thing — which is the case for the round-trip assertions discussed on #4.

The 3 pre-existing no-unused-vars lint errors in headers-generator.* and getters/body.ts are still there, untouched.

A `format: date` field maps to DateTime, the same as `date-time`, and
json_serializable then encodes it with `toIso8601String()`. So a spec that
asks for a calendar date receives `2026-05-12T00:00:00.000` and rejects it -
these fields usually carry a `^\d{4}-\d{2}-\d{2}$` pattern.

Date properties and date query parameters now carry a `_DateOnlyConverter`,
emitted into the files that need one. json_serializable applies a
JsonConverter to nullable fields and to lists on its own, so the one
converter covers `DateTime`, `DateTime?` and `List<DateTime>`. `date-time`
is untouched, and DateTime stays the Dart type either way, so only the
encoding changes.

Query parameters needed the schema format threaded through to the params
generator, which had been dropping it.
@barney-ws barney-ws added the bug Something isn't working label Aug 13, 2026

@qwlong qwlong left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Built the branch (413 tests pass), generated from a spec covering every shape this touches, and put the output through freezed 3.x + json_serializable 6.14 on Dart 3.9. The fix is correct and the claims in the description hold up under checking.

Verified

The converter reaches everything it should. Generating a model with required / nullable / list date properties, a date-time control, an inline nested object, and date path / query / header parameters:

Location @_DateOnlyConverter()
required DateTime date
DateTime? maybeDate
List<DateTime>? dates
inline nested object property ✅ (via the getters/object.ts path)
query parameter
DateTime? createdAt (date-time) ✅ correctly untouched

The nullable/list claim is real. The generated .g.dart confirms json_serializable wraps a non-nullable JsonConverter on its own rather than rejecting it:

date: const _DateOnlyConverter().fromJson(json['date'] as String),
dates: (json['dates'] as List<dynamic>?)
    ?.map((e) => const _DateOnlyConverter().fromJson(e as String)),
'dates': instance.dates?.map(const _DateOnlyConverter().toJson).toList(),
'createdAt': instance.createdAt?.toIso8601String(),

Worth calling out given the last two PRs — this is exactly the layer that rejected @JsonValue(1.5) and @JsonValue(true) on #6, and here it accepts what's being generated. Checking the .g.dart rather than assuming was the right instinct.

Timezones are handled correctly. date.year/.month/.day are frame-dependent, so I went after this specifically. Dart's getters respect the isUtc flag, and the round trip that matters — server "2026-05-12"fromJsontoJson — is stable under TZ=UTC, Pacific/Kiritimati (+14), Pacific/Midway (-11) and America/New_York, because DateTime.parse on a bare date yields a local, non-UTC value whose components can't drift. DateTime.utc(2026,5,12) also reports 2026-05-12 in every zone. The only case that shifts is a caller explicitly doing .toLocal() on a UTC instant across a date boundary, which is the caller moving the date, not the converter. padLeft(4) handles year < 1000 (0999-01-02).

One gap: header parameters

case 'query' carries the format now, but case 'header' right below it doesn't, and HeaderParameter has no format field. Details inline — same defect, same switch statement, not fixed.

Path parameters are genuinely fine, for the record: format: date there generates as String and is interpolated directly, so there's no DateTime to mis-encode.

Minor

The converter is emitted into every file that needs it — 3 copies in my sample. It's library-private per file so there's no conflict, and the build confirms it, but a shared models/date_only_converter.dart would avoid the duplication if you'd rather have one definition. Purely a style call.

On the framing in the description

Agreed this is the third instance of the same class, though it's worth separating them: numeric enums (#4) were a genuine wrong-wire-format bug like this one, whereas the boolean branch on #6 can't reach the wire at all — json_serializable rejects @JsonValue(true) at build time. This PR is the first of the three where the generated code both compiles and round-trips correctly end to end, which the round-trip assertions in the description actually demonstrate. That's the bar the CI changes discussed on #4 would enforce; main still has || true on build_runner, dart analyze and dart test, so none of this is being checked automatically yet.

Comment thread packages/core/src/generators/endpoint-generator.ts Outdated
barney-ws and others added 2 commits August 13, 2026 16:33
The query parameter path read `format` straight off the parameter schema, so
it only ever matched a scalar `format: date`. An array of dates - a multi-date
filter, `type: array` with `items: { format: date }` - has no `format` of its
own, so `List<DateTime>` query parameters were left with the default DateTime
encoding and still sent full timestamps.

Both paths now decide the same way, through `TypeMapper.isDateOnlySchema`,
which already unwrapped arrays for model properties. `QueryParameter` carries
the resulting `dateOnly` flag rather than a raw `format` string, so there is
one place left that knows what makes a value a calendar date.

That predicate no longer returns early on `items`, which stopped a schema with
both `items` and `oneOf` from having its union checked, and it takes a typed
schema instead of `any`. A `$ref` returns false explicitly: a scalar component
schema generates a class of its own today rather than a DateTime field, so
there is nothing for the converter to annotate.

The converter itself moves into a `date-only-converter.hbs` partial, shared by
the model and params templates instead of being copied into both, and its doc
comment now says that the time of day is dropped and that the calendar date is
read in whatever zone the DateTime carries.

Verified against Dart: generated a spec with scalar, repeated and `date-time`
query parameters, ran build_runner over the output and encoded it.

  toJson  -> {"fromDate":"2026-05-12","days":["2026-03-04","2026-12-31"],"at":"2026-05-12T09:30:00.000Z"}
  query   -> {fromDate: 2026-05-12, days[0]: 2026-03-04, days[1]: 2026-12-31, at: 2026-05-12T09:30:00.000Z}
  decode  -> 2026-05-12 00:00:00.000 | [2026-03-04 00:00:00.000] | 2026-05-12 09:30:00.000Z

`dart analyze` is clean on the result. 6 tests added: the repeated query
parameter, a model with no date parameters, a nested inline object (the
getters/object.ts path, which had the plumbing but no coverage) and three for
`isDateOnlySchema` itself. The query parameter tests now run through
EndpointGenerator rather than hand-built parameter lists, so they cover the
plumbing that dropped the format in the first place. packages/core: 40 files /
419 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `format: date` header parameter mapped to DateTime like the properties and
query parameters did, and nothing annotated it, so `X-Effective-Date` went out
as `2026-05-12T00:00:00.000`. Headers were the one path of the three left with
the default DateTime encoding.

`HeaderParameter` now carries the same `dateOnly` flag, decided by the same
`isDateOnlySchema`, and both renderers that feed the headers template pass it
through - the per-endpoint one in ParamsGenerator and the consolidating one in
HeadersGenerator. Verified against Dart, where the header used to carry a
timestamp:

  headers -> {X-Effective-Date: 2026-05-12, X-Tenant: acme}

Two smaller things in the same area:

- `generateJsonKeyAnnotation` still asked for `fromJson: _dateTimeFromJson,
  toJson: _dateTimeToJson`. Those helpers only ever existed in the model
  template, behind a flag hardcoded to false, and this branch deleted them, so
  the annotation named functions that no longer exist anywhere. Nothing in the
  repo calls the method - it is a public static, so the branch goes rather than
  the method itself, which would be an API break.
- The converter's doc comment read as though `DateTime(2026, 5, 13).toUtc()`
  demonstrated the advice above it, when it is the counter-example.

packages/core: 40 files / 421 tests pass. `dart analyze` clean on the generated
headers model.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@qwlong qwlong left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 26a6c9c8 and 8db5f215

Rebuilt, 421 tests passing, and generated a full API then built it with freezed 3.x + json_serializable 6.14 on Dart 3.9. The header gap is closed, and the two commits fixed more than I'd reported.

Headers

Both renderers are covered, and the annotation survives all the way into the generated serializer:

// models/headers/export_pdf_headers.f.dart
@_DateOnlyConverter()
required DateTime xEffectiveDate,
required String xTenant,

// export_pdf_headers.f.g.dart
'X-Effective-Date': const _DateOnlyConverter().toJson(instance.xEffectiveDate),

Covering HeadersGenerator as well as ParamsGenerator is the part I'd have missed — I only ever exercised the default path, so a fix to that one alone would have looked complete from where I was standing.

The array query parameter bug I walked past

Replacing format?: string with dateOnly?: boolean computed through TypeMapper.isDateOnlySchema(p.schema) fixes something my last review didn't catch: the old param.format === 'date' only ever saw a scalar's own format, so an array of dates never matched. My check used a scalar query parameter and therefore missed it entirely. Now:

@_DateOnlyConverter()
List<DateTime>? days,

// .g.dart
'days': instance.days?.map(const _DateOnlyConverter().toJson).toList(),

Same for isDateOnlySchema(propDetails)(propSchema) in model-generator.ts — that one was reading a shape that had already lost the nesting.

Also verified

  • The converter actually reaches the wire for query parameters. An annotation json_serializable honours is worth nothing if the query path sidesteps toJson(), so I checked: toQueryParameters() opens with final json = toJson(); and flattens from there, and the service passes its result straight to Dio. It applies.
  • $ref returning false is correctly reasoned. A $ref to a scalar {type: string, format: date} component generates class CalDate and the property comes out CalDate?, not DateTime — so there's no DateTime to annotate and nothing is lost by not following the ref, exactly as the comment claims.
  • Everything builds. Model, nested inline object, params and headers together: build_runner wrote all outputs, dart analyze reports No issues found.
  • Extracting the converter into a shared partial removes the per-file duplication I'd noted, and the doc comment now spells out the timezone caveat — including the DateTime(2026, 5, 13).toUtc() case, which is the one that will actually bite someone.

Nothing further from me on this one.

@barney-ws

Copy link
Copy Markdown
Collaborator Author

So is it ready to merge? @qwlong

@barney-ws
barney-ws merged commit 51d79de into main Aug 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants