Repository navigation
fix(http): apply the merge patch transform to the base model - #12299
Open
om singhal (Om-singhaI) wants to merge 1 commit into
Open
om singhal (Om-singhaI) wants to merge 1 commit into
om singhal (Om-singhaI) wants to merge 1 commit into
Conversation
The merge patch mutator transformed the properties and indexer of a model but left the clone's baseModel pointing at the original base. For a model that uses extends, MergePatchUpdate emitted an allOf to the unpatched base, so inherited required properties stayed required and an empty patch was rejected. The base model now goes through the same mutator, so it is emitted as BaseMergePatchUpdate (or its OrCreate and ReplaceOnly variants) and the inherited properties follow the same rules as the model's own.
microsoft-github-policy-service
Bot
added
lib:http
emitter:openapi3
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Friendly-named base models can cause duplicate schema names and break previously valid OpenAPI emission.
1 open issue
What changed in this PR
Fixes #11534 by applying HTTP merge-patch transforms to base models, so inherited properties follow patch rules while preserving inheritance.
Changes:
- Transform base models using the matching merge-patch mode.
- Add inheritance, discriminator, and OpenAPI schema regression tests.
- Document the fix and diagnostic changes.
| File | Description |
|---|---|
| packages/openapi3/test/merge-patch.test.ts | Verify patched base schema references. |
| packages/http/test/merge-patch.test.ts | Cover inherited properties, discriminators, and nested transforms. |
| packages/http/src/merge-patch.ts | Transform and assign patched base models. |
| .chronus/changes/fix-merge-patch-base-model-2026-9-16-18-59-38.md | Document inheritance behavior and diagnostic changes. |
Review effort: Balanced
| if (model.baseModel) { | ||
| // Inherited properties are part of the patch document too, so the base model goes | ||
| // through the same transform instead of pointing at the original base. | ||
| const base = cachedMutateSubgraph(program, self, model.baseModel).type; |
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.
Fixes #11534
The merge patch mutator in
packages/http/src/merge-patch.tstransforms the properties and indexer of a model, but the clone keepsbaseModelpointing at the original base. So formodel Widget extends Base,MergePatchUpdate<Widget>emittedallOf: [{ $ref: Base }], inherited required properties stayed required, and{}was rejected. NestedWidgetMergePatchUpdateOrCreatehad the sameallOf.The
Modelmutator now runsbaseModelthrough the same mutator and sets the result on the clone. The base gets the matching name, such asBaseMergePatchUpdateorBaseMergePatchUpdateOrCreate, and is shared withMergePatchUpdate<Base>. Inherited properties follow the model's own rules: a requiredidbecomes optional but not nullable, optional ones also takenull, and a discriminator on the base stays required. I kept the base instead of flattening so emitters still see the inheritance and discriminator. A patched base's discriminator mapping still lists the unpatched derived models, asMergePatchUpdate<Pet>does today.Two changes to note:
@pathor other metadata property now reportsmerge-patch-contains-metadata, as the model's own properties do. On main it only got themetadata-ignoredwarning.@friendlyNamenow gets a patched copy with the same friendly name, so openapi3 reportsduplicate-type-namewhen the original base is also emitted. Nested models already hit this on main. Happy to fix that here too.Smaller output changes: a
Record<Thing>base now patches its values, and a read only inherited@key @pathno longer warns.I added three cases to
packages/http/test/merge-patch.test.ts, one topackages/openapi3/test/merge-patch.test.ts, and a changeset.Testing, built from this branch:
vitest run test/merge-patch.test.ts: 40 passed inpackages/httpand 7 inpackages/openapi3. Withsrc/merge-patch.tsfrom main, the 4 new cases fail.vitest run: 433 passed inpackages/http, 2713 inpackages/openapi3(two workers).{},{"label":"x"}and{"label":null}now validate againstWidgetPatch.