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

Phil/ts codegen fix #1855

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Phil/ts codegen fix #1855

merged 2 commits into from
Jan 7, 2025

Conversation

psFried
Copy link
Member

@psFried psFried commented Jan 7, 2025

Description:

Fixes a bug in Typescript code generation when the intersection of two schemas results in an enum property that is not allowed to exist.


This change is Reviewable

We saw a derivation that failed validation because the generated Typescript
code was syntactically invalid. This was caused by the inferred `Shape` having
an empty set of enum variants (`enum_: Some([])`).  The empty enum variants
cause the generated code to omit any sort of type, which causes Deno to fail to
parse it.

I'm not completely certain whether the fault is with the Typescript code
generation or the Shape inferrence, but it's pretty easy to filter out empty
enums in the code generation, which results in generated code correctly having
the `never` type.

I also added a debug log of the generated types file, so that this type of
thing is easier to debug in the future.
@psFried psFried marked this pull request as ready for review January 7, 2025 16:01
@psFried psFried requested a review from jshearer January 7, 2025 17:52
@psFried psFried merged commit 3e35bad into master Jan 7, 2025
4 checks passed
@psFried psFried deleted the phil/ts-codegen-fix branch January 7, 2025 18:10
@psFried psFried added bug Something isn't working control-plane flowctl Issues related to the user facing CLI labels Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working control-plane flowctl Issues related to the user facing CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants