kettanaito / react-advanced-form

Functional reactive forms. Multi-layer validation, custom styling, field grouping, reactive props, and much more.
https://redd.gitbook.io/react-advanced-form
MIT License
217 stars 24 forks source link

Resolve validation rule when the referenced field(s) updates #197

Closed kettanaito closed 6 years ago

kettanaito commented 6 years ago

Branch: dev/rule-subscription

Environment

What

We need to cover the scenario when the validation rule is referencing another fields.

Why

Validation rule with the fields reference doesn't resolve when the reference field(s) updates.

How

Validation rules:

export default {
  name: {
    confirmPassword: {
      matches: ({ value, fields }) => {
        return (value === fields.userPassword.value);
      }
    }
  }
};

Steps to reproduce:

  1. Fill something into userPasswordfield.
  2. Fill the same value into confirmPasswordfield.
  3. See confirmPassword field as valid, since the values are equal.
  4. Change the value of userPassword, without touching the other field.
  5. See confirmPassword field still valid, although the values do not match anymore.

Possible solution

kettanaito commented 6 years ago

Tightly related to #172, as the latter changes the interface used for fields subscriptions.

kettanaito commented 6 years ago

In order to resolve the validation rule in real time using the subscription system, the rule resolver needs to use that subscribe function introduce by RxProps change.

That is okay, however, that makes the usage of fields quite limited. Up to useless, I would say.

kettanaito commented 6 years ago

There is also an option to limit the depth level of groups to just one (forbid nested groups, which are, in fact, not allow in the moment). Therefore, knowing the field position in the Object Proxy is more predictable.

What I don't like about proxying is that we need to iterate over multiple levels of objects to proxy proper parts. Current solution with subscribe has no iteration at all, which I favor and like.

kettanaito commented 6 years ago

Continue on the topic of fields proxy, I've got an idea about recursive proxying without any iteration. To my surprise, that works really good and the only obstacle I am facing now is that I'm uncertain how to resolve the initial value of the resolver function, since deep reference of the non-existing keys throws. I would like for it to return undefined instead. In the worse case, it returns an instance of Proxy.

Recursive proxy

The proof of concept of recursive proxy is quite simple:

function proxy(origin, callback) {
  return new Proxy(origin, {
    get: (target, propName) => {
      const propValue = target[propName];
      if (callback) callback(propName);

      return proxy({}, callback);
    }
  });
}

function analyzeMethod(method) {
  const refPath = [];
  const fields = { fieldOne: {} };
  const proxiedFields = proxy(fields, propName => refPath.push(propName));

  method({ fields: proxiedFields });
  console.log('ref path', refPath);
}

analyzeMethod(({ fields }) => {
  const foo = fields.groupName.fieldName.propName;
  fields.groupTwo.pooper;
  return foo ? true : false;
});

Codepen source

The analyzeMethod performs what its name states, and returns the array of key refs mentiond in the provided method.

Caveats

kettanaito commented 6 years ago

Update

The separation of the field refs was acheived by adding a temporary property to the root fields Object. The presence of that property states that the current target is a root level Object, allowing to push it to the different array entry.

kettanaito commented 6 years ago

Update

Resolving initial value with recursive Proxy turned out to be easier than I thought.

Since it's possible to collect the array of field path refs, it's then possible to ensure those refs are set to undefined, in case missing, on fields immutable Map. The drawback is, the resolver function is executed twice:

1) To compose the collection of field path refs 2) To resolve the initial value using "safe" fields

kettanaito commented 6 years ago

Interface changes merged under #206 🎉

Now it should be much easier to integrate subscriptions into validation rules, since the resolver function interface didn't change.

kettanaito commented 6 years ago

Started the implementation of the actual issue.

Isolated the live method call based on ref subscription, changed the implementation of existing RxProps functionality.

As it turned out, the subscription based on the rule must be created just once. Hence, validateSync isn't the right place to do so. It must be handled upon the field's registration, so to not create redundant observers for the fields which are not mounted, but which rules are present in the validation schema.

Update 11.03

A clear algorithm must be defined.

  1. Analyze the existing type/name rules relative to the registered field.
  2. In case the rule(s) is present, make it reactive, collecting the dependencies of its resolver.
  3. As a subscribe handler, an appropriate validation type must be triggered. Calling just form.validateField() would be wrong (is it?)
  4. Keep created observables somewhere to prevent creating multiple observables for the same rule/field pair.
kettanaito commented 6 years ago

The obstacle I am facing at the moment is that at the rule dispatch point (in fieldUtils.validateSync) the fields Object contains no safe/proxied values. Therefore using new reference interface throws.

Since rules are not meant to change during the runtime of the application, it should be safe to analuze the rule's dependencies (references) and store them. Alternatively, even construct the ruleArgs Object with proxied fields, and use that args whenever the rule is dispatched.

Another option would be to ensafe the fields each time the rule is called. Which, considering the static nature of rule resolver mentioned above, would be not performant.

Roadmap

Flush fields references of the rule

Upon new field registration:

  1. Check if the field has any rules relevant to it.
  2. In case it does, analyze the rule resolver function as mentioned below.
function getRuleDependencies(fieldProps, form) {
  // flattenDeep the field rules, transforming the end value:
  return {
    resolver: f(),
    refs: [['fieldOne', 'value'], ['groupName', 'fieldName', 'propName']]
  };
}
  1. Store the analyzed Object in the form.someMapToStoreIt.
  2. Use the generated Map of transformed rules during the validation.
kettanaito commented 6 years ago

The current state of the feature branch contains the working implementation of this feature.

However, I had an idea that the same logic can be simplified if moved to applyRule function of fieldUtils.validateSync(). In the end, that is the end point for any validation rule applicable at the moment, so it serves the same goal as the rule gathering logic of the current implementation.

I would give it a try, and in case it works, and reduces the bundle file size, I would accept that as the solution. Otherwise, the current implementation will be used.

kettanaito commented 6 years ago

Update

Just merged the rest of the implementation, including simplified and refined sync validation algorithm, generation of form.state.rxRules and creating the rule resolvers' observables. That part is working as a charm.

I am still to fix failing serialization, as changes in flattenDeep have broken it, and ensure that all tests are passing. Then the changes are going to be merged into 1.0.

kettanaito commented 6 years ago

Update

The implementation is merged into 1.0 branch.

It will be published under the 1.0 version as soon as it lands. Closing.