-
-
Notifications
You must be signed in to change notification settings - Fork 528
Fix Record<string, never> appearing in discriminator union #1248
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
Conversation
🦋 Changeset detectedLatest commit: d233633 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 |
11e4fcf
to
69fc7ef
Compare
Deploying with
|
Latest commit: |
d233633
|
Status: | ✅ Deploy successful! |
Preview URL: | https://c96ff172.openapi-ts.pages.dev |
Branch Preview URL: | https://no-empty-object-union.openapi-ts.pages.dev |
69fc7ef
to
358c55c
Compare
@@ -4,8 +4,8 @@ module.exports = { | |||
parserOptions: { | |||
project: ["./tsconfig.json"], | |||
}, | |||
extends: ["eslint:recommended", "plugin:@typescript-eslint/strict"], | |||
plugins: ["@typescript-eslint", "prettier"], | |||
extends: ["eslint:recommended", "plugin:@typescript-eslint/strict", "plugin:vitest/recommended"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor linting improvements to test files (mostly just catches test.only()
from lingering, which can be easy to forget to remove)
358c55c
to
f86107d
Compare
@@ -34143,7 +34143,7 @@ export interface components { | |||
public?: boolean; | |||
/** Format: uri-template */ | |||
pulls_url: string; | |||
pushed_at: number | string; | |||
pushed_at: number | string | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 This is a bugfix
@@ -73010,7 +73010,7 @@ export interface components { | |||
files?: string[]; | |||
bin?: Record<string, never>; | |||
man?: Record<string, never>; | |||
directories?: string | Record<string, never>; | |||
directories?: string | Record<string, never> | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong, but is generated from:
oneOf:
- type: string
- type: object
nullable: true
This is correct, at least as long as we generate Record<string, never>
for empty objects (with no additionaalProperties)
f86107d
to
5d11dea
Compare
5d11dea
to
d233633
Compare
Changes
If a discriminator has
type: "object"
in it, it would completely change the output.Unfortunately, fixing this did cause some⚠️ minor breaking changes in generated types. But I’m hopeful the change is an improvement and not a regression, as the new types should yield better TS inference.
How to Review
Tests should pass.
Checklist
examples/
directory updated (only applicable for openapi-typescript)