From 2d5506cedeed7175c98b37e6bc3fe7a8dc152d68 Mon Sep 17 00:00:00 2001 From: Stefan Dirix Date: Mon, 1 Jun 2026 12:02:52 +0200 Subject: [PATCH] fix: refactor update data to avoid lodash issues Replace the lodash/fp/set call in UPDATE_DATA with a dedicated helper. This fixes issues with numeric segments (e.g. "group-key.15") and bracket characters in property names (e.g. "test[0]"). Fixes #2102 Fixes #2397 --- packages/core/src/reducers/core.ts | 14 +- packages/core/src/util/index.ts | 1 + packages/core/src/util/setData.ts | 187 ++++++++++++ packages/core/test/reducers/core.test.ts | 265 ++++++++++++++++++ .../src/examples/special-property-names.ts | 133 +++++++++ packages/examples/src/index.ts | 2 + 6 files changed, 595 insertions(+), 7 deletions(-) create mode 100644 packages/core/src/util/setData.ts create mode 100644 packages/examples/src/examples/special-property-names.ts diff --git a/packages/core/src/reducers/core.ts b/packages/core/src/reducers/core.ts index 3ec21a3dba..3c6900803a 100644 --- a/packages/core/src/reducers/core.ts +++ b/packages/core/src/reducers/core.ts @@ -24,10 +24,9 @@ */ import cloneDeep from 'lodash/cloneDeep'; -import setFp from 'lodash/fp/set'; -import unsetFp from 'lodash/fp/unset'; import get from 'lodash/get'; import isEqual from 'lodash/isEqual'; +import { setDataAt, unsetDataAt } from '../util/setData'; import { CoreActions, INIT, @@ -242,15 +241,16 @@ export const coreReducer: Reducer = ( const newData = action.updater(cloneDeep(oldData)); let newState: any; if (newData !== undefined) { - newState = setFp( + newState = setDataAt( + state.data === undefined ? {} : state.data, action.path, newData, - state.data === undefined ? {} : state.data + state.schema ); } else { - newState = unsetFp( - action.path, - state.data === undefined ? {} : state.data + newState = unsetDataAt( + state.data === undefined ? {} : state.data, + action.path ); } const errors = validate(state.validator, newState); diff --git a/packages/core/src/util/index.ts b/packages/core/src/util/index.ts index 82917614b4..0d4ec3564f 100644 --- a/packages/core/src/util/index.ts +++ b/packages/core/src/util/index.ts @@ -28,6 +28,7 @@ export * from './ids'; export * from './label'; export * from './path'; export * from './resolvers'; +export * from './setData'; export * from './runtime'; export * from './schema'; export * from './uischema'; diff --git a/packages/core/src/util/setData.ts b/packages/core/src/util/setData.ts new file mode 100644 index 0000000000..9c85aa68ac --- /dev/null +++ b/packages/core/src/util/setData.ts @@ -0,0 +1,187 @@ +/* + The MIT License + + Copyright (c) 2017-2019 EclipseSource Munich + https://github.com/eclipsesource/jsonforms + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. +*/ + +import type { JsonSchema } from '../models'; +import { encode } from './path'; +import { resolveSchema } from './resolvers'; +import { hasType } from './util'; + +const splitPath = (path: string): string[] => path.split('.'); + +/** + * Walks one step in the schema along the given data path segment, so we can + * tell whether a missing intermediate container should be created as an + * array or as an object. + */ +const stepSchema = ( + schema: JsonSchema | undefined, + segment: string, + rootSchema: JsonSchema | undefined +): JsonSchema | undefined => { + if (!schema || !rootSchema) { + return undefined; + } + const pointer = hasType(schema, 'array') + ? Array.isArray(schema.items) + ? `/items/${segment}` + : '/items' + : `/properties/${encode(segment)}`; + return resolveSchema(schema, pointer, rootSchema); +}; + +const cloneContainer = (data: any): any => { + if (Array.isArray(data)) { + return [...data]; + } + return { ...(data ?? {}) }; +}; + +const assign = (container: any, segment: string, value: any): any => { + if (Array.isArray(container)) { + const index = Number(segment); + container[Number.isInteger(index) ? index : (segment as any)] = value; + } else { + container[segment] = value; + } + return container; +}; + +/** + * Immutably sets `value` at the dotted `path` within `data`. + * + * Numeric path segments and segments containing bracket notation are treated + * as plain object property names. The optional `rootSchema` is consulted when + * a new intermediate container has to be created, so that arrays are still + * created where the schema declares an array type. + */ +export const setDataAt = ( + data: any, + path: string, + value: any, + rootSchema?: JsonSchema +): any => { + const segments = splitPath(path); + if (segments.length === 0) { + return value; + } + return doSet(data, segments, 0, value, rootSchema, rootSchema); +}; + +const doSet = ( + data: any, + segments: string[], + index: number, + value: any, + currentSchema: JsonSchema | undefined, + rootSchema: JsonSchema | undefined +): any => { + const segment = segments[index]; + const childSchema = stepSchema(currentSchema, segment, rootSchema); + const container = cloneContainer(data); + + if (index === segments.length - 1) { + return assign(container, segment, value); + } + + const existingChild = data?.[segment]; + let nextValue; + if ( + existingChild !== undefined && + existingChild !== null && + typeof existingChild === 'object' + ) { + nextValue = doSet( + existingChild, + segments, + index + 1, + value, + childSchema, + rootSchema + ); + } else { + const initial = hasType(childSchema, 'array') ? [] : {}; + nextValue = doSet( + initial, + segments, + index + 1, + value, + childSchema, + rootSchema + ); + } + return assign(container, segment, nextValue); +}; + +/** + * Immutably unsets the value at the dotted `path` within `data`. + * + * Numeric path segments and bracket notation in segments are treated as + * plain object property names, mirroring the semantics of {@link setDataAt}. + */ +export const unsetDataAt = (data: any, path: string): any => { + const segments = splitPath(path); + if (segments.length === 0) { + return data; + } + return doUnset(data, segments, 0); +}; + +const doUnset = (data: any, segments: string[], index: number): any => { + if (data === undefined || data === null) { + return data; + } + const segment = segments[index]; + if (index === segments.length - 1) { + if (Array.isArray(data)) { + const container = [...data]; + const numericIndex = Number(segment); + if (Number.isInteger(numericIndex)) { + delete container[numericIndex]; + } + return container; + } + if (!Object.prototype.hasOwnProperty.call(data, segment)) { + return data; + } + const container: { [key: string]: any } = { ...data }; + delete container[segment]; + return container; + } + + const existingChild = data[segment]; + if ( + existingChild === undefined || + existingChild === null || + typeof existingChild !== 'object' + ) { + return data; + } + const nextValue = doUnset(existingChild, segments, index + 1); + if (nextValue === existingChild) { + return data; + } + const container = cloneContainer(data); + return assign(container, segment, nextValue); +}; diff --git a/packages/core/test/reducers/core.test.ts b/packages/core/test/reducers/core.test.ts index df5e9a51e2..3f03756abc 100644 --- a/packages/core/test/reducers/core.test.ts +++ b/packages/core/test/reducers/core.test.ts @@ -595,6 +595,271 @@ test('core reducer - update - setting a state slice as undefined should remove t t.deepEqual(Object.keys(after.data), ['fizz']); }); +test('core reducer - update - numeric property key on nested object should not be treated as array index (#2397)', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + 'group-key': { + type: 'object', + properties: { + '15': { + type: 'string', + }, + }, + }, + }, + }; + + const before: JsonFormsCore = { + data: {}, + schema, + uischema: { + type: 'Label', + }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const after = coreReducer( + before, + update('group-key.15', () => 'something') + ); + + t.false(Array.isArray((after.data as any)['group-key'])); + t.deepEqual(after.data, { 'group-key': { '15': 'something' } }); +}); + +test('core reducer - update - property name containing brackets should be treated as a single key (#2102)', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + 'test[0]': { type: 'string' }, + 'object[0]': { + type: 'object', + properties: { + test: { type: 'string' }, + }, + }, + }, + }; + + const before: JsonFormsCore = { + data: {}, + schema, + uischema: { + type: 'Label', + }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const afterFirst = coreReducer( + before, + update('test[0]', () => 'TEST') + ); + t.deepEqual(afterFirst.data, { 'test[0]': 'TEST' }); + + const afterSecond = coreReducer( + afterFirst, + update('object[0].test', () => 'NESTED') + ); + t.deepEqual(afterSecond.data, { + 'test[0]': 'TEST', + 'object[0]': { test: 'NESTED' }, + }); +}); + +test('core reducer - update - array elements addressed by numeric segment continue to work', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + items: { + type: 'array', + items: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + }, + }, + }; + + const before: JsonFormsCore = { + data: { items: [{ name: 'a' }, { name: 'b' }] }, + schema, + uischema: { + type: 'Label', + }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const after = coreReducer( + before, + update('items.1.name', () => 'updated') + ); + + t.true(Array.isArray((after.data as any).items)); + t.is((after.data as any).items.length, 2); + t.deepEqual(after.data, { items: [{ name: 'a' }, { name: 'updated' }] }); +}); + +test('core reducer - update - rebuilds containers along the path while keeping sibling references untouched', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + parent: { + type: 'object', + properties: { + target: { type: 'string' }, + siblingObject: { + type: 'object', + properties: { value: { type: 'string' } }, + }, + }, + }, + untouchedObject: { + type: 'object', + properties: { value: { type: 'string' } }, + }, + }, + }; + + const siblingObject = { value: 'untouched-nested' }; + const untouchedObject = { value: 'untouched-root' }; + + const before: JsonFormsCore = { + data: { + parent: { target: 'old', siblingObject }, + untouchedObject, + }, + schema, + uischema: { type: 'Label' }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const after = coreReducer( + before, + update('parent.target', () => 'new') + ); + + // Containers on the path are fresh references + t.not(after.data, before.data); + t.not((after.data as any).parent, (before.data as any).parent); + // Siblings on those containers keep their original references + t.is((after.data as any).parent.siblingObject, siblingObject); + t.is((after.data as any).untouchedObject, untouchedObject); + t.is((after.data as any).parent.target, 'new'); +}); + +test('core reducer - update - creates an array when the schema declares one even if the segment is non-numeric in the parent', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + list: { + type: 'array', + items: { type: 'string' }, + }, + }, + }; + + const before: JsonFormsCore = { + data: {}, + schema, + uischema: { + type: 'Label', + }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const after = coreReducer( + before, + update('list.0', () => 'first') + ); + + t.true(Array.isArray((after.data as any).list)); + t.deepEqual(after.data, { list: ['first'] }); +}); + +test('core reducer - update - creates a schema-declared array of objects when the path traverses a missing array', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + users: { + type: 'array', + items: { + type: 'object', + properties: { name: { type: 'string' } }, + }, + }, + }, + }; + + const before: JsonFormsCore = { + data: {}, + schema, + uischema: { type: 'Label' }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const after = coreReducer( + before, + update('users.0.name', () => 'Alice') + ); + + t.true(Array.isArray((after.data as any).users)); + t.is((after.data as any).users.length, 1); + t.deepEqual(after.data, { users: [{ name: 'Alice' }] }); +}); + +test('core reducer - update - unset works for numeric and bracket-containing keys', (t) => { + const schema: JsonSchema = { + type: 'object', + properties: { + 'group-key': { + type: 'object', + properties: { + '15': { type: 'string' }, + 'non-numeric-key': { type: 'string' }, + }, + }, + 'test[0]': { type: 'string' }, + }, + }; + + const before: JsonFormsCore = { + data: { + 'group-key': { '15': 'fifteen', 'non-numeric-key': 'kept' }, + 'test[0]': 'TEST', + }, + schema, + uischema: { type: 'Label' }, + errors: [], + validator: new Ajv().compile(schema), + }; + + const afterNumeric = coreReducer( + before, + update('group-key.15', () => undefined) + ); + t.deepEqual(afterNumeric.data, { + 'group-key': { 'non-numeric-key': 'kept' }, + 'test[0]': 'TEST', + }); + + const afterBracket = coreReducer( + afterNumeric, + update('test[0]', () => undefined) + ); + t.deepEqual(afterBracket.data, { + 'group-key': { 'non-numeric-key': 'kept' }, + }); +}); + test('core reducer - updateErrors - should update errors with empty list', (t) => { const before: JsonFormsCore = { data: {}, diff --git a/packages/examples/src/examples/special-property-names.ts b/packages/examples/src/examples/special-property-names.ts new file mode 100644 index 0000000000..8e2aa938d7 --- /dev/null +++ b/packages/examples/src/examples/special-property-names.ts @@ -0,0 +1,133 @@ +/* + The MIT License + + Copyright (c) 2017-2019 EclipseSource Munich + https://github.com/eclipsesource/jsonforms + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. +*/ +import { registerExamples } from '../register'; + +export const schema = { + type: 'object', + properties: { + numericKeys: { + type: 'object', + title: 'Object with purely numeric property names', + properties: { + '15': { + type: 'string', + title: 'Property "15"', + }, + '42': { + type: 'string', + title: 'Property "42"', + }, + }, + }, + 'property[0]': { + type: 'string', + title: 'Top-level property containing brackets: "property[0]"', + }, + 'nested[0]': { + type: 'object', + title: 'Object property whose name contains brackets: "nested[0]"', + properties: { + value: { + type: 'string', + title: 'Inner value', + }, + }, + }, + 'dashed-key': { + type: 'object', + title: 'Mixed: dashed parent with numeric and dashed children', + properties: { + '15': { + type: 'string', + title: 'Numeric child "15"', + }, + 'non-numeric-key': { + type: 'string', + title: 'Dashed sibling', + }, + }, + }, + }, +}; + +export const uischema = { + type: 'VerticalLayout', + elements: [ + { + type: 'Group', + label: 'Numeric property names', + elements: [ + { + type: 'Control', + scope: '#/properties/numericKeys/properties/15', + }, + { + type: 'Control', + scope: '#/properties/numericKeys/properties/42', + }, + ], + }, + { + type: 'Group', + label: 'Property names containing brackets', + elements: [ + { + type: 'Control', + scope: '#/properties/property[0]', + }, + { + type: 'Control', + scope: '#/properties/nested[0]/properties/value', + }, + ], + }, + { + type: 'Group', + label: 'Mixed numeric and dashed property names', + elements: [ + { + type: 'Control', + scope: '#/properties/dashed-key/properties/15', + }, + { + type: 'Control', + scope: '#/properties/dashed-key/properties/non-numeric-key', + }, + ], + }, + ], +}; + +export const data = {}; + +registerExamples([ + { + name: 'special-property-names', + label: 'Special property names (numeric and bracket characters)', + data, + schema, + uischema, + }, +]); diff --git a/packages/examples/src/index.ts b/packages/examples/src/index.ts index 9bd232955f..cf75ae737f 100644 --- a/packages/examples/src/index.ts +++ b/packages/examples/src/index.ts @@ -77,6 +77,7 @@ import * as readonly from './examples/readonly'; import * as rule from './examples/rule'; import * as ruleInheritance from './examples/ruleInheritance'; import * as scope from './examples/scope'; +import * as specialPropertyNames from './examples/special-property-names'; import * as string from './examples/string'; import * as stringArray from './examples/stringArray'; import * as text from './examples/text'; @@ -145,6 +146,7 @@ export { rule, ruleInheritance, scope, + specialPropertyNames, stepper, steppershownav, string,