refactor: open enums with lax client - #2760
Draft
mariechatfield wants to merge 4 commits into
Draft
Conversation
Same lax-mode enum/union parsing as rc.4, but rc.3 also relaxes date parsing, which reintroduces the RFCDate->Date migration the rc.4 branch (#2759) doesn't need. This branch exists to compare that cost against rc.4's strict-dates variant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Lax mode drops the RFCDate wrapper on date-only fields in favor of plain Date, parsed off the wire at UTC midnight (Speakeasy's types.date()). Outbound sites just need the Date passed through instead of wrapped. Inbound sites are the real risk: several read the API's Date back out via .toString() (safe under the old RFCDate, which always round-tripped through the same UTC frame) or local Date components, both of which silently roll the calendar day back by one west of UTC. Fixed with a new formatWireDateToStringDate helper (dateFormatting.ts) for the UTC-safe read, alongside the existing local-midnight helpers for outbound use. Found and fixed real latent bugs beyond the mechanical RFCDate swap: getPendingFutureHomeAddress's sort/filter and several HomeAddressView display sites were comparing/parsing Date.toString() output, not a YYYY-MM-DD string, once the field type changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Drift from the RFCDate->Date migration that the pre-commit hook missed (it only formats staged files at commit time) but CI's format:check catches.
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.
Summary
Stacked on #2759 (which installs rc.4 -- lax enums/unions, strict dates). This branch swaps that for
0.3.0-rc.3, which is lax on dates too, and adds only what that costs: the RFCDate->Date migration.Base is
refactor/marie/SDK-1307-rc4, notmain, on purpose -- the diff here is deliberately just the incremental cost of choosing rc.3 over rc.4:63a24fbe-- bump to rc.3.29d922ac-- the RFCDate->Date migration this reintroduces (22 files). Includes one inbound-parsing gotcha worth flagging in review: reading a wire date back with localDatecomponents (.toString(),new Date(str)) rather than throughformatWireDateToStringDatesilently shifts the day in some timezones -- see that helper's TSDoc insrc/helpers/dateFormatting.ts.Everything else (open-enum narrowing, the discriminated-union guard, the exhaustiveness fallback) is identical to #2759 and not repeated here -- review those there.
Why this exists
#2759's description frames the decision as rc.4-strict-dates vs. rc.3-lax-dates. This PR makes the rc.3 side of that comparison concrete: it's the RFCDate migration, and nothing else. If the team decides rc.4 is the way to go, this branch/PR can just be closed without merging.
A concrete case for why dates and null-tolerance aren't separable
Observed a live example:
PUT .../employees/:id/pay_schedulecan returntransition_pay_period: null, even though the OAS schema says that field is either a full object or absent -- nevernull. That's the same crash class SDK-1307 exists to fix, just a different shape (a structural null, not an unrecognized enum/union member).Checked both packages directly to see whether rc.4 (strict dates) still catches this:
laxMode: lax): the field's generated schema istypes.optional(...), and the SDK's owntypes.optional()helper (src/types/primitives.ts) unions in az.null().transform(() => undefined)branch. A literalnullparses cleanly toundefined.laxMode: strict): same field, plain Zod.optional()-- tolerates a missing key, not an explicitnull. This throwsResponseValidationError, uncaught, same as before any of this work started.laxModeis a single gen.yaml flag (lax/strict), unlikeforwardCompatibleEnumsByDefault/forwardCompatibleUnionsByDefault, which are separate flags -- that's specifically why rc.4 can keep lenient enums/unions while reverting to strict dates. There's no equivalent scoped flag for "tolerate null on an optional-but-never-null field" independent of dates -- confirmed by diffing the two packages' generated code, not just going on the changelog description. If we want this class of null-handling crash covered by the generator (rather than defensively coded per call site), that requireslaxMode: lax-- i.e. rc.3, with the RFCDate migration in this PR as the cost.Worth noting for scope: our SDK code shows zero type changes either way, because the exported TS type (
transitionPayPeriod?: PayScheduleAssignmentTransitionPayPeriod | undefined) is byte-identical between rc.3 and rc.4 -- neither ever declared| null. The leniency is a runtime-only difference, invisible totsc, sotscclean on rc.4 does not mean rc.4 is safe from this crash.Test plan
tsc --noEmitclean (0 errors) on this branch against rc.3PayrollConfiguration.test.tsxgross-up,PaymentFlow.test.tsx,PayrollLanding.test.tsx-- all pass in isolation)eslintclean (only pre-existing, unrelated warnings)🤖 Generated with Claude Code