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

Argument Validation Primitives #694

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Argument Validation Primitives #694

wants to merge 2 commits into from

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Dec 26, 2020

Big thanks to @snewcomer and @rwjblue for helping with the initial design!

Rendered

Copy link
Contributor

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

This is a wonderful, well written RFC. Surely seems like the right direction.

lazy, and so we cannot know the values until they are used.

While `validateArgs` will be autotracked on its own, it will not be _consumed_
by the external tracking contexts. This prevents it from impacting the tracking
Copy link
Contributor

Choose a reason for hiding this comment

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

it will not be consumed by the external tracking contexts

How is this guaranteed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the ability to disable autotracking contexts internally, and use it for various reasons (such as backwards compatibility). We have not exposed this capability, and generally don't think we will do so as in most cases it doesn't make sense, but I think here since this is a DEBUG only feature it may make sense. It does mean that validations may not rerun when you think they would, but it also prevents behavior from diverging too much in production.

This method is only run in development builds, and is inert in production
builds.

### `setValidateArgs` documentation
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like this. Similar to composing validations on changesets.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

Is there still interest in moving this forward?

@chriskrycho chriskrycho added the S-Proposed In the Proposed Stage label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Proposed In the Proposed Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants