fix: RenderProps deps validate (#218)

* fix: RenderProps deps validate

* test: more test case

* refactor: Move validate to field inner check

* refactor: Re-order validate inner sep

* fix: Validate should snapshot status

* test: fix test case

* test: fix test case

Author: zombieJ

src/Field.tsx

@@ -26,9 +26,9 @@ import {
   getValue,
 } from './utils/valueUtil';
 
-export type ShouldUpdate =
+export type ShouldUpdate<Values = any> =
   | boolean
-  | ((prevValues: Store, nextValues: Store, info: { source?: string }) => boolean);
+  | ((prevValues: Values, nextValues: Values, info: { source?: string }) => boolean);
 
 function requireUpdate(
   shouldUpdate: ShouldUpdate,
@@ -63,7 +63,7 @@ export interface InternalFieldProps<Values = any> {
   name?: InternalNamePath;
   normalize?: (value: StoreValue, prevValue: StoreValue, allValues: Store) => StoreValue;
   rules?: Rule[];
-  shouldUpdate?: ShouldUpdate;
+  shouldUpdate?: ShouldUpdate<Values>;
   trigger?: string;
   validateTrigger?: string | string[] | false;
   validateFirst?: boolean | 'parallel';
@@ -309,49 +309,62 @@ class Field extends React.Component<InternalFieldProps, FieldState> implements F
     }
   };
 
-  public validateRules = (options?: ValidateOptions) => {
-    const { validateFirst = false, messageVariables } = this.props;
-    const { triggerName } = (options || {}) as ValidateOptions;
+  public validateRules = (options?: ValidateOptions): Promise<string[]> => {
+    // We should fixed namePath & value to avoid developer change then by form function
     const namePath = this.getNamePath();
+    const currentValue = this.getValue();
 
-    let filteredRules = this.getRules();
-    if (triggerName) {
-      filteredRules = filteredRules.filter((rule: RuleObject) => {
-        const { validateTrigger } = rule;
-        if (!validateTrigger) {
-          return true;
-        }
-        const triggerList = toArray(validateTrigger);
-        return triggerList.includes(triggerName);
-      });
-    }
+    // Force change to async to avoid rule OOD under renderProps field
+    const rootPromise = Promise.resolve().then(() => {
+      if (!this.mounted) {
+        return [];
+      }
 
-    const promise = validateRules(
-      namePath,
-      this.getValue(),
-      filteredRules,
-      options,
-      validateFirst,
-      messageVariables,
-    );
+      const { validateFirst = false, messageVariables } = this.props;
+      const { triggerName } = (options || {}) as ValidateOptions;
+
+      let filteredRules = this.getRules();
+      if (triggerName) {
+        filteredRules = filteredRules.filter((rule: RuleObject) => {
+          const { validateTrigger } = rule;
+          if (!validateTrigger) {
+            return true;
+          }
+          const triggerList = toArray(validateTrigger);
+          return triggerList.includes(triggerName);
+        });
+      }
+
+      const promise = validateRules(
+        namePath,
+        currentValue,
+        filteredRules,
+        options,
+        validateFirst,
+        messageVariables,
+      );
+
+      promise
+        .catch(e => e)
+        .then((errors: string[] = []) => {
+          if (this.validatePromise === rootPromise) {
+            this.validatePromise = null;
+            this.errors = errors;
+            this.reRender();
+          }
+        });
+
+      return promise;
+    });
+
+    this.validatePromise = rootPromise;
     this.dirty = true;
-    this.validatePromise = promise;
     this.errors = [];
 
     // Force trigger re-render since we need sync renderProps with new meta
     this.reRender();
 
-    promise
-      .catch(e => e)
-      .then((errors: string[] = []) => {
-        if (this.validatePromise === promise) {
-          this.validatePromise = null;
-          this.errors = errors;
-          this.reRender();
-        }
-      });
-
-    return promise;
+    return rootPromise;
   };
 
   public isFieldValidating = () => !!this.validatePromise;

src/interface.ts

@@ -46,7 +46,7 @@ type Validator = (
   rule: RuleObject,
   value: StoreValue,
   callback: (error?: string) => void,
-) => Promise<void> | void;
+) => Promise<void | any> | void;
 
 export type RuleRender = (form: FormInstance) => RuleObject;
 

src/useForm.ts

@@ -578,8 +578,11 @@ export class FormStore {
     });
 
     // Notify dependencies children with parent update
+    // We need delay to trigger validate in case Field is under render props
     const childrenFields = this.getDependencyChildrenFields(namePath);
