longshotlabs / simpl-schema

A JavaScript schema validation package that supports direct validation of MongoDB update modifier objects
https://www.npmjs.com/package/simpl-schema
MIT License
560 stars 114 forks source link

Ensure a dependency exists when checking for errors and validity #434

Open znewsham opened 3 years ago

znewsham commented 3 years ago

Fixes #359

  1. add a new "private" function _ensureKeyDep that will add a dependency if Tracker is provided to the schema and no dependency exists.
  2. call _ensureKeyDep from keyIsInvalid and keyErrorMessage

All tests are passing

aldeed commented 3 years ago

I don't think the Meteor tests run on GitHub, and since this is a Meteor thing, I'll have to confirm this in the Meteor app. It should be possible to add tests for it in tests/meteor-app, too. My only concern is needing to look at the way that deps are marked changed, because it doesn't help to have the dep if a different dep is marked as changed. In particular, if I change a label or validate at the subschema level, is it reactive for the parent schema? Should it be?

It seems like the most elegant solution would be to have validation bubble up and down so that every schema involved is aware of what the current state is, and is reactive to it.

znewsham commented 3 years ago

I'm really nervous about bubbling - autoform (where I mostly use schema + tracker) is already a little over-reactive.

The change in this PR ensures that whatever the "base" schema is (in use by autoform) will track all dependencies within that form - as validation is performed on the validation context attached to the base schema.

If the concern is that field a is an object, and field a.b has a validation error, but you want to check to see if field a is valid - I've resolved this issue by adding a new method to my local version of ValidationContext that you can ask specifically if "a or any of it's children are invalid" - this way, in the handful of cases that I need this type of (expensive) reactivity, I have access to it - but the 95% of the places I dont need this, dont need to pay the price.

I'll look into adding meteor tests at some point when I have time, but tbh, I've pretty much made the decision to maintain my own local version of simpl-schema now, as a lot of the changes I want, need to be made to the core simpl-schema code, but don't necessarily make sense to everyone else.