Skip to content

Commit

Permalink
Avoid using Object.isFrozen to prevent dev/prod differences.
Browse files Browse the repository at this point in the history
If you revert the previous commit and run `npm test`, you'll see all the
tests this dynamic Object.isFrozen check has been silently protecting
from failing, but only (and this is the important part) in development.

Since we only actually freeze objects with Object.freeze in development,
this Object.isFrozen check does not help in production, so an object
that would have been frozen in development gets reused as a mutable
copy, potentially acquiring properties it should not acquire (a bug
fixed by the previous commit, first reported in issue #9735).
  • Loading branch information
benjamn committed May 19, 2022
1 parent 466519f commit 9fc3818
Showing 1 changed file with 10 additions and 14 deletions.
24 changes: 10 additions & 14 deletions src/utilities/common/mergeDeep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,17 @@ export class DeepMerger<TContextArgs extends any[]> {

public shallowCopyForMerge<T>(value: T): T {
if (isNonNullObject(value)) {
if (this.pastCopies.has(value)) {
// In order to reuse a past copy, it must be mutable, but copied objects
// can sometimes be frozen while this DeepMerger is still active.
if (!Object.isFrozen(value)) return value;
this.pastCopies.delete(value);
}
if (Array.isArray(value)) {
value = (value as any).slice(0);
} else {
value = {
__proto__: Object.getPrototypeOf(value),
...value,
};
if (!this.pastCopies.has(value)) {
if (Array.isArray(value)) {
value = (value as any).slice(0);
} else {
value = {
__proto__: Object.getPrototypeOf(value),
...value,
};
}
this.pastCopies.add(value);
}
this.pastCopies.add(value);
}
return value;
}
Expand Down

0 comments on commit 9fc3818

Please sign in to comment.