Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Improve validation with asynchronous pipeline #175

Open
wants to merge 22 commits into
base: canary
Choose a base branch
from

Conversation

Mosoc
Copy link

@Mosoc Mosoc commented Apr 30, 2019

As reported in #173 #174

There are some todo:

  • Add schema property to validation object
  • Only create Ajv instance when schema is verified as non-empty object
  • Completion of type definition
  • Implement asynchronous structure validation
  • Make customize support both (a)sync function
  • Check valid or invalid situation
  • Handle errors by promise chaining
  • Update and run test cases
  • Documentation

Copy link
Contributor

@abz53378 abz53378 left a comment

Choose a reason for hiding this comment

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

We can just use the wrapper.instance().validate(result) to test the async function.
some refs:

@Mosoc
Copy link
Author

Mosoc commented May 15, 2019

@abz53378
My current solution is: Add async syntax to componentDidMount with await validate function resolution.

    async componentDidMount() {
      const {refId, validation = {}, onDeploy, required = false} = this.props;
      if (isEmpty(validation) && !required) {
        // no validation
        return;
      }
      const key = refId.getPathArr()[0];
      this.callbackId = onDeploy(key, await this.validate);
    }

Then run test with waiting this lifecycle, is it a good way?

   it('should onDeploy be called', async () => {
    const wrapper =  mount(<WrapperComponent
      {...props}
      required
    />);
    const instance = wrapper.instance();
    await instance.componentDidMount();
    expect(onDeploy).toBeCalledWith('posts', wrapper.instance().validate);
  });

@abz53378
Copy link
Contributor

@Mosoc , I don't think it's a good way to make the componentDidMount async, since the onDeploy function actually is just used to register the callback, not execute the callback. It's same as the registerCallback

@Mosoc Mosoc changed the title [WIP] Improve validation with asynchronous pipeline Improve validation with asynchronous pipeline May 16, 2019
@Mosoc
Copy link
Author

Mosoc commented May 16, 2019

@abz53378
Finally, I update the test files as your suggestions, and add new cases about async functions except unexpected error one. But I tested it manually by following snippet and it worked.

  it('should use customized validator with unexpected error', async () => {
    const result = {
      data: {
        0: { url: ''}
      }
    };
    const wrapper =  mount(<WrapperComponent
      {...props}
      validation={
        {
          validator: () => {
            const value = 1;
            value = 2;
          }
        }
      }
    />);
    await wrapper.instance().validate(result)
    expect(wrapper.state()).toMatchObject({
      error: true,
      errorInfo: [{
        message: 'Error: "value" is read-only'
      }]
    })
  });

@Mosoc
Copy link
Author

Mosoc commented May 18, 2019

I rechecked, and found that only one place needs to be modified.

In docs folder, there is a galleryValidation.

export const galleryValidation = {
  validator: (content, reject) => {
    if (content.length === 0) {
      return reject("should at least have one photo");
    }
  }
};

Maybe it could be following snippet in new version:

export const galleryValidation = {
  validator: (content) => {
    if (content.length === 0) {
      return "should at least have one photo";
    }
  }
};

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants