fix: #1627 onValidationError did not always trigger

josdejong · Nov 13, 2024 · dfb8998 · dfb8998
1 parent 1093554
commit dfb8998
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 24 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -6,6 +6,7 @@
 
 - Fix: when editing a key giving it a ` (copy)` suffix, the suffix was removed. 
 - Fix: upgrade to the latest version of `ace`.
+- Fix: #1627 `onValidationError` did not always trigger.
 
 ## 2024-06-19, version 10.1.0
 

diff --git a/docs/api.md b/docs/api.md
@@ -193,7 +193,7 @@ Constructs a new JSONEditor.
 - `{function} onValidationError(errors: ValidationError[])`
 
   Set a callback function for validation and parse errors. Available in all modes.
-  The `ValidationError` contains a `type`, `path`, and `message`. 
+  The `ValidationError` contains a `type`, and an `error` object. 
 
   On validation of the json, if errors of any kind were found this callback is invoked with the errors data.
 

diff --git a/src/js/util.js b/src/js/util.js
@@ -1235,18 +1235,21 @@ export function contains (array, item) {
  * @param {Array} prevErr previous validation errors
  */
 export function isValidationErrorChanged (currErr, prevErr) {
-  if (!prevErr && !currErr) { return false }
-  if ((prevErr && !currErr) || (!prevErr && currErr)) { return true }
-  if (prevErr.length !== currErr.length) { return true }
-
-  for (let i = 0; i < currErr.length; ++i) {
-    let pErr
-    if (currErr[i].type === 'error') {
-      pErr = prevErr.find(p => p.line === currErr[i].line)
-    } else {
-      pErr = prevErr.find(p => p.dataPath === currErr[i].dataPath && p.schemaPath === currErr[i].schemaPath)
-    }
-    if (!pErr) {
+  if (!currErr && !prevErr) {
+    return false
+  }
+  if (!Array.isArray(currErr) || !Array.isArray(prevErr) || prevErr.length !== currErr.length) {
+    return true
+  }
+
+  for (let i = 0; i < currErr.length; i++) {
+    const currItem = currErr[i]
+    const prevItem = prevErr[i]
+
+    if (
+      currItem.type !== prevItem.type ||
+      JSON.stringify(currItem.error) !== JSON.stringify(prevItem.error)
+    ) {
       return true
     }
   }
@@ -1257,7 +1260,7 @@ export function isValidationErrorChanged (currErr, prevErr) {
 /**
  * Uniquely merge array of elements
  * @param {Array<string|number>} inputArray1
- * @param {Array<string|number?} inputArray2
+ * @param {Array<string|number>} inputArray2
  * @returns {Array<string|number>} an array with unique merged elements
  */
 export function uniqueMergeArrays (inputArray1, inputArray2) {

diff --git a/test/util.test.js b/test/util.test.js
@@ -86,30 +86,31 @@ describe('util', () => {
   })
 
   describe('isValidationErrorChanged', () => {
-    const err1 = { keyword: 'enum', dataPath: '.gender', schemaPath: '#/properties/gender/enum', params: { allowedValues: ['male', 'female'] }, message: 'should be equal to one of: "male", "female"', schema: ['male', 'female'], parentSchema: { title: 'Gender', enum: ['male', 'female'] }, data: null, type: 'validation' }
-    const err2 = { keyword: 'type', dataPath: '.age', schemaPath: '#/properties/age/type', params: { type: 'integer' }, message: 'should be integer', schema: 'integer', parentSchema: { description: 'Age in years', type: 'integer', minimum: 0, examples: [28, 32] }, data: '28', type: 'validation' }
-    const err3 = { dataPath: '.gender', message: 'Member must be an object with properties "name" and "age"' }
+    const err1 = { type: 'validation', error: { keyword: 'enum', dataPath: '.gender', schemaPath: '#/properties/gender/enum', params: { allowedValues: ['male', 'female'] }, message: 'should be equal to one of: "male", "female"', schema: ['male', 'female'], parentSchema: { title: 'Gender', enum: ['male', 'female'] }, data: null, type: 'validation' }}
+    const err2 = { type: 'validation', error: { keyword: 'type', dataPath: '.age', schemaPath: '#/properties/age/type', params: { type: 'integer' }, message: 'should be integer', schema: 'integer', parentSchema: { description: 'Age in years', type: 'integer', minimum: 0, examples: [28, 32] }, data: '28', type: 'validation' }}
+    const err3 = { type: 'validation', error: { dataPath: '.gender', message: 'Member must be an object with properties "name" and "age"' }}
+    const err3b = { type: 'validation', error: { dataPath: '.gender', message: 'Must be an object' }}
 
-    it('empty value for both current and previoues error should return false', () => {
+    it('empty value for both current and previous error should return false', () => {
       assert.strictEqual(isValidationErrorChanged(), false)
     })
 
-    it('empty value for one of current and previoues error should return true', () => {
+    it('empty value for one of current and previous error should return true', () => {
       assert.strictEqual(isValidationErrorChanged([err1]), true)
       assert.strictEqual(isValidationErrorChanged(undefined, [err1]), true)
     })
 
-    it('different length of current and previoues errors should return true', () => {
+    it('different length of current and previous errors should return true', () => {
       assert.strictEqual(isValidationErrorChanged([err1], []), true)
       assert.strictEqual(isValidationErrorChanged([err1], [err1, err2]), true)
     })
 
-    it('same values for current and previoues errors should return false', () => {
-      assert.strictEqual(isValidationErrorChanged([err1, err2, err3], [err2, err3, err1]), false)
+    it('different values for current and previous errors should return true', () => {
+      assert.strictEqual(isValidationErrorChanged([err1, err2], [err3, err1]), true)
     })
 
-    it('different values for current and previoues errors should return true', () => {
-      assert.strictEqual(isValidationErrorChanged([err1, err2], [err3, err1]), true)
+    it('different message', () => {
+      assert.strictEqual(isValidationErrorChanged([err3], [err3b]), true)
     })
   })