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

Output empty object response body as {} type instead of void type #250

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mashabow
Copy link
Contributor

@mashabow mashabow commented Oct 31, 2023

Types of changes

  • Features
    • resolves #
  • Bug fixes
    • resolves #
  • Maintenance
  • Documentation

Changes

I found that when we have an empty object for the response body, it is outputted as void type instead of {} type. So, I fixed it.

Since I teaked schema2value, it might affect other parts beyond just the response body. However, based on the existing tests, everything seems okay.

レスポンスボディが空オブジェクトのとき、{} 型ではなく void 型で出力される場合があるようなので、修正してみました。

schema2value をいじったので、レスポンスボディだけでなく他の部分にも影響すると思いますが、既存のテストを見るかぎりでは悪影響はなさそうです。

Additional context

Community note

Please upvote with reacting as 👍 to express your agreement.

Comment on lines 18 to 23
resBody: {
limit: number
offset: number
data: {
}[]
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この部分のスキーマは以下のようになっていました。

"schema": {
"required": ["data", "limit", "offset"],
"properties": {
"limit": { "type": "number" },
"offset": { "type": "number" },
"data": { "type": "array", "items": { "type": "object" } }
}
}

ここは「レスポンスボディが空オブジェクトだった場合」でなく「レスポンスボディの一部分が空オブジェクトだった場合」ですが、修正後の {}[] の方が正しく型を表しています。

@@ -123,11 +123,11 @@ export const schema2value = (
} else if (isArraySchema(schema)) {
isArray = true;
value = schema2value(schema.items);
} else if (schema.properties || schema.additionalProperties) {
} else if (schema.type === 'object' || schema.properties || 'additionalProperties' in schema) {
Copy link

@mikana0918 mikana0918 Jan 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (schema.type === 'object' || schema.properties || 'additionalProperties' in schema) {
} else if (schema.type === 'object' || schema.properties || schema.additionalProperties) {

@mashabow PRの確認遅くなっておりすみません。こちら他の箇所と同様に schema.additionalPropertiesとするのはいかがでしょうか? (それに関して問題等ありそうでしょうか。)もし特段の理由がなければ、ドットでのアクセスのほうが周りのコードと乖離がなく、読む際に迷いもなくなるので、いいかな?と思いました。

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.

2 participants