Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add supportsOptionalClassProperties to TypeScriptZod target language #2478

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

inferrinizzard
Copy link
Contributor

@inferrinizzard inferrinizzard commented Jan 24, 2024

Summary

Added supportsOptionalClassProperties to TypeScriptZodTargetLanguage in order to fix handling of optional properties in JSON Schema that are not listed in required array.

JSON Schema

{
  "properties": {
    "foo": { "type": "string" },           // standard string
    "bar": { "type": ["string", "null"] }, // string | null union
    "baz": { "type": "string" },           // string, optional
    "qux": { "type": ["string", "null"] }, // string | null union, optional
  },
  "required": ["foo", "bar"]
}

Old

export const Schema = z.object({
  foo: z.string(),                               // standard string
  bar: z.union(z.string(), z.null()),            // string | null union
  baz: z.union(z.string(), z.null()).optional(), // string, optional
  qux: z.union(z.string(), z.null()).optional(), // string | null union, optional
})

New

export const Schema = z.object({
  foo: z.string(),                               // standard string
  bar: z.union(z.string(), z.null()),            // string | null union
  baz: z.string().optional(),                    // string, optional
  qux: z.union(z.string(), z.null()).optional(), // string | null union, optional
})

@inferrinizzard
Copy link
Contributor Author

CI Test failure looks like #2462

@dvdsgl
Copy link
Member

dvdsgl commented Jan 27, 2024

Yes, sadly our CI is currently haunted.

@inferrinizzard
Copy link
Contributor Author

@dvdsgl Are you guys looking for more maintainers? Would like to help out if there's space available

@dvdsgl
Copy link
Member

dvdsgl commented Feb 13, 2024

Yes please!

We're getting desperate: https://replit.com/bounties/@dvdsgl/fix-our-oss-project

Want to see if you can fix CI? I'll pay you the bounty.

@Adam724
Copy link
Contributor

Adam724 commented Feb 14, 2024

Hello. I believe this PR should fix #2462. The failing tests are now passing locally for me on that branch.

@dvdsgl dvdsgl merged commit fdd4052 into glideapps:master Feb 14, 2024
24 checks passed
@inferrinizzard
Copy link
Contributor Author

@dvdsgl great to hear that the CI issue has finally been fixed - is there a link to the maintainer slack (or is that defunct?)
Would like to chat to some folks to see which areas of the codebase need more effort before jumping on fixing more issues.

@dvdsgl
Copy link
Member

dvdsgl commented Feb 15, 2024

We don't have a discourse or slack anymore, but we could make a WhatsApp thingy for now: https://chat.whatsapp.com/E3Heqe9Tjl8IBVqpeFMyPp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants