From 64c8f2e9fb6719c440676e89bae7e4723888cda5 Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Thu, 17 Jul 2025 17:56:29 +0900 Subject: [PATCH 1/9] add used --- src/tests/render/patchmap.test.js | 8 +++++++ src/utils/deepmerge/deepmerge.js | 2 ++ src/utils/deepmerge/deepmerge.test.js | 30 +++++++++++++++++++++++++++ src/utils/findIndexByPriority.js | 2 +- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/tests/render/patchmap.test.js b/src/tests/render/patchmap.test.js index 354883f5..30cef2e3 100644 --- a/src/tests/render/patchmap.test.js +++ b/src/tests/render/patchmap.test.js @@ -45,6 +45,8 @@ const sampleData = [ radius: 6, }, }, + { type: 'text', text: 'text-1' }, + { type: 'text', text: 'text-2' }, ], attrs: { x: 200, y: 300 }, }, @@ -89,6 +91,12 @@ describe('patchmap test', () => { expect(item).toBeDefined(); expect(item.id).toBe('item-1'); + const itemChildren = [...item.children]; + expect(itemChildren.length).toBe(3); + expect(itemChildren[0].type).toBe('background'); + expect(itemChildren[1].type).toBe('text'); + expect(itemChildren[2].type).toBe('text'); + const gridItems = grid.children; expect(gridItems.length).toBe(5); }); diff --git a/src/utils/deepmerge/deepmerge.js b/src/utils/deepmerge/deepmerge.js index 908304b6..ee3835a4 100644 --- a/src/utils/deepmerge/deepmerge.js +++ b/src/utils/deepmerge/deepmerge.js @@ -75,9 +75,11 @@ const mergeArray = (target, source, options, visited) => { } } else if (i < merged.length) { merged[i] = item; + used.add(i); return; } merged.push(item); + used.add(merged.length - 1); }); return merged; diff --git a/src/utils/deepmerge/deepmerge.test.js b/src/utils/deepmerge/deepmerge.test.js index 0ce099ba..cb99b421 100644 --- a/src/utils/deepmerge/deepmerge.test.js +++ b/src/utils/deepmerge/deepmerge.test.js @@ -197,6 +197,36 @@ describe('deepMerge – arrayMerge by id → label → type', () => { ], }, ], + [ + { + components: [], + }, + { + components: [ + { type: 'text', text: '5' }, + { type: 'text', text: '6' }, + { type: 'text', text: '7' }, + ], + }, + { + components: [ + { type: 'text', text: '5' }, + { type: 'text', text: '6' }, + { type: 'text', text: '7' }, + ], + }, + ], + [ + { + components: [{ type: 'text', text: 'original' }], + }, + { + components: [null, { type: 'text', text: 'new' }], + }, + { + components: [null, { type: 'text', text: 'new' }], + }, + ], ])('Case %#', (left, right, expected) => { expect(deepMerge(left, right)).toEqual(expected); }); diff --git a/src/utils/findIndexByPriority.js b/src/utils/findIndexByPriority.js index 02525e86..505e3c80 100644 --- a/src/utils/findIndexByPriority.js +++ b/src/utils/findIndexByPriority.js @@ -27,7 +27,7 @@ export const findIndexByPriority = ( for (const key of priorityKeys) { if (key in criteria) { return arr.findIndex( - (item, idx) => !usedIndexes.has(idx) && item[key] === criteria[key], + (item, idx) => !usedIndexes.has(idx) && item?.[key] === criteria?.[key], ); } } From 5b1a338b7c6a3ea9d4ced79c937c77a68aa6f709 Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Thu, 17 Jul 2025 18:09:49 +0900 Subject: [PATCH 2/9] fix childrenable/componentsable --- src/display/mixins/Childrenable.js | 5 ++++- src/display/mixins/Componentsable.js | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/display/mixins/Childrenable.js b/src/display/mixins/Childrenable.js index 62f516b6..0c14e6a7 100644 --- a/src/display/mixins/Childrenable.js +++ b/src/display/mixins/Childrenable.js @@ -20,12 +20,15 @@ export const Childrenable = (superClass) => { // new elements beforehand and validate them all at once. // 1. Filter out only the definitions for new elements. + const used = new Set(); const newElementDefs = []; const newElementIndices = []; // Store original indices to update the array later. childrenChanges.forEach((change, index) => { - if (findIndexByPriority(elements, change) === -1) { + if (findIndexByPriority(elements, change, used) === -1) { newElementDefs.push(change); newElementIndices.push(index); + } else { + used.add(index); } }); diff --git a/src/display/mixins/Componentsable.js b/src/display/mixins/Componentsable.js index f54eaf2e..a3cdd735 100644 --- a/src/display/mixins/Componentsable.js +++ b/src/display/mixins/Componentsable.js @@ -20,12 +20,15 @@ export const Componentsable = (superClass) => { // validate them all at once. // 1. Filter out only the definitions for components that need to be newly created. + const used = new Set(); const newComponentDefs = []; const newComponentIndices = []; // Store original indices to update the array later. componentsChanges.forEach((change, index) => { - if (findIndexByPriority(components, change) === -1) { + if (findIndexByPriority(components, change, used) === -1) { newComponentDefs.push(change); newComponentIndices.push(index); + } else { + used.add(index); } }); From 86628120d1fd177f99ba09fb782b2b7164a0ed2e Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Thu, 17 Jul 2025 18:20:47 +0900 Subject: [PATCH 3/9] fix --- src/utils/deepmerge/deepmerge.test.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/utils/deepmerge/deepmerge.test.js b/src/utils/deepmerge/deepmerge.test.js index cb99b421..88062216 100644 --- a/src/utils/deepmerge/deepmerge.test.js +++ b/src/utils/deepmerge/deepmerge.test.js @@ -217,15 +217,9 @@ describe('deepMerge – arrayMerge by id → label → type', () => { }, ], [ - { - components: [{ type: 'text', text: 'original' }], - }, - { - components: [null, { type: 'text', text: 'new' }], - }, - { - components: [null, { type: 'text', text: 'new' }], - }, + { components: [{ type: 'text', text: 'original' }] }, + { components: [null, { type: 'text', text: 'new' }] }, + { components: [null, { type: 'text', text: 'new' }] }, ], ])('Case %#', (left, right, expected) => { expect(deepMerge(left, right)).toEqual(expected); From 9417f530974846a6169cbae425700ec26d195cce Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Thu, 17 Jul 2025 19:17:15 +0900 Subject: [PATCH 4/9] fix --- src/display/mixins/Childrenable.js | 5 +++-- src/display/mixins/Componentsable.js | 5 +++-- src/tests/render/patchmap.test.js | 21 ++++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/display/mixins/Childrenable.js b/src/display/mixins/Childrenable.js index 0c14e6a7..f40cd72e 100644 --- a/src/display/mixins/Childrenable.js +++ b/src/display/mixins/Childrenable.js @@ -24,11 +24,12 @@ export const Childrenable = (superClass) => { const newElementDefs = []; const newElementIndices = []; // Store original indices to update the array later. childrenChanges.forEach((change, index) => { - if (findIndexByPriority(elements, change, used) === -1) { + const foundIndex = findIndexByPriority(elements, change, used) === -1; + if (foundIndex === -1) { newElementDefs.push(change); newElementIndices.push(index); } else { - used.add(index); + used.add(foundIndex); } }); diff --git a/src/display/mixins/Componentsable.js b/src/display/mixins/Componentsable.js index a3cdd735..950464bb 100644 --- a/src/display/mixins/Componentsable.js +++ b/src/display/mixins/Componentsable.js @@ -24,11 +24,12 @@ export const Componentsable = (superClass) => { const newComponentDefs = []; const newComponentIndices = []; // Store original indices to update the array later. componentsChanges.forEach((change, index) => { - if (findIndexByPriority(components, change, used) === -1) { + const foundIndex = findIndexByPriority(components, change, used); + if (foundIndex === -1) { newComponentDefs.push(change); newComponentIndices.push(index); } else { - used.add(index); + used.add(foundIndex); } }); diff --git a/src/tests/render/patchmap.test.js b/src/tests/render/patchmap.test.js index 30cef2e3..4040d4fc 100644 --- a/src/tests/render/patchmap.test.js +++ b/src/tests/render/patchmap.test.js @@ -47,6 +47,7 @@ const sampleData = [ }, { type: 'text', text: 'text-1' }, { type: 'text', text: 'text-2' }, + { type: 'text', id: 'new-text' }, ], attrs: { x: 200, y: 300 }, }, @@ -92,7 +93,7 @@ describe('patchmap test', () => { expect(item.id).toBe('item-1'); const itemChildren = [...item.children]; - expect(itemChildren.length).toBe(3); + expect(itemChildren.length).toBe(4); expect(itemChildren[0].type).toBe('background'); expect(itemChildren[1].type).toBe('text'); expect(itemChildren[2].type).toBe('text'); @@ -191,6 +192,24 @@ describe('patchmap test', () => { expect(group.x).toBe(350); expect(group.y).toBe(250); }); + + it('', () => { + patchmap.update({ + path: '$..[?(@.id=="item-1")]', + changes: { + components: [ + { type: 'text', id: 'new-text', text: '2' }, + { type: 'text', id: 'B', text: '99' }, + { type: 'text', id: 'new-text', text: '3' }, + ], + }, + }); + + const item = patchmap.selector('$..[?(@.id=="item-1")]')[0]; + expect(item.children.length).toBe(6); + expect(item.children[3].text).toBe('2'); + expect(item.children[4].text).toBe('99'); + }); }); describe('select', () => { From 02bc4cc79ab82a8556dbab316d7cd93dc36b5f3f Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Fri, 18 Jul 2025 10:34:32 +0900 Subject: [PATCH 5/9] fix --- src/display/mixins/Childrenable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/display/mixins/Childrenable.js b/src/display/mixins/Childrenable.js index f40cd72e..0ac30153 100644 --- a/src/display/mixins/Childrenable.js +++ b/src/display/mixins/Childrenable.js @@ -24,7 +24,7 @@ export const Childrenable = (superClass) => { const newElementDefs = []; const newElementIndices = []; // Store original indices to update the array later. childrenChanges.forEach((change, index) => { - const foundIndex = findIndexByPriority(elements, change, used) === -1; + const foundIndex = findIndexByPriority(elements, change, used); if (foundIndex === -1) { newElementDefs.push(change); newElementIndices.push(index); From f3634bec494cca8eac29aea6fbbb6489d77f0606 Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Fri, 18 Jul 2025 10:35:19 +0900 Subject: [PATCH 6/9] fix --- src/tests/render/patchmap.test.js | 2 +- src/utils/findIndexByPriority.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/render/patchmap.test.js b/src/tests/render/patchmap.test.js index 4040d4fc..e98553a3 100644 --- a/src/tests/render/patchmap.test.js +++ b/src/tests/render/patchmap.test.js @@ -193,7 +193,7 @@ describe('patchmap test', () => { expect(group.y).toBe(250); }); - it('', () => { + it('should handle array updates with duplicate ids correctly', () => { patchmap.update({ path: '$..[?(@.id=="item-1")]', changes: { diff --git a/src/utils/findIndexByPriority.js b/src/utils/findIndexByPriority.js index 505e3c80..2df459b1 100644 --- a/src/utils/findIndexByPriority.js +++ b/src/utils/findIndexByPriority.js @@ -27,7 +27,7 @@ export const findIndexByPriority = ( for (const key of priorityKeys) { if (key in criteria) { return arr.findIndex( - (item, idx) => !usedIndexes.has(idx) && item?.[key] === criteria?.[key], + (item, idx) => !usedIndexes.has(idx) && item?.[key] === criteria[key], ); } } From ad6efe777d69498ddb595c3d1ae9197ff154ed5a Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Fri, 18 Jul 2025 11:22:44 +0900 Subject: [PATCH 7/9] refactor --- src/display/mixins/Childrenable.js | 44 +++++--------------------- src/display/mixins/Componentsable.js | 47 +++++----------------------- src/display/mixins/utils.js | 45 ++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 77 deletions(-) diff --git a/src/display/mixins/Childrenable.js b/src/display/mixins/Childrenable.js index 0ac30153..d32be3e6 100644 --- a/src/display/mixins/Childrenable.js +++ b/src/display/mixins/Childrenable.js @@ -1,53 +1,23 @@ -import { isValidationError } from 'zod-validation-error'; import { deepMerge } from '../../utils/deepmerge/deepmerge'; import { findIndexByPriority } from '../../utils/findIndexByPriority'; -import { validate } from '../../utils/validator'; import { mapDataSchema } from '../data-schema/element-schema'; import { newElement } from '../elements/creator'; import { UPDATE_STAGES } from './constants'; +import { validateAndPrepareChanges } from './utils'; const KEYS = ['children']; export const Childrenable = (superClass) => { const MixedClass = class extends superClass { _applyChildren(relevantChanges, options) { - const { children: childrenChanges } = relevantChanges; + let { children: childrenChanges } = relevantChanges; let elements = [...this.children]; - // --- Start of Performance Optimization --- - // This logic mirrors the optimization in `Componentsable.js`. - // Instead of validating each new element inside the loop, we identify - // new elements beforehand and validate them all at once. - - // 1. Filter out only the definitions for new elements. - const used = new Set(); - const newElementDefs = []; - const newElementIndices = []; // Store original indices to update the array later. - childrenChanges.forEach((change, index) => { - const foundIndex = findIndexByPriority(elements, change, used); - if (foundIndex === -1) { - newElementDefs.push(change); - newElementIndices.push(index); - } else { - used.add(foundIndex); - } - }); - - // 2. If new elements exist, perform a single batch validation. - // This is far more efficient than validating one-by-one inside the loop. - if (newElementDefs.length > 0) { - const validatedNewDefs = validate(newElementDefs, mapDataSchema); - if (isValidationError(validatedNewDefs)) { - throw validatedNewDefs; - } - - // 3. Update the original changes array with the validated, default-filled definitions. - validatedNewDefs.forEach((validatedDef, i) => { - const originalIndex = newElementIndices[i]; - childrenChanges[originalIndex] = validatedDef; - }); - } - // --- End of Performance Optimization --- + childrenChanges = validateAndPrepareChanges( + elements, + childrenChanges, + mapDataSchema, + ); if (options.arrayMerge === 'replace') { elements.forEach((element) => { diff --git a/src/display/mixins/Componentsable.js b/src/display/mixins/Componentsable.js index 950464bb..e92f6dad 100644 --- a/src/display/mixins/Componentsable.js +++ b/src/display/mixins/Componentsable.js @@ -1,56 +1,23 @@ -import { isValidationError } from 'zod-validation-error'; import { deepMerge } from '../../utils/deepmerge/deepmerge'; import { findIndexByPriority } from '../../utils/findIndexByPriority'; -import { validate } from '../../utils/validator'; import { newComponent } from '../components/creator'; import { componentArraySchema } from '../data-schema/component-schema'; import { UPDATE_STAGES } from './constants'; +import { validateAndPrepareChanges } from './utils'; const KEYS = ['components']; export const Componentsable = (superClass) => { const MixedClass = class extends superClass { _applyComponents(relevantChanges, options) { - const { components: componentsChanges } = relevantChanges; + let { components: componentsChanges } = relevantChanges; let components = [...this.children]; - // --- Start of Performance Optimization --- - // The main performance bottleneck is calling `validate` for each new component - // inside a loop. To solve this, we pre-filter new components and - // validate them all at once. - - // 1. Filter out only the definitions for components that need to be newly created. - const used = new Set(); - const newComponentDefs = []; - const newComponentIndices = []; // Store original indices to update the array later. - componentsChanges.forEach((change, index) => { - const foundIndex = findIndexByPriority(components, change, used); - if (foundIndex === -1) { - newComponentDefs.push(change); - newComponentIndices.push(index); - } else { - used.add(foundIndex); - } - }); - - // 2. If there are new components, perform a single batch validation. - // This is far more efficient than validating one-by-one inside the loop. - if (newComponentDefs.length > 0) { - const validatedNewDefs = validate( - newComponentDefs, - componentArraySchema, - ); - if (isValidationError(validatedNewDefs)) { - throw validatedNewDefs; - } - - // 3. Update the original changes array with the validated, default-filled definitions. - validatedNewDefs.forEach((validatedDef, i) => { - const originalIndex = newComponentIndices[i]; - componentsChanges[originalIndex] = validatedDef; - }); - } - // --- End of Performance Optimization --- + componentsChanges = validateAndPrepareChanges( + components, + componentsChanges, + componentArraySchema, + ); if (options.arrayMerge === 'replace') { components.forEach((component) => { diff --git a/src/display/mixins/utils.js b/src/display/mixins/utils.js index 733e1597..c2053e98 100644 --- a/src/display/mixins/utils.js +++ b/src/display/mixins/utils.js @@ -1,4 +1,7 @@ import gsap from 'gsap'; +import { isValidationError } from 'zod-validation-error'; +import { findIndexByPriority } from '../../utils/findIndexByPriority'; +import { validate } from '../../utils/validator'; export const tweensOf = (object) => gsap.getTweensOf(object); @@ -60,3 +63,45 @@ export const calcSize = (component, { source, size }) => { export const mixins = (baseClass, ...mixins) => { return mixins.reduce((target, mixin) => mixin(target), baseClass); }; + +/** + * Utility function to validate and prepare a new array of changes. + * This function filters out only the new elements (not present in currentElements), + * and performs batch validation using a Zod schema. + * + * @param {Array} currentElements - Array of current child elements (components) in the DOM + * @param {Array} changes - Array of change data to apply + * @param {import('zod').ZodSchema} schema - Zod schema to use for validation + * @returns {Array} The changes array, with validated and default-filled data + */ +export function validateAndPrepareChanges(currentElements, changes, schema) { + const used = new Set(); + const newElementDefs = []; + const newElementIndices = []; + + changes.forEach((change, index) => { + const foundIndex = findIndexByPriority(currentElements, change, used); + if (foundIndex === -1) { + newElementDefs.push(change); + newElementIndices.push(index); + } else { + used.add(foundIndex); + } + }); + + // Perform batch validation only if there are new elements to be added + if (newElementDefs.length > 0) { + const validatedNewDefs = validate(newElementDefs, schema); + if (isValidationError(validatedNewDefs)) { + throw validatedNewDefs; + } + + // Update the original changes array with the validated definitions (including defaults) + validatedNewDefs.forEach((validatedDef, i) => { + const originalIndex = newElementIndices[i]; + changes[originalIndex] = validatedDef; + }); + } + + return changes; +} From 690cb954b2176e18317f48e31f1416e592db7306 Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Fri, 18 Jul 2025 11:34:09 +0900 Subject: [PATCH 8/9] fix --- src/display/mixins/utils.js | 13 +++++++------ src/tests/render/patchmap.test.js | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/display/mixins/utils.js b/src/display/mixins/utils.js index c2053e98..cdfce544 100644 --- a/src/display/mixins/utils.js +++ b/src/display/mixins/utils.js @@ -70,16 +70,17 @@ export const mixins = (baseClass, ...mixins) => { * and performs batch validation using a Zod schema. * * @param {Array} currentElements - Array of current child elements (components) in the DOM - * @param {Array} changes - Array of change data to apply + * @param {Array} preparedChanges - Array of change data to apply * @param {import('zod').ZodSchema} schema - Zod schema to use for validation * @returns {Array} The changes array, with validated and default-filled data */ -export function validateAndPrepareChanges(currentElements, changes, schema) { +export const validateAndPrepareChanges = (currentElements, changes, schema) => { + const preparedChanges = [...changes]; const used = new Set(); const newElementDefs = []; const newElementIndices = []; - changes.forEach((change, index) => { + preparedChanges.forEach((change, index) => { const foundIndex = findIndexByPriority(currentElements, change, used); if (foundIndex === -1) { newElementDefs.push(change); @@ -99,9 +100,9 @@ export function validateAndPrepareChanges(currentElements, changes, schema) { // Update the original changes array with the validated definitions (including defaults) validatedNewDefs.forEach((validatedDef, i) => { const originalIndex = newElementIndices[i]; - changes[originalIndex] = validatedDef; + preparedChanges[originalIndex] = validatedDef; }); } - return changes; -} + return preparedChanges; +}; diff --git a/src/tests/render/patchmap.test.js b/src/tests/render/patchmap.test.js index e98553a3..8f2e53e1 100644 --- a/src/tests/render/patchmap.test.js +++ b/src/tests/render/patchmap.test.js @@ -209,6 +209,7 @@ describe('patchmap test', () => { expect(item.children.length).toBe(6); expect(item.children[3].text).toBe('2'); expect(item.children[4].text).toBe('99'); + expect(item.children[5].text).toBe('3'); }); }); From 9702bf94164415b76e384d0b2bc7b166e510b194 Mon Sep 17 00:00:00 2001 From: MinHo Lim Date: Fri, 18 Jul 2025 11:43:32 +0900 Subject: [PATCH 9/9] fix --- src/display/mixins/utils.js | 2 +- src/tests/render/patchmap.test.js | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/display/mixins/utils.js b/src/display/mixins/utils.js index cdfce544..df6ab2aa 100644 --- a/src/display/mixins/utils.js +++ b/src/display/mixins/utils.js @@ -70,7 +70,7 @@ export const mixins = (baseClass, ...mixins) => { * and performs batch validation using a Zod schema. * * @param {Array} currentElements - Array of current child elements (components) in the DOM - * @param {Array} preparedChanges - Array of change data to apply + * @param {Array} changes - Array of change data to apply * @param {import('zod').ZodSchema} schema - Zod schema to use for validation * @returns {Array} The changes array, with validated and default-filled data */ diff --git a/src/tests/render/patchmap.test.js b/src/tests/render/patchmap.test.js index 8f2e53e1..eccdcecf 100644 --- a/src/tests/render/patchmap.test.js +++ b/src/tests/render/patchmap.test.js @@ -207,9 +207,14 @@ describe('patchmap test', () => { const item = patchmap.selector('$..[?(@.id=="item-1")]')[0]; expect(item.children.length).toBe(6); - expect(item.children[3].text).toBe('2'); - expect(item.children[4].text).toBe('99'); - expect(item.children[5].text).toBe('3'); + + const newTextChildren = item.children.filter((c) => c.id === 'new-text'); + expect(newTextChildren.length).toBe(2); + expect(newTextChildren.map((c) => c.text).sort()).toEqual(['2', '3']); + + const childB = item.children.find((c) => c.id === 'B'); + expect(childB).toBeDefined(); + expect(childB.text).toBe('99'); }); });