Repository navigation
[typescript-fetch] Add generateAnyOfAsUnion option to model anyOf as a union type - #25394
Open
falk-stefan wants to merge 4 commits into
Open
falk-stefan wants to merge 4 commits into
falk-stefan wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 43 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
View guided diff | Re-trigger cubic
…p anyOf with own properties as interface
This branch has not been deployed
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.
Context
typescript-fetchgenerates ananyOfof$refschemas as one interface that merges the properties and therequiredlists of all members:becomes
so neither a
Catnor aDogvalue type-checks on its own. The same schema withoneOfgivesexport type PetData = Cat | Dog;. AnanyOfof primitives (number,string) gives an empty interface (#19155).Change
A new option
generateAnyOfAsUnion(defaultfalse, so existing output is unchanged). When it is on, ananyOfmodel goes through the existingoneOfpath:Cat | Dog,number | string,Array<Dog> | Cat | string), in both the separate-model andwithoutRuntimeChecksmodes;instanceOf*,FromJSONandToJSONare generated as foroneOf, with one difference that follows fromanyOfallowing several members to match: for object members, the result combines every matching member ({ ...CatToJSON(value), ...DogToJSON(value) }), so a value matching both keeps all its properties. Primitive and array members keep first match, as a value can only be one of them. AnanyOfwith a discriminator keeps the discriminatorswitchand does not combine;nullis added to the type when at least one member is nullable (foroneOfit stays exactly one, as in [typescript-fetch] Preserve nullable primitive oneOf branches #25100).If two matching members declare the same property with different types, the last matching member wins; members are not reconciled. An
anyOfschema that also declares its ownpropertiesstays an interface, so those properties aren't lost.Changes to the shared union template (also affect
oneOf)Review feedback on this PR found problems in
modelOneOf.mustachethatoneOfalready has on master. SinceanyOfnow uses the same template, they are fixed here, for both:if (typeof json === 'object'). Before,if (typeof json !== 'object') return json;returned every primitive before the declared primitive checks, which were unreachable.date/date-timemembers of a union with model members are now parsed and serialized (before, the raw value was returned).{}, as an unmatched object already did, instead of being returned unchanged. Enum members keep working:oneOfModelsis split into object and enum models, and enum members are checked outside the object block.item !== nullbefore callinginstanceOf*, so arrays containingnullno longer throw.Valid input decodes and encodes as before, apart from the date members. The
oneOfandoneOf-not-enumsamples change accordingly.Why an option and not a custom template
CONTRIBUTING asks to consider
-tbefore adding an option. That isn't enough here: the member properties are merged inDefaultCodegenbefore templates run, andoneOfModels,oneOfArrays,oneOfPrimitivesand the import filtering inTypeScriptFetchClientCodegenare computed fromoneOfonly, so a template override cannot produce the union, its imports or its (de)serializers. Changing the default would break every existinganyOfuser, so it is opt-in, likegenerateOneOfAnyOfWrappersin the Kotlin generator.Tests
TypeScriptFetchClientCodegenTest: default output unchanged (merged interface); with the option on, unions for a top-levelanyOf, an inlineanyOfproperty, a primitiveanyOf, nullable members, a discriminatedanyOfand a model/array/primitive mix, in both runtime-check modes; combinedFromJSON/ToJSONfor a value matching several members; ananyOfwith own properties stays an interface;oneOfoutput is the same with the option on and off; null array items, unreachable-primitive and enum/date-time member cases foroneOfandanyOf. Spec:src/test/resources/3_0/typescript-fetch/anyOf.yaml.bin/configs/typescript-fetch-any-of-union.yaml→samples/client/petstore/typescript-fetch/builds/any-of-union, which type-checks withtsc --noEmit. All samples were regenerated; besides the new one, only theoneOfandoneOf-not-enumtypescript-fetch samples change (template changes above).Related: #6376, #19155, #12432, #8894
PR checklist
cc @macjohnny @joscha