Skip to content

Commit dc02a1c

Browse files
sdirixdarius-lesch
andauthored
fix(core): refactor update data to avoid lodash issues (#2585)
Fixes #2102 Fixes #2397 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]"). Improvements on top of the lodash/fp/set replacement: - Store "__proto__" segments as own properties via defineProperty and traverse own properties only, so such keys neither corrupt the container's prototype nor get dropped. Clone containers key-by-key because downleveled object spreads assign instead of define. - Read the updater's old data with resolveData instead of lodash get, so reads use the same literal path semantics as writes. - Fall back to lodash's index heuristic when creating missing containers without schema type information. - Match lodash's isIndex semantics with a strict index regex instead of Number() coercion. - Avoid strict-mode TypeErrors when unsetting non-index array properties (e.g. "length") and return the same reference when there is nothing to unset. - Treat an empty path as addressing the root, and replace non-object root data instead of spreading it. Also documents the path semantics change in MIGRATION.md and adds unit tests for setDataAt/unsetDataAt. Co-authored-by: Darius Lesch <darius.lesch@proton.me>
1 parent 3326e7f commit dc02a1c

8 files changed

Lines changed: 859 additions & 9 deletions

File tree

MIGRATION.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
# Migration guide
22

3+
## Migration to JSON Forms 3.9
4+
5+
### Data update paths treat all segments literally
6+
7+
Data updates (e.g. dispatched `update` actions) previously wrote to the form data via lodash's `set`/`unset`, which interpret bracket notation and array indices in paths.
8+
This corrupted data for property names that look like lodash path syntax, for example numeric property names like `"15"` were turned into array indices and names containing brackets like `"prop[0]"` were split up (see [#2397](https://github.com/eclipsesource/jsonforms/issues/2397) and [#2102](https://github.com/eclipsesource/jsonforms/issues/2102)).
9+
10+
Updates now use the new `setDataAt`/`unsetDataAt` utilities of `@jsonforms/core`, which split paths on `.` and treat every segment as a literal property name, matching how JSON Forms resolves values for display.
11+
When a missing intermediate container is created, the JSON Schema decides whether it becomes an array or an object; without schema type information, a numeric follow-up segment creates an array, as before.
12+
13+
If you dispatch update actions yourself, make sure to use dot-separated paths (e.g. `update('list.0.name', ...)`) instead of lodash bracket syntax (e.g. `update('list[0].name', ...)`), which is no longer interpreted.
14+
315
## Migrating to JSON Forms 3.8
416

517
### `Translator` type changed from overloaded signatures to a generic conditional type

packages/core/src/reducers/core.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@
2424
*/
2525

2626
import cloneDeep from 'lodash/cloneDeep';
27-
import setFp from 'lodash/fp/set';
28-
import unsetFp from 'lodash/fp/unset';
29-
import get from 'lodash/get';
3027
import isEqual from 'lodash/isEqual';
28+
import { resolveData } from '../util/resolvers';
29+
import { setDataAt, unsetDataAt } from '../util/setData';
3130
import {
3231
CoreActions,
3332
INIT,
@@ -238,19 +237,20 @@ export const coreReducer: Reducer<JsonFormsCore, CoreActions> = (
238237
errors,
239238
};
240239
} else {
241-
const oldData: any = get(state.data, action.path);
240+
const oldData: any = resolveData(state.data, action.path);
242241
const newData = action.updater(cloneDeep(oldData));
243242
let newState: any;
244243
if (newData !== undefined) {
245-
newState = setFp(
244+
newState = setDataAt(
245+
state.data === undefined ? {} : state.data,
246246
action.path,
247247
newData,
248-
state.data === undefined ? {} : state.data
248+
state.schema
249249
);
250250
} else {
251-
newState = unsetFp(
252-
action.path,
253-
state.data === undefined ? {} : state.data
251+
newState = unsetDataAt(
252+
state.data === undefined ? {} : state.data,
253+
action.path
254254
);
255255
}
256256
const errors = validate(state.validator, newState);

packages/core/src/util/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export * from './ids';
2828
export * from './label';
2929
export * from './path';
3030
export * from './resolvers';
31+
export * from './setData';
3132
export * from './runtime';
3233
export * from './schema';
3334
export * from './uischema';

packages/core/src/util/setData.ts

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/*
2+
The MIT License
3+
4+
Copyright (c) 2017-2019 EclipseSource Munich
5+
https://github.com/eclipsesource/jsonforms
6+
7+
Permission is hereby granted, free of charge, to any person obtaining a copy
8+
of this software and associated documentation files (the "Software"), to deal
9+
in the Software without restriction, including without limitation the rights
10+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
11+
copies of the Software, and to permit persons to whom the Software is
12+
furnished to do so, subject to the following conditions:
13+
14+
The above copyright notice and this permission notice shall be included in
15+
all copies or substantial portions of the Software.
16+
17+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
18+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
19+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
20+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
21+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
22+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
23+
THE SOFTWARE.
24+
*/
25+
26+
import type { JsonSchema } from '../models';
27+
import { encode } from './path';
28+
import { resolveSchema } from './resolvers';
29+
import { deriveTypes, hasType } from './util';
30+
31+
const splitPath = (path: string): string[] => path.split('.');
32+
33+
/**
34+
* Whether the segment addresses an array element, i.e. is a canonical
35+
* non-negative integer without leading zeros.
36+
*/
37+
const isIndexSegment = (segment: string): boolean =>
38+
/^(?:0|[1-9]\d*)$/.test(segment);
39+
40+
/**
41+
* Walks one step in the schema along the given data path segment, so we can
42+
* tell whether a missing intermediate container should be created as an
43+
* array or as an object.
44+
*/
45+
const stepSchema = (
46+
schema: JsonSchema | undefined,
47+
segment: string,
48+
rootSchema: JsonSchema | undefined
49+
): JsonSchema | undefined => {
50+
if (!schema || !rootSchema) {
51+
return undefined;
52+
}
53+
const pointer = hasType(schema, 'array')
54+
? Array.isArray(schema.items)
55+
? `/items/${segment}`
56+
: '/items'
57+
: `/properties/${encode(segment)}`;
58+
return resolveSchema(schema, pointer, rootSchema);
59+
};
60+
61+
const assign = (container: any, segment: string, value: any): any => {
62+
if (segment === '__proto__') {
63+
// A plain assignment would overwrite the container's prototype instead
64+
// of creating an own property.
65+
Object.defineProperty(container, segment, {
66+
value,
67+
writable: true,
68+
enumerable: true,
69+
configurable: true,
70+
});
71+
} else {
72+
container[segment] = value;
73+
}
74+
return container;
75+
};
76+
77+
const cloneContainer = (data: any): any => {
78+
if (Array.isArray(data)) {
79+
return [...data];
80+
}
81+
if (typeof data === 'object' && data !== null) {
82+
// Not a spread because downleveled object spreads assign instead of
83+
// defining properties, which mishandles own "__proto__" properties.
84+
const clone: { [key: string]: any } = {};
85+
for (const key of Object.keys(data)) {
86+
assign(clone, key, data[key]);
87+
}
88+
return clone;
89+
}
90+
return {};
91+
};
92+
93+
/**
94+
* Looks up the own property `segment` of `data`, ignoring inherited
95+
* properties like `__proto__`, mirroring the semantics of `resolveData`.
96+
*/
97+
const ownPropertyValue = (data: any, segment: string): any =>
98+
data !== null &&
99+
data !== undefined &&
100+
Object.prototype.hasOwnProperty.call(data, segment)
101+
? data[segment]
102+
: undefined;
103+
104+
/**
105+
* Determines the container to create for a missing child. The schema takes
106+
* precedence; without any schema type information the next path segment
107+
* decides, mirroring the behavior of lodash's `set`.
108+
*/
109+
const createInitialContainer = (
110+
childSchema: JsonSchema | undefined,
111+
nextSegment: string
112+
): any => {
113+
const types = childSchema ? deriveTypes(childSchema) : [];
114+
if (types.includes('array')) {
115+
return [];
116+
}
117+
if (types.length > 0) {
118+
return {};
119+
}
120+
return isIndexSegment(nextSegment) ? [] : {};
121+
};
122+
123+
/**
124+
* Immutably sets `value` at the dotted `path` within `data`.
125+
*
126+
* Numeric path segments and segments containing bracket notation are treated
127+
* as plain object property names. The optional `rootSchema` is consulted when
128+
* a new intermediate container has to be created, so that arrays are still
129+
* created where the schema declares an array type. Without schema type
130+
* information, a canonical numeric follow-up segment creates an array.
131+
*
132+
* An empty `path` addresses the root, i.e. `value` itself is returned.
133+
*/
134+
export const setDataAt = (
135+
data: any,
136+
path: string,
137+
value: any,
138+
rootSchema?: JsonSchema
139+
): any => {
140+
if (path === '') {
141+
return value;
142+
}
143+
return doSet(data, splitPath(path), 0, value, rootSchema, rootSchema);
144+
};
145+
146+
const doSet = (
147+
data: any,
148+
segments: string[],
149+
index: number,
150+
value: any,
151+
currentSchema: JsonSchema | undefined,
152+
rootSchema: JsonSchema | undefined
153+
): any => {
154+
const segment = segments[index];
155+
const container = cloneContainer(data);
156+
157+
if (index === segments.length - 1) {
158+
return assign(container, segment, value);
159+
}
160+
161+
const childSchema = stepSchema(currentSchema, segment, rootSchema);
162+
const existingChild = ownPropertyValue(data, segment);
163+
const child =
164+
existingChild !== null && typeof existingChild === 'object'
165+
? existingChild
166+
: createInitialContainer(childSchema, segments[index + 1]);
167+
const nextValue = doSet(
168+
child,
169+
segments,
170+
index + 1,
171+
value,
172+
childSchema,
173+
rootSchema
174+
);
175+
return assign(container, segment, nextValue);
176+
};
177+
178+
/**
179+
* Immutably unsets the value at the dotted `path` within `data`.
180+
*
181+
* Numeric path segments and bracket notation in segments are treated as
182+
* plain object property names, mirroring the semantics of {@link setDataAt}.
183+
* Unsetting an array element leaves a hole, i.e. the array is not compacted.
184+
* If there is nothing to unset at `path`, `data` is returned unchanged.
185+
*/
186+
export const unsetDataAt = (data: any, path: string): any => {
187+
if (path === '') {
188+
return data;
189+
}
190+
return doUnset(data, splitPath(path), 0);
191+
};
192+
193+
const doUnset = (data: any, segments: string[], index: number): any => {
194+
if (data === null || typeof data !== 'object') {
195+
return data;
196+
}
197+
const segment = segments[index];
198+
if (index === segments.length - 1) {
199+
if (!Object.prototype.hasOwnProperty.call(data, segment)) {
200+
return data;
201+
}
202+
if (Array.isArray(data)) {
203+
// Non-index own properties of arrays (e.g. `length`) are not form data
204+
// and deleting them could throw in strict mode.
205+
if (!isIndexSegment(segment)) {
206+
return data;
207+
}
208+
const container = [...data];
209+
delete container[Number(segment)];
210+
return container;
211+
}
212+
const container = cloneContainer(data);
213+
delete container[segment];
214+
return container;
215+
}
216+
217+
const existingChild = ownPropertyValue(data, segment);
218+
if (existingChild === null || typeof existingChild !== 'object') {
219+
return data;
220+
}
221+
const nextValue = doUnset(existingChild, segments, index + 1);
222+
if (nextValue === existingChild) {
223+
return data;
224+
}
225+
return assign(cloneContainer(data), segment, nextValue);
226+
};

0 commit comments

Comments
 (0)