Skip to content

Review of api/validators #62659

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

Open
wants to merge 8 commits into
base: prototype/signal-forms
Choose a base branch
from

Conversation

mmalerba
Copy link
Contributor

@mmalerba mmalerba commented Jul 16, 2025

See individual commits which address the TODOs I initially commented

@mmalerba mmalerba requested a review from kirjs July 16, 2025 00:49
@ngbot ngbot bot added this to the Backlog milestone Jul 16, 2025
@mmalerba mmalerba requested a review from leonsenft July 16, 2025 00:49
@mmalerba mmalerba force-pushed the review/api/validators branch from f80ee5a to 22b7174 Compare July 16, 2025 03:49
mmalerba added 3 commits July 16, 2025 22:10
Improves the ergonomics of the `errors` option offered by the standard
validators.
1. Renames it to `error` because in the majority of cases it will be
   used to return a single `ValidationError`.
2. Allows directly using a constant `ValidationError` insetad of having
   to create a function like `() => ValidationError.custom()`
@mmalerba mmalerba force-pushed the review/api/validators branch from f04b1ed to ed163e7 Compare July 17, 2025 00:02
@mmalerba mmalerba marked this pull request as ready for review July 17, 2025 00:04
@mmalerba mmalerba force-pushed the review/api/validators branch from 5599dfb to 9f2fb96 Compare July 17, 2025 16:45
@mmalerba
Copy link
Contributor Author

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request focuses on refactoring the forms validators API to enhance consistency and reactivity. Key changes include renaming minlength/maxlength to camelCase, standardizing validator configuration with an error property, and updating most validators to use a more reactive implementation with define and computed. The JSDoc and test suites have also been updated accordingly.

@mmalerba mmalerba force-pushed the review/api/validators branch from 9f2fb96 to 63929b2 Compare July 17, 2025 17:04
@mmalerba mmalerba requested a review from leonsenft July 17, 2025 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants