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

[WIP] Implement async validation #1

Closed
wants to merge 4 commits into from
Closed

[WIP] Implement async validation #1

wants to merge 4 commits into from

Conversation

Mosoc
Copy link
Owner

@Mosoc Mosoc commented Apr 28, 2019

因為擔心英文言不及意,請讓我用中文敘述一次。
會用第一個部分英文版本另外在 canner/canner 開個 issue,
第二部分會直接去編輯現有的那個 issue。

Part One

首先,validation 這個物件會整個過一次 ajv.compile(),再把 validator, errorMessage 從 validation 解構出來,在進行後續處理。這很明顯事先有 ajv驗證之後,錯誤訊息和自訂函數才陸陸續續加進來。

結論上,我覺得這種混在一起的方式相當不好,主要有幾個問題:

  • 不好進行 Type checking 也不好做 Type Management ,等於要解構別人已經定義好的型別再插入其他的屬性,也怕這些或者往後多加進去的屬性影響 ajv 運作,或跟 ajv 的命名衝突;也有可能是反過來是 ajv 往後如果多加了 validator和errorMessage 的 options 讓命名衝突。

現在 canner下的 型別定義
其實只做了很粗淺定義而已
可以對照 ajv下的 規格說明 作一下補充

  • 就是不管有沒有要跑 ajv validation 每個 field 都會起個 ajv instance,這個跟當年 AngularJS 每個數值都要 $watch 監看一樣會影響效能。

我覺得應該把會經過 ajv 的部分 跟我們自定義的 validator, errorMessage 有所區隔,方法大致有兩種。

一種是直接分成不同 props

<string
  keyName="text"
  title="Text"
  validation={{pattern: '^[a-zA-Z0-9]{4,10}$'}}
  customizedValidator={()=>( /* ... */ )}
  errorMessage="Error!"
/>

另一種是還在同個props 但在物件中給予獨立的屬性:schema ( ajv 也是用這個關鍵字 )

<string
  keyName="text"
  title="Text"
  validation= {{
    schema: {{pattern: '^[a-zA-Z0-9]{4,10}$'}} // JSONSchemaOptions
    validator: {()=>()} //ValidationFunction
    errorMessage="Error!"
  }}
/>

只要用 const { schema, validator, errorMessage} = validation; 解構取值就好

let validate = null
if(schema && !isEmpty(schema)) {
  const ajv = new Ajv();
  validate = ajv.compile(schema);
}

只要帶 schema 裡面的東西進去 compile 就好,如果沒 schema 的話連 ajv instance 都不用 new。

Part Two

再來是這次的主角: Async Validation
比較快的實作會是下面這樣直接套用 async/await 語法

validate = async () => {
 // ...
  let validatorResult = null;
  if(validator && isFunction(validator)) {
    validatorResult = await validator(value, reject);
  }
  // ...
}

當然會需要多一些驗證和例外處理,包含 Timeout 。尤其是 validator 帶進來的函式是第三方套件的狀況下。

不過看 canner 的 source code ,只有 server 端有用到 async / await 語法,不知道是不是怕過 babel 及 webpack 之後會有行為不如預習或有效能的考量?

有鑑於判斷到底是不是 async function/ function with promise return 是一件有點麻煩的事情,在瀏覽器端沒有除了runtime 判斷以外很好的跨瀏覽器解決方案。

極端而言甚至會有這兩種狀態:

function foo(bar) {
  if (bar)
    return new Promise(resolve => ...);

   // NOTE: please, never do this!
   return null;
}
async function x() {}
function y() { return Promise.resolve(); }
function z() { return y() || x(); }

如果純用 Promise 解決的情境,我偏好用 類似 Formik 原始碼文件中這種不管哪種傳進來是不是回傳值帶 Promise 都先包一層起來的處理法。

包含原本的 ajv 驗證都 promise 化,用 promise.all 再把回傳值 map reduce的方式去處理,會比較清爽一點。

另外,因為是需要時間去處理的程序,可能需要一個地方或者就放在 error message 處,顯示驗證中 ( Validating... ) 的文字敘述,或者其他圖片動畫的示意。

@abz53378
Copy link

Hi, Bruno,

Thanks for this great suggestion.

For part 1, it's great, we all agree the reasons you mentioned, so the breaking change is OK. And we'd like to take the usage

    <string
      keyName="text"
      title="Text"
      validation= {{
        schema: {pattern: '^[a-zA-Z0-9]{4,10}$'} // JSONSchemaOptions
        validator: ()=>() //ValidationFunction
        errorMessage: "Error!"
      }}
    />

FYI, there is a better way that we can create the AJV instance of a field only once at the compile time (Canner has the compiler that transforms schema to component tree) instead of creating instance at every validation. But we will improve this later, your solution is great for now.

For part2, async/await is available in the schema and source code, and using the formik way also looks great to us. So, we'd like to remove the callback and handle the Error. Here are the possible use cases, how do you think?

  • The validator returns String which represents the error message.
        // the validator which returns string
        function(value) {
          if (!syncValidate(value)) {
            return 'the value is invalid';
          }
        }
        // the validator which returns promise of string
        function(value) {
          return asyncValidate(value)
        	.then(valid => {
        		if (!valid) {
        			return 'the value is invalid';
        		}
        	});
        }
        // the validator which use async/await
        async function(value) {
          const valid = await asyncValidate(value)
        	if (!valid) {
        		return 'the value is invalid';
        	}
        }
  • The validator throws Error. There are two kinds of Error, one is that the user throws, and the other is the unexpected Error. The validation HOC should get the error message of the Errors.
        // the validator which throws string
        function(value) {
          if (!syncValidate(value)) {
            throw new Error('the value is invalid');
          }
        }
        // the validator which returns promise of string
        function(value) {
          return asyncValidate(value)
        	.then(valid => {
        		if (!valid) {
        			throw new Error('the value is invalid');
        		}
        	});
        }
        // the validator which use async/await
        async function(value) {
          const valid = await asyncValidate(value)
        	if (!valid) {
        		throw new Error('the value is invalid');
        	}
        }
        
        // the unexpected error
        function(value) {
        	const a = '123';
          // throw an unexpected error, should be invalid
        	a(value);
        }
  • The validator returns other represent the value is con as valid.

Although, the better solution to handle the Error is that we provide a ValidationError for users to make different behavior like logging the error stack of an unexpected Error in the console, we think we can implement this later, the current use cases above are good enough for now.

Summary

  • We will have a breaking change you mentioned
        <string
          keyName="text"
          title="Text"
          validation= {{
            schema: {pattern: '^[a-zA-Z0-9]{4,10}$'} // JSONSchemaOptions
            validator: ()=>() //ValidationFunction
            errorMessage: "Error!"
          }}
        />
  • We will remove the callback argument in a validator function.
  • If the validator is not a function, the value is invalid, and we log the error in the console
  • We will Promisify the validator function with Formik way
  • the resolved value of promised validator is may be string(invalid), Error(invalid), or others(valid)
  • Please ensure your test cases have the following situation
    • sync validator, return string → invalid
    • sync validator, throw Error → invalid
    • sync validator, unexpected Error → invalid
    • sync validator, no return, → valid
    • async validator, async/await, return string → invalid
    • async validator, async/await, throw Error → invalid
    • async validator, async/await, no return → invalid
    • async validator, return Promise → invalid
    • async validator, return Promise → invalid
    • async validator, return Promise → valid
    • validator is not a function → invalid

How do you think? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants