Skip to content

Commit 64012ea

Browse files
committed
fix #538
1 parent 53d4468 commit 64012ea

File tree

4 files changed

+25
-29
lines changed

4 files changed

+25
-29
lines changed

packages/core/src/metadata/IMetadataSubscription.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ export interface IMetadataSubscription {
2626
delete: () => void;
2727
/**
2828
* Called by the metadata manager when the cache receives an update that concerns this subscription.
29+
* This is NOT called with a clone of the value, so the subscription should not modify the value.
30+
* If the subscription can't guarantee that the value won't be modified, it should clone the value.
2931
*
3032
* @param value
3133
* @returns Whether the subscription has updated, i.e. the value was different from the current memorized value.

packages/core/src/metadata/MetadataManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ export class MetadataManager {
250250
const cacheItem = source.subscribe(subscription);
251251
cacheItem.cyclesWithoutListeners = 0;
252252

253-
subscription.onUpdate(source.readCacheItem(cacheItem, subscription.bindTarget.storageProp));
253+
const initialValue = source.readCacheItem(cacheItem, subscription.bindTarget.storageProp);
254+
subscription.onUpdate(initialValue);
254255
}
255256

256257
/**

packages/core/src/metadata/MetadataSubscription.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,19 @@ export class MetadataSubscription implements IMetadataSubscription {
5050
const currentValue = this.metadataManager.readShortLived(this.bindTarget);
5151
if (!areObjectsEqual(currentValue, value)) {
5252
this.value = value;
53+
// note: write already clones the value
5354
this.metadataManager.write(value, this.bindTarget, this.uuid);
5455
}
5556
}
5657

57-
/**
58-
* DO NOT CALL!
59-
*
60-
* Notifies the subscription of an updated value in the cache.
61-
*
62-
* @param value
63-
*/
6458
public onUpdate(value: unknown): boolean {
59+
// The value is NOT cloned, but that's fine for the comparison.
60+
// If we actually notify the callback, we need to clone the value, as we can't guarantee that the callback won't modify it.
6561
try {
6662
if (!areObjectsEqual(this.value, value)) {
67-
this.value = value;
68-
this.callbackSignal.set(value);
63+
const clonedValue = structuredClone(value);
64+
this.value = clonedValue;
65+
this.callbackSignal.set(clonedValue);
6966

7067
return true;
7168
}

packages/core/src/parsers/bindTargetParser/BindTargetParser.ts

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,17 @@ export class BindTargetParser {
5555
filePath: string,
5656
scope?: BindTargetScope,
5757
): BindTargetDeclaration {
58-
const bindTargetDeclaration: BindTargetDeclaration = {} as BindTargetDeclaration;
59-
60-
// listen to children
61-
bindTargetDeclaration.listenToChildren = unvalidatedBindTargetDeclaration.listenToChildren;
62-
6358
// storage prop
64-
bindTargetDeclaration.storageProp = new PropPath(
59+
const storageProp = new PropPath(
6560
unvalidatedBindTargetDeclaration.storageProp.map(x => new PropAccess(x.type, x.prop.value)),
6661
);
6762

6863
// storage type
64+
let storageType: string;
6965
if (unvalidatedBindTargetDeclaration.storageType === undefined) {
70-
bindTargetDeclaration.storageType = this.mb.metadataManager.defaultSource;
66+
storageType = this.mb.metadataManager.defaultSource;
7167
} else {
72-
bindTargetDeclaration.storageType = this.validateStorageType(
73-
unvalidatedBindTargetDeclaration.storageType,
74-
fullDeclaration,
75-
);
68+
storageType = this.validateStorageType(unvalidatedBindTargetDeclaration.storageType, fullDeclaration);
7669
}
7770

7871
// storage path
@@ -81,25 +74,28 @@ export class BindTargetParser {
8174
value: filePath,
8275
};
8376

84-
const source = this.mb.metadataManager.getSource(bindTargetDeclaration.storageType);
77+
const source = this.mb.metadataManager.getSource(storageType);
8578
if (source === undefined) {
8679
throw new MetaBindInternalError({
8780
errorLevel: ErrorLevel.CRITICAL,
8881
effect: 'can not validate bind target',
89-
cause: `Source '${bindTargetDeclaration.storageType}' not found. But validation was successful. This should not happen.`,
82+
cause: `Source '${storageType}' not found. But validation was successful. This should not happen.`,
9083
context: {
9184
fullDeclaration: fullDeclaration,
9285
sources: [...this.mb.metadataManager.sources.keys()],
9386
},
9487
});
9588
}
9689

97-
bindTargetDeclaration.storagePath = source.validateStoragePath(
98-
storagePathToValidate,
99-
hadStoragePath,
100-
fullDeclaration,
101-
this,
102-
);
90+
const storagePath = source.validateStoragePath(storagePathToValidate, hadStoragePath, fullDeclaration, this);
91+
92+
// construct bind target declaration
93+
const bindTargetDeclaration: BindTargetDeclaration = {
94+
storageType: storageType,
95+
storagePath: storagePath,
96+
storageProp: storageProp,
97+
listenToChildren: unvalidatedBindTargetDeclaration.listenToChildren,
98+
};
10399

104100
// eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
105101
if (source.id === BindTargetStorageType.SCOPE) {

0 commit comments

Comments
 (0)