Skip to content

Commit d5941d8

Browse files
authored
Merge pull request #765 from callstack-internal/feature/structural-sharing-cache-pr-2
[Structural Sharing] PR 2: Reference-stable fastMerge and removeNestedNullValues
2 parents 4bd871a + 8c998e6 commit d5941d8

3 files changed

Lines changed: 418 additions & 224 deletions

File tree

lib/utils.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ function mergeObject<TObject extends Record<string, unknown>>(
8989

9090
const targetObject = isMergeableObject(target) ? target : undefined;
9191

92+
// Track whether the merge actually changed anything compared to target.
93+
// If nothing changed, we return the original target reference for reference stability.
94+
let hasChanged = !targetObject;
95+
9296
// First we want to copy over all keys from the target into the destination object,
9397
// in case "target" is a mergable object.
9498
// If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object
@@ -103,6 +107,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
103107
const shouldOmitNullishProperty = options.shouldRemoveNestedNulls && (targetProperty === null || sourceProperty === null);
104108

105109
if (targetProperty === undefined || shouldOmitNullishProperty) {
110+
hasChanged = true;
106111
continue;
107112
}
108113

@@ -125,6 +130,9 @@ function mergeObject<TObject extends Record<string, unknown>>(
125130

126131
// If the source value is not a mergable object, we need to set the key directly.
127132
if (!isMergeableObject(sourceProperty)) {
133+
if (destination[key] !== sourceProperty) {
134+
hasChanged = true;
135+
}
128136
destination[key] = sourceProperty;
129137
continue;
130138
}
@@ -134,6 +142,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
134142
// To achieve this, we first mark these nested objects with an internal flag.
135143
// When calling fastMerge again with "mark" removal mode, the marked objects will be removed.
136144
if (options.objectRemovalMode === 'mark' && targetProperty === null) {
145+
hasChanged = true;
137146
targetProperty = {[ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true};
138147
metadata.replaceNullPatches.push([[...basePath, key], {...sourceProperty}]);
139148
}
@@ -142,6 +151,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
142151
// has the internal flag set, we replace the entire destination object with the source one and remove
143152
// the flag.
144153
if (options.objectRemovalMode === 'replace' && sourceProperty[ONYX_INTERNALS__REPLACE_OBJECT_MARK]) {
154+
hasChanged = true;
145155
// We do a spread here in order to have a new object reference and allow us to delete the internal flag
146156
// of the merged object only.
147157
const sourcePropertyWithoutMark = {...sourceProperty};
@@ -150,10 +160,14 @@ function mergeObject<TObject extends Record<string, unknown>>(
150160
continue;
151161
}
152162

153-
destination[key] = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result;
163+
const merged = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result;
164+
if (merged !== targetProperty) {
165+
hasChanged = true;
166+
}
167+
destination[key] = merged;
154168
}
155169

156-
return destination as TObject;
170+
return hasChanged ? (destination as TObject) : (targetObject as TObject);
157171
}
158172

159173
/** Checks whether the given object is an object and not null/undefined. */
@@ -170,35 +184,36 @@ function isMergeableObject<TObject extends Record<string, unknown>>(value: unkno
170184
return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value);
171185
}
172186

173-
/** Deep removes the nested null values from the given value. */
187+
/** Deep removes the nested null values from the given value. Returns the original reference if no nulls were found. */
174188
function removeNestedNullValues<TValue extends OnyxInput<OnyxKey> | null>(value: TValue): TValue {
175-
if (value === null || value === undefined || typeof value !== 'object') {
189+
if (value === null || value === undefined || typeof value !== 'object' || Array.isArray(value)) {
176190
return value;
177191
}
178192

179-
if (Array.isArray(value)) {
180-
return [...value] as TValue;
181-
}
182-
193+
let hasChanged = false;
183194
const result: Record<string, unknown> = {};
184195

185196
// eslint-disable-next-line no-restricted-syntax, guard-for-in
186197
for (const key in value) {
187198
const propertyValue = value[key];
188199

189200
if (propertyValue === null || propertyValue === undefined) {
201+
hasChanged = true;
190202
continue;
191203
}
192204

193205
if (typeof propertyValue === 'object' && !Array.isArray(propertyValue)) {
194-
const valueWithoutNestedNulls = removeNestedNullValues(propertyValue);
195-
result[key] = valueWithoutNestedNulls;
206+
const cleaned = removeNestedNullValues(propertyValue);
207+
if (cleaned !== propertyValue) {
208+
hasChanged = true;
209+
}
210+
result[key] = cleaned;
196211
} else {
197212
result[key] = propertyValue;
198213
}
199214
}
200215

201-
return result as TValue;
216+
return hasChanged ? (result as TValue) : value;
202217
}
203218

204219
/** Formats the action name by uppercasing and adding the key if provided. */

tests/unit/fastMergeTest.ts

Lines changed: 0 additions & 213 deletions
This file was deleted.

0 commit comments

Comments
 (0)