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

feat(validation): use class-transformer to support validation of nested objects #188

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

SegaraRai
Copy link
Contributor

Types of changes

What kind of change does this PR introduce? (check at least one)

  • Breaking change
  • Bugfix
  • Feature
  • Code style update
  • Refactoring
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Description

Issue Number: N/A

class-transformerを用いてオブジェクトをインスタンスに変換することで子孫オブジェクトに対してもバリデーションが行われるようにします。
enableCircularCheck: truemultipart/form-dataでファイルが渡された際に循環参照を持つオブジェクトになるため指定しています。

BREAKING CHANGE

破壊的変更かちょっと微妙ですが、以下の2点があります。

  • ユーザーはclass-transformer及びreflect-metadataパッケージを追加する必要があります
    • class-validatorともどもpeerDependenciesにでも入れたほうが良い?
  • 追加の検証が働くようになるためもしかしたら通らなくなることがあるかもしれません

@LumaKernel
Copy link
Contributor

ありがとうございます。
バージョニングはおそらくminor(x.[here].x)になりそうです。(0.xなのでmajorともとれそうですが)
peerDependenciesは

  • frourioはそもそもランタイムを一切持たないビルド専門と考えられる
  • peerDependencies自体がoptionalかrequiredか不定みたいなところはあるが、requiredだとすると3者は必須ではないのでpeer指定はいらない

といったところでいらないかなと考えています。

@メンバー
SegaraRai さんが言うように、enableCircularCheck: true はクラスインスタンスのメソッドが循環してしまうための対処のようです。特定のクラスインスタンスをスキップするようなのも見つからなかったので、パフォーマンス求めるならそこは自分で再帰的にチェックする関数を作ることになるかもしれない。ひとまずこれは入れたい。

Copy link
Contributor

@LumaKernel LumaKernel left a comment

Choose a reason for hiding this comment

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

ありがとうございます!

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.

None yet

2 participants