fix: correctly handled mapped union types by AmadeusK525 · Pull Request #2490 · vega/ts-json-schema-generator

AmadeusK525 · 2026-03-10T12:47:03Z

Mapped types may take Unions as "parameters", and, in that case, they
must distribute over each Union individually. This was wrong before, it
was creating an intersection.

This implementation handles discriminated unions as well and preserves
the potential @discriminator annotation. Non-OpenAPI discriminator
were not tested (they generate if/then schemas).

Closes #2106

arthurfiorette · 2026-03-10T12:48:55Z

@domoritz is this change correct here? I don't have vega-lite context

I don't have vega-lite context either, but I took a look at the source code that was used to generate the types and the new schema is correct. It's correctly accounting for unions using anyOf instead of merging them into an object schema. Any validations that were previously passing should still pass with the new schema

Let's confirm. Please clone https://github.com/vega/vega-lite, link to this version of the schema generator, generate the schema, and run npm run test. If everything still passes, we are good. Could you do that to confirm that we don't have a regression in Vega-Lite?

arthurfiorette · 2026-03-10T12:50:28Z

We previously simplified the type instead of keeping the anyOf format. Should we mark this as a breaking change?

Yeah, it may be considered a breaking change, but I really don't see the gain in simplifying these Unions. I agree that allOf can be simplified into a parent merged schema, but anyOf and oneOf are good ways of representing unions (and, in my case, working with discriminated unions and validating via ajv, they're absolutely necessary).

I don't think this is a breaking change, though. It is in the sense that it changes the generated schemas by fixing them (they were wrong before), but the schemas are more correct now and if anything was previously validating but stops validating due to this change, the validation BEFORE this fix was wrong.

I agree it's a fix. Let's still make it a minor version bump, not just a patch? We might need to mark this pull request as feat: to get that behavior.

I would really like to keep the simplifications where possible. Makes schemas much more readable.

The annotations from referenced types were not always preserved. Mainly, in `preserveAnnotation`, `isAssignableTo` and `inferMap` function flows were wrongly stripping them.

Mapped types may take Unions as "parameters", and, in that case, they must distribute over each Union individually. This was wrong before, it was creating an intersection. This implementation handles discriminated unions as well and preserves the potential `@discriminator` annotation. Non-OpenAPI `discriminator` were not tested (they generate `if`/`then` schemas). Closes vega#2106

This formatter was mutating the original type by deleting the `discriminator` annotation. If the type was shared/referenced in multiple places, this would result in the annotation being lost.

This allows for projects requiring the package from GitHub to build them directly.

domoritz

Left a few comments. Looks good overall.

domoritz · 2026-03-17T14:44:06Z

Let's confirm. Please clone https://github.com/vega/vega-lite, link to this version of the schema generator, generate the schema, and run npm run test. If everything still passes, we are good. Could you do that to confirm that we don't have a regression in Vega-Lite?

domoritz · 2026-03-17T14:45:50Z

        const annotations = type.getAnnotations();
+        // Copy annotations to avoid mutating the original object, which may be shared
+        // with other AnnotatedType instances (e.g., via preserveAnnotation).
+        let restAnnotations = annotations;


This doesn't actually make a copy. You could also still use annotations. The comment about not mutating is for like 66 now, I guess?

domoritz · 2026-03-17T14:46:20Z

    insideTypes: Set<BaseType> = new Set(),
 ): boolean {
+    // Keep original source for infer map so annotations (e.g. @discriminator) are preserved
+    const originalSource = source;


Again, this is not a copy.

domoritz · 2026-03-17T14:47:53Z

I agree it's a fix. Let's still make it a minor version bump, not just a patch? We might need to mark this pull request as feat: to get that behavior.

domoritz · 2026-03-17T14:49:19Z

I would really like to keep the simplifications where possible. Makes schemas much more readable.

arthurfiorette · 2026-06-18T23:51:45Z

@AmadeusK525 do you still need the fixes on this PR?

AmadeusK525 · 2026-06-22T16:18:16Z

@arthurfiorette Hey, yes I do. Sorry, a lot of stuff came up in life and I couldn't get back to this. I'll ask someone else (who is using the fork because of this feature) to try and handle this for me. Thanks for reminding me.

arthurfiorette requested changes Mar 10, 2026

View reviewed changes

arthurfiorette assigned arthurfiorette and AmadeusK525 and unassigned arthurfiorette Mar 10, 2026

AmadeusK525 force-pushed the fix/mapped-distributed-unions branch 4 times, most recently from b887cca to fd1881e Compare March 10, 2026 16:37

AmadeusK525 added 4 commits March 16, 2026 09:12

fix: fix annotation preservation in some cases

9315395

The annotations from referenced types were not always preserved. Mainly, in `preserveAnnotation`, `isAssignableTo` and `inferMap` function flows were wrongly stripping them.

fix: don't delete annotation in AnnotatedTypeFormatter

758f556

This formatter was mutating the original type by deleting the `discriminator` annotation. If the type was shared/referenced in multiple places, this would result in the annotation being lost.

feat: add prepare script

5a6affd

This allows for projects requiring the package from GitHub to build them directly.

AmadeusK525 force-pushed the fix/mapped-distributed-unions branch from fd1881e to 5a6affd Compare March 16, 2026 13:49

domoritz reviewed Mar 17, 2026

View reviewed changes

arthurfiorette added the feedback Waiting for feedback resolution label Jun 18, 2026

Conversation

AmadeusK525 commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmadeusK525 Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domoritz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arthurfiorette commented Jun 18, 2026

Uh oh!

AmadeusK525 commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AmadeusK525 Mar 10, 2026 •

edited

Loading