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

SchemaTypeの型計算量を削減 #8332

Merged
merged 6 commits into from
Feb 19, 2022
Merged

SchemaTypeの型計算量を削減 #8332

merged 6 commits into from
Feb 19, 2022

Conversation

tamaina
Copy link
Contributor

@tamaina tamaina commented Feb 19, 2022

What

  • SchemaTypeの型計算量を削減
  • schema.tsのコメント編集

Why

  • 型計算量が多すぎて型のインスタンス化は非常に深く、無限である可能性があります。が出ている箇所があったため、ちょっと計算量を減らすように
  • 現状に即していない記述を削除, 英語化

@syuilo
Copy link
Member

syuilo commented Feb 19, 2022

エラーが118から125に増えた

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

型がちゃんと検査されているということかしら?それともこれが原因で型が間違ったものになった?

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

120に減らしてみた

@syuilo
Copy link
Member

syuilo commented Feb 19, 2022

型がちゃんと検査されているということかしら?それともこれが原因で型が間違ったものになった?

後者かと思ったけど前者かも(未確認)

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

後者だと思う

optionalまわりがまずいことになってそうだと思っている

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

まずいことになっているのを直した

@tamaina
Copy link
Contributor Author

tamaina commented Feb 19, 2022

(型計算量減らしてもダメな時はダメ)

@tamaina tamaina requested a review from syuilo February 19, 2022 13:23
@syuilo
Copy link
Member

syuilo commented Feb 19, 2022

逆に今まで問題なかったところで

Excessive stack depth comparing types 'SchemaTypeDef<?>' and 'SchemaTypeDef<?>'.ts(2321)

が出るようになってるわね(endpoints/test.tsなど)

まあafterの方がシンプルになっているしYOSASOU

@syuilo syuilo merged commit fd8f816 into develop Feb 19, 2022
@syuilo syuilo deleted the shrink-schema-type branch February 19, 2022 14:21
@syuilo
Copy link
Member

syuilo commented Feb 19, 2022

🙏🙏🙏

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