-    this.validateFields(childrenFields);
+    if (childrenFields.length) {
+      this.validateFields(childrenFields);
+    }
 
     this.notifyObservers(prevStore, childrenFields, {
       type: 'dependenciesUpdate',

tests/common/InfoField.tsx

@@ -13,17 +13,21 @@ export const Input = ({ value = '', ...props }) => <input {...props} value={valu
  */
 const InfoField: React.FC<InfoFieldProps> = ({ children, ...props }) => (
   <Field {...props}>
-    {(control, { errors, validating }) => (
-      <div>
-        {children ? React.cloneElement(children, control) : <Input {...control} />}
-        <ul className="errors">
-          {errors.map((error, index) => (
-            <li key={index}>{error}</li>
-          ))}
-        </ul>
-        {validating && <span className="validating" />}
-      </div>
-    )}
+    {(control, info) => {
+      const { errors, validating } = info;
+
+      return (
+        <div>
+          {children ? React.cloneElement(children, control) : <Input {...control} />}
+          <ul className="errors">
+            {errors.map((error, index) => (
+              <li key={index}>{error}</li>
+            ))}
+          </ul>
+          {validating && <span className="validating" />}
+        </div>
+      );
+    }}
   </Field>
 );
 

tests/common/index.js renamed to tests/common/index.ts

@@ -25,7 +25,7 @@ export function matchError(wrapper, error) {
   }
 }
 
-export function getField(wrapper, index = 0) {
+export function getField(wrapper, index: string | number = 0) {
   if (typeof index === 'number') {
     return wrapper.find(Field).at(index);
   }

tests/dependencies.test.js

@@ -167,7 +167,7 @@ describe('Form.Dependencies', () => {
     const spy = jest.fn();
     const wrapper = mount(
       <Form>
-        <Field dependencies={['field_1']} shouldUpdate={() => true}>
+        <Field dependencies={['field_2']} shouldUpdate={() => true}>
           {() => {
             spy();
             return 'gogogo';
@@ -182,27 +182,20 @@ describe('Form.Dependencies', () => {
       </Form>,
     );
     expect(spy).toHaveBeenCalledTimes(1);
-    await changeValue(getField(wrapper, 2), 'value2');
+    await changeValue(getField(wrapper, 1), 'value1');
     // sync start
     //   valueUpdate -> rerender by shouldUpdate
     //   depsUpdate  -> rerender by deps
     //   [ react rerender once -> 2 ]
     // sync end
-    // async start
-    //   validateFinish -> rerender by shouldUpdate
-    //   [ react rerender once -> 3 ]
-    // async end
-    expect(spy).toHaveBeenCalledTimes(3);
-    await changeValue(getField(wrapper, 1), 'value1');
+    expect(spy).toHaveBeenCalledTimes(2);
+
+    await changeValue(getField(wrapper, 2), 'value2');
     // sync start
     //   valueUpdate -> rerender by shouldUpdate
     //   depsUpdate  -> rerender by deps
-    //   [ react rerender once -> 4 ]
+    //   [ react rerender once -> 3 ]
     // sync end
-    // async start
-    //   validateFinish -> rerender by shouldUpdate
-    //   [ react rerender once -> 5 ]
-    // async end
-    expect(spy).toHaveBeenCalledTimes(5);
+    expect(spy).toHaveBeenCalledTimes(3);
   });
 });

tests/legacy/dynamic-binding.test.js

@@ -2,8 +2,6 @@ import React from 'react';
 import { mount } from 'enzyme';
 import Form, { Field } from '../../src';
 import { Input } from '../common/InfoField';
-import { changeValue, getField } from '../common';
-import timeout from '../common/timeout';
 
 describe('legacy.dynamic-binding', () => {
   const getInput = (wrapper, id) => wrapper.find(id).last();

tests/validate.test.js renamed to tests/validate.test.tsx

@@ -498,7 +498,7 @@ describe('Form.Validate', () => {
 
     [
       { name: 'serialization', first: true, second: false, validateFirst: true },
-      { name: 'parallel', first: true, second: true, validateFirst: 'parallel' },
+      { name: 'parallel', first: true, second: true, validateFirst: 'parallel' as const },
     ].forEach(({ name, first, second, validateFirst }) => {
       it(name, async () => {
         let ruleFirst = false;
@@ -532,6 +532,7 @@ describe('Form.Validate', () => {
         await changeValue(wrapper, 'test');
         await timeout();
 
+        wrapper.update();
         matchError(wrapper, 'failed first');
 
         expect(ruleFirst).toEqual(first);
@@ -610,5 +611,80 @@ describe('Form.Validate', () => {
     wrapper.find('button').simulate('click');
     expect(renderProps.mock.calls[0][1]).toEqual(expect.objectContaining({ validating: true }));
   });
+
+  it('renderProps should use latest rules', async () => {
+    let failedTriggerTimes = 0;
+    let passedTriggerTimes = 0;
+
+    interface FormStore {
+      username: string;
+      password: string;
+    }
+
+    const Demo = () => (
+      <Form>
+        <InfoField name="username" />
+        <Form.Field<FormStore> shouldUpdate={(prev, cur) => prev.username !== cur.username}>
+          {(_, __, { getFieldValue }) => {
+            const value = getFieldValue('username');
+
+            if (value === 'removed') {
+              return null;
+            }
+
+            return (
+              <InfoField
+                dependencies={['username']}
+                name="password"
+                rules={
+                  value !== 'light'
+                    ? [
+                        {
+                          validator: async () => {
+                            failedTriggerTimes += 1;
+                            throw new Error('Failed');
+                          },
+                        },
+                      ]
+                    : [
+                        {
+                          validator: async () => {
+                            passedTriggerTimes += 1;
+                          },
+                        },
+                      ]
+                }
+              />
+            );
+          }}
+        </Form.Field>
+      </Form>
+    );
+
+    const wrapper = mount(<Demo />);
+
+    expect(failedTriggerTimes).toEqual(0);
+    expect(passedTriggerTimes).toEqual(0);
+
+    // Failed of second input
+    await changeValue(getField(wrapper, 1), '');
+    matchError(getField(wrapper, 2), true);
+
+    expect(failedTriggerTimes).toEqual(1);
+    expect(passedTriggerTimes).toEqual(0);
+
+    // Changed first to trigger update
+    await changeValue(getField(wrapper, 0), 'light');
+    matchError(getField(wrapper, 2), false);
+
+    expect(failedTriggerTimes).toEqual(1);
+    expect(passedTriggerTimes).toEqual(1);
+
+    // Remove should not trigger validate
+    await changeValue(getField(wrapper, 0), 'removed');
+
+    expect(failedTriggerTimes).toEqual(1);
+    expect(passedTriggerTimes).toEqual(1);
+  });
 });
 /* eslint-enable no-template-curly-in-string */