fix: empty strings in enums when type [string, null] #1200
Conversation
🦋 Changeset detectedLatest commit: 3505067 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -51,13 +51,13 @@ export function defaultSchemaObjectTransform(schemaObject: SchemaObject | Refere | |||
| if (schemaObject.enum) { | |||
| let items = schemaObject.enum as any[]; | |||
| if ("type" in schemaObject) { | |||
| if (schemaObject.type === "string" || (Array.isArray(schemaObject.type) && schemaObject.type.includes("string"))) items = items.map((t) => escStr(t || "")); // empty/missing values are empty strings | |||
| if (schemaObject.type === "string" || (Array.isArray(schemaObject.type) && schemaObject.type.includes("string"))) items = items.map((t) => escStr(t)); // empty/missing values are empty strings | |||
There was a problem hiding this comment.
👍 Nice cleanup here. I think this was a byproduct of being written before tsUnionOf() deduplicated things.
| expect(generated).toBe(`"" | "false positive" | "won't fix" | "used in tests"`); | ||
| }); | ||
|
|
||
| test("enum + polymorphism + nullable 3", () => { |
|
Could you add a changeset so the changelog is updated (details in comment)? 🙏 |
toomuchdesign
force-pushed
the
fix/polimorphism-empty-strings
branch
from
cb44f01 to
1fd4b0b
Compare
|
Thank you @drwpow! Cheers! |
| @@ -278,9 +278,19 @@ describe("Schema Object", () => { | |||
| expect(generated).toBe("OneOf<[string, boolean, number, null]>"); | |||
| }); | |||
|
|
|||
| test("enum + polymorphism + nullable", () => { | |||
| const generated = transformSchemaObject({ type: ["string", "null"], enum: ["", "false positive", "won't fix", "used in tests"] }, options); | |||
| expect(generated).toBe(`"" | "false positive" | "won't fix" | "used in tests" | null`); | |||
There was a problem hiding this comment.
Yeah thanks for changing these. I don’t think the old strings were meaningful / helpful.
|
The test failures may be from the snapshots failing. In the The diff that shows is also helpful to see what it does on large real-world schemas, and they’re helpful in addition to the ever-essential unit tests. |
|
All updated 👍 I've seen a few |
Yup those look like good diffs to me 🙂 |
Changes
Related to: #1197
JSON schemas like:
{ "properties": { "petType": { "type": [ "null", "string" ], "enum": [ "foo", "bar" ] } } }..results into:
...which seems to add an unexpected empty string to the union. See the example in this JSON schema validator.
With this PR the empty
""is removed from the union:Happy to review the PR based on your suggestions!
nullnot listed as enumIn case type contains
nullbut thennullis not listed as enum:{ "properties": { "petType": { "type": [ "null", "string" ], "enum": [ "foo", "bar" ] } } }..I understand the result should not contain
null. See this online validator test case.How to Review
Take a look at the new test cases added. The fix didn't introduce any regressive test error.
Checklist
examples/directory updated (only applicable for openapi-typescript)