From 4426c7dec31d110ee92f16f1524e2875156fdc77 Mon Sep 17 00:00:00 2001 From: Guido Grassel Date: Thu, 27 Apr 2023 07:43:09 +0200 Subject: [PATCH] C - Resubmit fixes and code hardening changes from multiple earier PRs to trunk and 3.2 rel. branch Signed-off-by: Guido Grassel Change-Id: Ibe154a5cd45434eb873a8de6baf6010a2a447e66 --- .../declarative_frontend/engine/stateMgmt.js | 406 ++++++++++++++---- .../src/lib/common/observed_object.ts | 5 +- .../pu_observed_property_abstract.ts | 10 +- .../pu_observed_property_object.ts | 48 ++- .../pu_observed_property_simple.ts | 35 +- .../pu_synced_property_object_nested.ts | 7 +- .../pu_synced_property_object_one_way.ts | 287 +++++++++++-- .../pu_synced_property_object_two_way.ts | 46 +- .../pu_synced_property_simple_one_way.ts | 30 +- .../pu_synced_property_simple_two_way.ts | 6 +- .../src/lib/partial_update/pu_view.ts | 26 +- .../state_mgmt/src/lib/sdk/environment.ts | 1 + 12 files changed, 708 insertions(+), 199 deletions(-) diff --git a/frameworks/bridge/declarative_frontend/engine/stateMgmt.js b/frameworks/bridge/declarative_frontend/engine/stateMgmt.js index 37dec1d5221..1aaa7dd9c4e 100644 --- a/frameworks/bridge/declarative_frontend/engine/stateMgmt.js +++ b/frameworks/bridge/declarative_frontend/engine/stateMgmt.js @@ -1435,6 +1435,7 @@ class Environment { prop = AppStorage.SetAndProp(key, tmp); this.props_.set(key, prop); + return true; } envProps(properties) { properties.forEach(property => { @@ -1827,6 +1828,10 @@ class SubscribableHandler { } }); } + // notify a property has been 'read' + // this functionality is in preparation for observed computed variables + // enable calling from 'get' trap handler functions to this function once + // adding support for observed computed variables notifyObjectPropertyHasBeenRead(propName) { this.owningProperties_.forEach((subscribedId) => { @@ -3049,7 +3054,7 @@ class ObservedPropertyAbstractPU extends ObservedPropertyAbstract { this.dependentElementIds_ = new Set(); } notifyPropertyRead() { - stateMgmtConsole.error(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.error(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: \ notifyPropertyRead, DO NOT USE with PU. Use notifyPropertyHasBeenReadPU`); } notifyPropertyHasBeenReadPU() { @@ -3076,7 +3081,7 @@ class ObservedPropertyAbstractPU extends ObservedPropertyAbstract { subscriber.syncPeerHasChanged(this); } else { - stateMgmtConsole.warn(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: notifyPropertryHasChangedPU: unknown subscriber ID '${subscribedId}' error!`); + stateMgmtConsole.warn(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: notifyPropertryHasChangedPU: unknown subscriber ID '${subscribedId}' error!`); } } }); @@ -3084,7 +3089,7 @@ class ObservedPropertyAbstractPU extends ObservedPropertyAbstract { markDependentElementsDirty(view) { // TODO ace-ets2bundle, framework, compilated apps need to update together // this function will be removed after a short transiition periode - stateMgmtConsole.warn(`markDependentElementsDirty no longer supported. App will work ok, but + stateMgmtConsole.warn(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: markDependentElementsDirty no longer supported. App will work ok, but please update your ace-ets2bundle and recompile your application!`); } /** @@ -3135,7 +3140,7 @@ class ObservedPropertyAbstractPU extends ObservedPropertyAbstract { ObservedPropertyAbstract. Need to overwrite these functions to do nothing for PU */ notifyHasChanged(_) { - stateMgmtConsole.error(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.error(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: \ notifyHasChanged, DO NOT USE with PU. Use syncPeerHasChanged() or objectPropertyHasChangedPU()`); } hasChanged(_) { @@ -3228,12 +3233,12 @@ class ObservedPropertySimpleAbstractPU extends ObservedPropertyAbstractPU { * property. */ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { - constructor(value, owningView, propertyName) { + constructor(localInitValue, owningView, propertyName) { super(owningView, propertyName); - this.setValueInternal(value); + this.setValueInternal(localInitValue); } aboutToBeDeleted(unsubscribeMe) { - this.unsubscribeFromOwningProperty(); + this.unsubscribeWrappedObject(); if (unsubscribeMe) { this.unlinkSuscriber(unsubscribeMe.id__()); } @@ -3260,7 +3265,7 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { this.notifyPropertyHasBeenReadPU(); } - unsubscribeFromOwningProperty() { + unsubscribeWrappedObject() { if (this.wrappedValue_) { if (this.wrappedValue_ instanceof SubscribaleAbstract) { this.wrappedValue_.removeOwningProperty(this); @@ -3276,11 +3281,20 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { and also notify with this.aboutToChange(); */ setValueInternal(newValue) { + if (newValue == undefined || newValue == null) { + stateMgmtConsole.error(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @State/@Provide value must not be undefined or null. Application error!`); + // TODO enable support for undefined and null + // unsubscribe old value, set wrappedValue_ to null/undefined + } if (typeof newValue !== 'object') { return false; } - this.unsubscribeFromOwningProperty(); + if (newValue == this.wrappedValue_) { + + return false; + } + this.unsubscribeWrappedObject(); if (ObservedObject.IsObservedObject(newValue)) { ObservedObject.addOwningProperty(newValue, this); @@ -3304,7 +3318,7 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { } getUnmonitored() { - // unmonitored get access , no call to otifyPropertyRead ! + // unmonitored get access , no call to notifyPropertyRead ! return this.wrappedValue_; } set(newValue) { @@ -3313,8 +3327,9 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { return; } - this.setValueInternal(newValue); - this.notifyPropertyHasChangedPU(); + if (this.setValueInternal(newValue)) { + this.notifyPropertyHasChangedPU(); + } } } /* @@ -3342,12 +3357,13 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectAbstractPU { * all definitions in this file are framework internal */ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU { - constructor(value, owningView, propertyName) { + constructor(localInitValue, owningView, propertyName) { super(owningView, propertyName); - if (typeof value === "object") { - throw new SyntaxError("ObservedPropertySimple value must not be an object"); + // TODO undefined and null support remove this if statement + if (typeof localInitValue === "object") { + throw new SyntaxError("ObservedPropertySimple constructor: @State/@Provide value must not be an object"); } - this.setValueInternal(value); + this.setValueInternal(localInitValue); } aboutToBeDeleted(unsubscribeMe) { if (unsubscribeMe) { @@ -3370,11 +3386,15 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU { */ setValueInternal(newValue) { - this.wrappedValue_ = newValue; + if (this.wrappedValue_ != newValue) { + this.wrappedValue_ = newValue; + return true; + } + return false; } getUnmonitored() { - // unmonitored get access , no call to otifyPropertyRead ! + // unmonitored get access , no call to notifyPropertyRead ! return this.wrappedValue_; } get() { @@ -3388,8 +3408,9 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU { return; } - this.setValueInternal(newValue); - this.notifyPropertyHasChangedPU(); + if (this.setValueInternal(newValue)) { + this.notifyPropertyHasChangedPU(); + } } } /* @@ -3406,28 +3427,77 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU { * See the License for the specific language governing permissions and * limitations under the License. */ +/** + * SynchedPropertyObjectOneWayPU + * implementation of @Prop decorated variables of type class object + * + * all definitions in this file are framework internal + * + */ +/** + * Initialisation scenarios: + * ------------------------- + * + * 1 - no local initialization, source provided (its ObservedObject value) + * wrap the ObservedObject into an ObservedPropertyObjectPU + * deep copy the ObservedObject into localCopyObservedObject_ + * + * 2 - local initialization, no source provided + * app transpiled code calls set + * leave source_ undefined + * no deep copy needed, but provided local init might need wrapping inside an ObservedObject to set to + * localCopyObservedObject_ + * + * 3 local initialization, source provided (its ObservedObject value) + * current app transpiled code is not optional + * sets source in constructor, as in case 1 + * calls set() to set the source value, but this will not deepcopy + * + * Update scenarios: + * ----------------- + * + * 1- assignment of a new Object value: this.aProp = new ClassA() + * rhs can be ObservedObject because of @Observed decoration or now + * notifyPropertryHasChangedPU + * + * 2- local ObservedObject member property change + * objectPropertyHasChangedPU called, eventSource is the ObservedObject stored in localCopyObservedObject_ + * no need to copy, notifyPropertryHasChangedPU + * + * 3- Rerender of the custom component triggered from the parent + * reset() is called (code generated by the transpiler), set the value of source_ , if that causes a change will call syncPeerHasChanged + * syncPeerHasChanged need to deep copy the ObservedObject from source to localCopyObservedObject_ + * notifyPropertryHasChangedPU + * + * 4- source_ ObservedObject member property change + * objectPropertyHasChangedPU called, eventSource is the ObservedObject stored source_.getUnmonitored + * notifyPropertryHasChangedPU + */ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU { constructor(source, owningChildView, thisPropertyName) { super(owningChildView, thisPropertyName); if (source && (typeof (source) === "object") && ("subscribeMe" in source)) { - // code path for @(Local)StorageProp, the souce is a ObservedPropertyObject in aLocalStorage) + // code path for @(Local)StorageProp, the source is a ObservedPropertyObject in a LocalStorage) this.source_ = source; this.sourceIsOwnObject = false; // subscribe to receive value change updates from LocalStorage source property this.source_.subscribeMe(this); } else { - // code path for @Prop + // code path for + // 1- source is of same type C in parent, not that the value(!) is provided, not the ObservedPropertyAbstract + // 2- nested Object/Array inside observed another object/array in parent if (!ObservedObject.IsObservedObject(source)) { stateMgmtConsole.warn(`@Prop ${this.info()} Provided source object's class lacks @Observed class decorator. Object property changes will not be observed.`); } - this.source_ = new ObservedPropertyObjectPU(source, this, thisPropertyName); + this.source_ = new ObservedPropertyObjectPU(source, this, this.getSourceObservedPropertyFakeName()); this.sourceIsOwnObject = true; } - // deep copy source Object and wrap it - this.setWrappedValue(this.source_.get()); + if (this.source_ != undefined) { + this.resetLocalValue(this.source_.get(), /* needCopyObject */ true); + } } /* @@ -3445,6 +3515,9 @@ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU { } super.aboutToBeDeleted(); } + getSourceObservedPropertyFakeName() { + return `${this.info()}_source`; + } syncPeerHasChanged(eventSource) { if (eventSource && this.source_ == eventSource) { // defensive programming: should always be the case! @@ -3452,12 +3525,13 @@ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU { const newValue = this.source_.getUnmonitored(); if (typeof newValue == "object") { - this.setWrappedValue(newValue); - this.notifyPropertyHasChangedPU(); + if (this.resetLocalValue(newValue, /* needCopyObject */ true)) { + this.notifyPropertyHasChangedPU(); + } } } else { - stateMgmtConsole.warn(`SynchedPropertyNesedObjectPU[${this.id__()}]: syncPeerHasChanged Unexpected situation. Ignorning event.`); + stateMgmtConsole.warn(`SynchedPropertyObjectOneWayPU[${this.id__()}]: syncPeerHasChanged Unexpected situation. Ignoring event.`); } } /** @@ -3465,58 +3539,207 @@ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU { * @param souceObject * @param changedPropertyName */ - objectPropertyHasChangedPU(souceObject, changedPropertyName) { + objectPropertyHasChangedPU(sourceObject, changedPropertyName) { this.notifyPropertyHasChangedPU(); } - objectPropertyHasBeenReadPU(souceObject, changedPropertyName) { + objectPropertyHasBeenReadPU(sourceObject, changedPropertyName) { this.notifyPropertyHasBeenReadPU(); } getUnmonitored() { // unmonitored get access , no call to notifyPropertyRead ! - return this.wrappedValue_; + return this.localCopyObservedObject_; } - // get 'read through` from the ObservedObject get() { this.notifyPropertyHasBeenReadPU(); - return this.wrappedValue_; + return this.localCopyObservedObject_; } // assignment to local variable in the form of this.aProp = // set 'writes through` to the ObservedObject set(newValue) { - if (this.wrappedValue_ == newValue) { + if (this.localCopyObservedObject_ == newValue) { return; } if (!ObservedObject.IsObservedObject(newValue)) { stateMgmtConsole.warn(`@Prop ${this.info()} Set: Provided new object's class - lacks @Observed class decorator. Object property changes will not be observed.`); + lacks '@Observed' class decorator. Object property changes will not be observed.`); + } + if (this.resetLocalValue(newValue, /* needCopyObject */ false)) { + this.notifyPropertyHasChangedPU(); } - this.setWrappedValue(newValue); - this.notifyPropertyHasChangedPU(); } + // called when updated from parent reset(sourceChangedValue) { - // if set causes an actual change, then, ObservedPropertyObject source_ will call syncPeerHasChanged - this.source_.set(sourceChangedValue); + if (this.source_ !== undefined) { + // if this.source_.set causes an actual change, then, ObservedPropertyObject source_ will call syncPeerHasChanged method + this.source_.set(sourceChangedValue); + } + else { + stateMgmtConsole.error(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: reset @Prop --- No source_. Internal error!`); + } + } + /* + unsubscribe from previous wrappped ObjectObject + take a shallow or (TODO) deep copy + copied Object might already be an ObservedObject (e.g. becuse of @Observed decroator) or might be raw + Therefore, conditionally wrap the object, then subscribe + return value true only if localCopyObservedObject_ has been changed + */ + resetLocalValue(newObservedObjectValue, needCopyObject) { + // note: We can not test for newObservedObjectValue == this.localCopyObservedObject_ + // here because the object might still be the same, but some property of it has changed + if (typeof newObservedObjectValue !== "object") { + // if not undefined or null, then the provided newObservedObjectValue must be an Object + stateMgmtConsole.error(`SynchedPropertyOneWayObjectPU[${this.id__()}]: setLocalValue new value must be an Object.`); + } + // unsubscribe from old wrappedValue ObservedOject + ObservedObject.removeOwningProperty(this.localCopyObservedObject_, this); + // shallow/deep copy value + // needed whenever newObservedObjectValue comes from source + // not needed on a local set (aka when called from set() method) + let copy = needCopyObject ? this.copyObject(newObservedObjectValue, this.info_) : newObservedObjectValue; + if (ObservedObject.IsObservedObject(copy)) { + // case: new ObservedObject + this.localCopyObservedObject_ = copy; + ObservedObject.addOwningProperty(this.localCopyObservedObject_, this); + } + else { + // wrap newObservedObjectValue raw object as ObservedObject and subscribe to it + stateMgmtConsole.warn(`@Prop ${this.info()} Provided source object's class \ + lacks @Observed class decorator. Object property changes will not be observed.`); + this.localCopyObservedObject_ = ObservedObject.createNew(copy, this); + } + return true; } - setWrappedValue(value) { + copyObject(value, propName) { + // ViewStackProcessor.getApiVersion function is not present in API9 + // therefore shallowCopyObject will always be used in API version 9 and before + // but the code in this file is the same regardless of API version + return ((typeof ViewStackProcessor["getApiVersion"] == "function") && + (ViewStackProcessor["getApiVersion"]() >= 10)) + ? this.deepCopyObject(value, propName) + : this.shallowCopyObject(value, propName); + } + // API 9 code path + shallowCopyObject(value, propName) { let rawValue = ObservedObject.GetRawObject(value); let copy; - // FIXME: Proper object deep copy missing here! - if (rawValue instanceof Array) { + if (rawValue == undefined || rawValue == null) { + copy = rawValue; + } + else if (typeof rawValue != "object") { + // TODO would it be better to crash the app here? + stateMgmtConsole.error(`@Prop ${this.info()} shallowCopyObject: request to copy non-object but defined value of type '${typeof rawValue}'. Internal error! Setting copy=original value.`); + copy = rawValue; + } + else if (rawValue instanceof Array) { + // case Array inside ObservedObject copy = ObservedObject.createNew([...rawValue], this); + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + } + else if (rawValue instanceof Date) { + // case Date inside ObservedObject + let d = new Date(); + d.setTime(rawValue.getTime()); + // subscribe, also Date gets wrapped / proxied by ObservedObject + copy = ObservedObject.createNew(d, this); + } + else if (rawValue instanceof SubscribaleAbstract) { + // case SubscriabableAbstract, no wrapping inside ObservedObject + copy = Object.assign({}, rawValue); + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + if (copy instanceof SubscribaleAbstract) { + // subscribe + copy.addOwningProperty(this); + } } - else { + else if (typeof rawValue == "object") { + // case Object that is not Array, not Date, not SubscribaleAbstract copy = ObservedObject.createNew(Object.assign({}, rawValue), this); + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + } + else { + // TODO in PR "F": change to exception throwing: + stateMgmtConsole.error(`@Prop ${this.info()} shallow failed. Attempt to copy unsupported value type '${typeof rawValue}' .`); + copy = rawValue; + } + return copy; + } + // API 10 code path + deepCopyObject(obj, variable) { + let copy = SynchedPropertyObjectOneWayPU.deepCopyObjectInternal(obj, variable); + // this subscribe to the top level object/array of the copy + // same as shallowCopy does + if ((obj instanceof SubscribaleAbstract) && + (copy instanceof SubscribaleAbstract)) { + copy.addOwningProperty(this); + } + else if (ObservedObject.IsObservedObject(obj) && ObservedObject.IsObservedObject(copy)) { + ObservedObject.addOwningProperty(copy, this); + } + return copy; + ; + } + // do not use this function from outside unless it is for testing purposes. + static deepCopyObjectInternal(obj, variable) { + if (!obj || typeof obj !== 'object') { + return obj; + } + let stack = new Array(); + let copiedObjects = new Map(); + return getDeepCopyOfObjectRecursive(obj); + function getDeepCopyOfObjectRecursive(obj) { + if (!obj || typeof obj !== 'object') { + return obj; + } + const alreadyCopiedObject = copiedObjects.get(obj); + if (alreadyCopiedObject) { + let msg = `@Prop deepCopyObject: Found reference to already copied object: Path ${variable ? variable : 'unknown variable'}`; + stack.forEach(stackItem => msg += ` - ${stackItem.name}`); + + return alreadyCopiedObject; + } + let copy; + if (obj instanceof Set) { + copy = new Set(); + for (const setKey of obj.keys()) { + stack.push({ name: setKey }); + copiedObjects.set(obj, copy); + copy.add(getDeepCopyOfObjectRecursive(setKey)); + stack.pop(); + } + } + else if (obj instanceof Map) { + copy = new Map(); + for (const mapKey of obj.keys()) { + stack.push({ name: mapKey }); + copiedObjects.set(obj, copy); + copy.set(mapKey, getDeepCopyOfObjectRecursive(obj.get(mapKey))); + stack.pop(); + } + } + else if (obj instanceof Date) { + copy = new Date(); + copy.setTime(obj.getTime()); + } + else if (obj instanceof Object) { + copy = Array.isArray(obj) ? [] : {}; + Object.setPrototypeOf(copy, Object.getPrototypeOf(obj)); + for (const objKey of Object.keys(obj)) { + stack.push({ name: objKey }); + copiedObjects.set(obj, copy); + Reflect.set(copy, objKey, getDeepCopyOfObjectRecursive(obj[objKey])); + stack.pop(); + } + } + return ObservedObject.IsObservedObject(obj) ? ObservedObject.createNew(copy, null) : copy; } - Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); - ObservedObject.addOwningProperty(this.wrappedValue_, this); - this.wrappedValue_ = copy; } } /* @@ -3540,16 +3763,19 @@ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU { * all definitions in this file are framework internal */ class SynchedPropertyObjectTwoWayPU extends ObservedPropertyObjectAbstractPU { - constructor(linkSource, owningChildView, thisPropertyName) { + constructor(source, owningChildView, thisPropertyName) { super(owningChildView, thisPropertyName); this.changeNotificationIsOngoing_ = false; - this.linkedParentProperty_ = linkSource; - if (this.linkedParentProperty_) { + this.source_ = source; + if (this.source_) { // register to the parent property - this.linkedParentProperty_.subscribeMe(this); + this.source_.subscribeMe(this); + // register to the ObservedObject + ObservedObject.addOwningProperty(this.source_.get(), this); + } + else { + stateMgmtConsole.error(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @Link/@Consume source must not be undefined. Application error!`); } - // register to the ObservedObject - ObservedObject.addOwningProperty(this.linkedParentProperty_.get(), this); } /* like a destructor, need to call this before deleting @@ -3557,19 +3783,24 @@ class SynchedPropertyObjectTwoWayPU extends ObservedPropertyObjectAbstractPU { */ aboutToBeDeleted() { // unregister from parent of this link - if (this.linkedParentProperty_) { - this.linkedParentProperty_.unlinkSuscriber(this.id__()); + if (this.source_) { + this.source_.unlinkSuscriber(this.id__()); // unregister from the ObservedObject - ObservedObject.removeOwningProperty(this.linkedParentProperty_.getUnmonitored(), this); + ObservedObject.removeOwningProperty(this.source_.getUnmonitored(), this); } super.aboutToBeDeleted(); } setObject(newValue) { - if (!this.linkedParentProperty_) { - stateMgmtConsole.warn(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: setObject, no linked parent property.`); + if (!this.source_) { + stateMgmtConsole.warn(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: setObject (assign a new value), @Link/@Consume: no linked parent property. Likely a consequence of earlier application error.`); return; } - this.linkedParentProperty_.set(newValue); + let oldValueObject = this.getUnmonitored(); + if (oldValueObject != undefined && oldValueObject != null) { + ObservedObject.removeOwningProperty(oldValueObject, this); + } + this.source_.set(newValue); + ObservedObject.addOwningProperty(this.getUnmonitored(), this); } /** * Called when sync peer ObservedPropertyObject or SynchedPropertyObjectTwoWay has chnaged value @@ -3599,7 +3830,7 @@ class SynchedPropertyObjectTwoWayPU extends ObservedPropertyObjectAbstractPU { getUnmonitored() { // unmonitored get access , no call to otifyPropertyRead ! - return (this.linkedParentProperty_ ? this.linkedParentProperty_.getUnmonitored() : undefined); + return (this.source_ ? this.source_.getUnmonitored() : undefined); } // get 'read through` from the ObservedProperty get() { @@ -3614,11 +3845,9 @@ class SynchedPropertyObjectTwoWayPU extends ObservedPropertyObjectAbstractPU { return; } - ObservedObject.removeOwningProperty(this.getUnmonitored(), this); // avoid circular notifications @Link -> source @State -> other but also back to same @Link this.changeNotificationIsOngoing_ = true; this.setObject(newValue); - ObservedObject.addOwningProperty(this.getUnmonitored(), this); this.notifyPropertyHasChangedPU(); this.changeNotificationIsOngoing_ = false; } @@ -3650,7 +3879,7 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU { // code path for @(Local)StorageProp this.source_ = source; this.sourceIsOwnObject = false; - // subscribe to receive value chnage updates from LocalStorge source property + // subscribe to receive value change updates from LocalStorage source property this.source_.subscribeMe(this); } else { @@ -3682,17 +3911,13 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU { if (eventSource && (eventSource == this.source_)) { // defensive, should always be the case - this.sourceHasChanged(eventSource); + this.setWrappedValue(eventSource.getUnmonitored()); + this.notifyPropertyHasChangedPU(); } } - sourceHasChanged(eventSource) { - - this.wrappedValue_ = eventSource.getUnmonitored(); - this.notifyPropertyHasChangedPU(); - } getUnmonitored() { - // unmonitored get access , no call to otifyPropertyRead ! + // unmonitored get access, no call to notifyPropertyRead ! return this.wrappedValue_; } // get 'read through` from the ObservedProperty @@ -3707,13 +3932,19 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU { return; } - this.wrappedValue_ = newValue; + this.setWrappedValue(newValue); this.notifyPropertyHasChangedPU(); } reset(sourceChangedValue) { // if set causes an actual change, then, ObservedPropertySimple source_ will call hasChanged - this.source_.set(sourceChangedValue); + if (this.source_ !== undefined) { + // if set causes an actual change, then, ObservedPropertySimple source_ will call hasChanged + this.source_.set(sourceChangedValue); + } + } + setWrappedValue(newValue) { + this.wrappedValue_ = newValue; } } /* @@ -3741,7 +3972,12 @@ class SynchedPropertySimpleTwoWayPU extends ObservedPropertySimpleAbstractPU { super(owningView, owningViewPropNme); this.changeNotificationIsOngoing_ = false; this.source_ = source; - this.source_.subscribeMe(this); + if (this.source_) { + this.source_.subscribeMe(this); + } + else { + stateMgmtConsole.error(`SynchedPropertySimpleTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @Link/@Consume source must not be undefined. Application error!`); + } } /* like a destructor, need to call this before deleting @@ -3850,14 +4086,13 @@ class SynchedPropertyNesedObjectPU extends ObservedPropertyObjectAbstractPU { this.notifyPropertyHasBeenReadPU(); } getUnmonitored() { - // + // unmonitored get access , no call to otifyPropertyRead ! return this.obsObject_; } // get 'read through` from the ObservedProperty get() { - // this.notifyPropertyRead(); this.notifyPropertyHasBeenReadPU(); return this.obsObject_; } @@ -3920,6 +4155,8 @@ class ViewPU extends NativeViewPartialUpdate { super(); this.parent_ = undefined; this.childrenWeakrefMap_ = new Map(); + // flag for initgial rendering or re-render on-going. + this.isRenderInProgress = false; this.watchedProps = new Map(); // Set of dependent elmtIds that need partial update // during next re-render @@ -4043,9 +4280,16 @@ class ViewPU extends NativeViewPartialUpdate { stateMgmtConsole.warn("ViewPU.updateStateVars unimplemented. Pls upgrade to latest eDSL transpiler version."); } initialRenderView() { + this.isRenderInProgress = true; this.initialRender(); + this.isRenderInProgress = false; } UpdateElement(elmtId) { + if (elmtId == this.id__()) { + // do not attempt to update itself. + // a @Prop can add a dependency of the ViewPU onto itself. Ignore it. + return; + } // do not process an Element that has been marked to be deleted const updateFunc = this.updateFuncByElmtId.get(elmtId); if ((updateFunc == undefined) || (typeof updateFunc !== "function")) { @@ -4053,11 +4297,13 @@ class ViewPU extends NativeViewPartialUpdate { } else { + this.isRenderInProgress = true; updateFunc(elmtId, /* isFirstRender */ false); // continue in native JSView // Finish the Update in JSView::JsFinishUpdateFunc // this function appends no longer used elmtIds (as recrded by VSP) to the given allRmElmtIds array this.finishUpdateFunc(elmtId); + this.isRenderInProgress = false; } } @@ -4123,6 +4369,9 @@ class ViewPU extends NativeViewPartialUpdate { // implements IMultiPropertiesChangeSubscriber viewPropertyHasChanged(varName, dependentElmtIds) { stateMgmtTrace.scopedTrace(() => { + if (this.isRenderInProgress) { + stateMgmtConsole.error(`@Component '${this.constructor.name}' (id: ${this.id__()}) State variable '${varName}' has changed during render! It's illegal to change @Component state while build (initial render or re-render) is on-going. Application error!`); + } this.syncInstanceId(); if (dependentElmtIds.size && !this.isFirstRender()) { @@ -4132,8 +4381,9 @@ class ViewPU extends NativeViewPartialUpdate { this.markNeedUpdate(); } - const union = new Set([...this.dirtDescendantElementIds_, ...dependentElmtIds]); - this.dirtDescendantElementIds_ = union; + for (const elmtId of dependentElmtIds) { + this.dirtDescendantElementIds_.add(elmtId); + } } let cb = this.watchedProps.get(varName); @@ -4221,6 +4471,10 @@ class ViewPU extends NativeViewPartialUpdate { this.UpdateElement(elmtId); this.dirtDescendantElementIds_.delete(elmtId); }); + if (this.dirtDescendantElementIds_.size) { + stateMgmtConsole.error(`@Component '${this.constructor.name}' (id: ${this.id__()}): New UINode objects added to update queue while re-render! \ + Likely caused by @Component state change during build phase, not allowed. Application error!`); + } } while (this.dirtDescendantElementIds_.size); } // given a list elementIds removes these from state variables dependency list and from elmtId -> updateFunc map diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/common/observed_object.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/common/observed_object.ts index 4e24d220a9a..141e4f7e241 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/common/observed_object.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/common/observed_object.ts @@ -153,7 +153,10 @@ class SubscribableHandler { }); } - + // notify a property has been 'read' + // this functionality is in preparation for observed computed variables + // enable calling from 'get' trap handler functions to this function once + // adding support for observed computed variables protected notifyObjectPropertyHasBeenRead(propName: string) { stateMgmtConsole.debug(`SubscribableHandler: notifyObjectPropertyHasBeenRead '${propName}'.`) this.owningProperties_.forEach((subscribedId) => { diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_abstract.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_abstract.ts index f66d379fac5..b38f93811ce 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_abstract.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_abstract.ts @@ -30,7 +30,7 @@ implements ISinglePropertyChangeSubscriber, IMultiPropertiesChangeSubscriber, } protected notifyPropertyRead() { - stateMgmtConsole.error(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.error(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: \ notifyPropertyRead, DO NOT USE with PU. Use notifyPropertyHasBeenReadPU`); } @@ -57,7 +57,7 @@ implements ISinglePropertyChangeSubscriber, IMultiPropertiesChangeSubscriber, } else if ('syncPeerHasChanged' in subscriber) { (subscriber as unknown as PeerChangeEventReceiverPU).syncPeerHasChanged(this); } else { - stateMgmtConsole.warn(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: notifyPropertryHasChangedPU: unknown subscriber ID '${subscribedId}' error!`); + stateMgmtConsole.warn(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: notifyPropertryHasChangedPU: unknown subscriber ID '${subscribedId}' error!`); } } }); @@ -67,7 +67,7 @@ implements ISinglePropertyChangeSubscriber, IMultiPropertiesChangeSubscriber, public markDependentElementsDirty(view: ViewPU) { // TODO ace-ets2bundle, framework, compilated apps need to update together // this function will be removed after a short transiition periode - stateMgmtConsole.warn(`markDependentElementsDirty no longer supported. App will work ok, but + stateMgmtConsole.warn(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: markDependentElementsDirty no longer supported. App will work ok, but please update your ace-ets2bundle and recompile your application!`); } @@ -104,7 +104,7 @@ implements ISinglePropertyChangeSubscriber, IMultiPropertiesChangeSubscriber, public purgeDependencyOnElmtId(rmElmtId: number): void { - stateMgmtConsole.debug(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']:purgeDependencyOnElmtId ${rmElmtId}`); + stateMgmtConsole.debug(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']:purgeDependencyOnElmtId ${rmElmtId}`); this.dependentElementIds_.delete(rmElmtId); } @@ -130,7 +130,7 @@ implements ISinglePropertyChangeSubscriber, IMultiPropertiesChangeSubscriber, ObservedPropertyAbstract. Need to overwrite these functions to do nothing for PU */ protected notifyHasChanged(_: T) { - stateMgmtConsole.error(`ObservedPropertyAbstract[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.error(`ObservedPropertyAbstractPU[${this.id__()}, '${this.info() || "unknown"}']: \ notifyHasChanged, DO NOT USE with PU. Use syncPeerHasChanged() or objectPropertyHasChangedPU()`); } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_object.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_object.ts index 95a3e4c021a..4983c98e46a 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_object.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_object.ts @@ -30,13 +30,14 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectA private wrappedValue_: T; - constructor(value: T, owningView: IPropertySubscriber, propertyName: PropertyInfo) { + constructor(localInitValue: T, owningView: IPropertySubscriber, propertyName: PropertyInfo) { super(owningView, propertyName); - this.setValueInternal(value); + + this.setValueInternal(localInitValue); } aboutToBeDeleted(unsubscribeMe?: IPropertySubscriber) { - this.unsubscribeFromOwningProperty(); + this.unsubscribeWrappedObject(); if (unsubscribeMe) { this.unlinkSuscriber(unsubscribeMe.id__()); } @@ -48,7 +49,7 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectA * @param eventSource */ syncPeerHasChanged(eventSource : ObservedPropertyAbstractPU) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: syncPeerHasChanged peer '${eventSource.info()}'.`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: syncPeerHasChanged peer '${eventSource.info()}'.`); this.notifyPropertyHasChangedPU(); } @@ -58,19 +59,18 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectA * @param changedPropertyName */ public objectPropertyHasChangedPU(souceObject: ObservedObject, changedPropertyName : string) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: \ objectPropertyHasChangedPU: contained ObservedObject property '${changedPropertyName}' has changed.`) this.notifyPropertyHasChangedPU(); } - public objectPropertyHasBeenReadPU(souceObject: ObservedObject, changedPropertyName : string) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: \ + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: \ objectPropertyHasBeenReadPU: contained ObservedObject property '${changedPropertyName}' has been read.`); this.notifyPropertyHasBeenReadPU(); } - private unsubscribeFromOwningProperty() { + private unsubscribeWrappedObject() { if (this.wrappedValue_) { if (this.wrappedValue_ instanceof SubscribaleAbstract) { (this.wrappedValue_ as SubscribaleAbstract).removeOwningProperty(this); @@ -79,53 +79,65 @@ class ObservedPropertyObjectPU extends ObservedPropertyObjectA } } } + /* actually update this.wrappedValue_ called needs to do value change check and also notify with this.aboutToChange(); */ private setValueInternal(newValue: T): boolean { + if (newValue== undefined || newValue==null) { + stateMgmtConsole.error(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @State/@Provide value must not be undefined or null. Application error!`); + // TODO enable support for undefined and null + // unsubscribe old value, set wrappedValue_ to null/undefined + } if (typeof newValue !== 'object') { stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}'] new value is NOT an object. Application error. Ignoring set.`); return false; } + + if (newValue == this.wrappedValue_){ + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}'] newValue unchanged`); + return false; + } - this.unsubscribeFromOwningProperty(); + this.unsubscribeWrappedObject(); if (ObservedObject.IsObservedObject(newValue)) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}'] new value is an ObservedObject already`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}'] new value is an ObservedObject already`); ObservedObject.addOwningProperty(newValue, this); this.wrappedValue_ = newValue; } else if (newValue instanceof SubscribaleAbstract) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}'] new value is an SubscribaleAbstract, subscribiung to it.`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}'] new value is an SubscribaleAbstract, subscribiung to it.`); this.wrappedValue_ = newValue; (this.wrappedValue_ as unknown as SubscribaleAbstract).addOwningProperty(this); } else { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}'] new value is an Object, needs to be wrapped in an ObservedObject.`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}'] new value is an Object, needs to be wrapped in an ObservedObject.`); this.wrappedValue_ = ObservedObject.createNew(newValue, this); } return true; } public get(): T { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: get`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: get`); this.notifyPropertyHasBeenReadPU(); return this.wrappedValue_; } public getUnmonitored(): T { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored returns '${JSON.stringify(this.wrappedValue_)}' .`); - // unmonitored get access , no call to otifyPropertyRead ! + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); + // unmonitored get access , no call to notifyPropertyRead ! return this.wrappedValue_; } public set(newValue: T): void { if (this.wrappedValue_ == newValue) { - stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: set with unchanged value - ignoring.`); + stateMgmtConsole.debug(`ObservedPropertyObjectPU[${this.id__()}, '${this.info() || "unknown"}']: set with unchanged value - ignoring.`); return; } stateMgmtConsole.debug(`ObservedPropertyObject[${this.id__()}, '${this.info() || "unknown"}']: set, changed`); - this.setValueInternal(newValue); - this.notifyPropertyHasChangedPU(); + if (this.setValueInternal(newValue)) { + this.notifyPropertyHasChangedPU(); + } } } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_simple.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_simple.ts index 578b6f96e5a..5c1e2776c1c 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_simple.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_observed_property_simple.ts @@ -28,12 +28,15 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU private wrappedValue_: T; - constructor(value: T, owningView: IPropertySubscriber, propertyName: PropertyInfo) { + constructor(localInitValue: T, owningView: IPropertySubscriber, propertyName: PropertyInfo) { super(owningView, propertyName); - if (typeof value === "object") { - throw new SyntaxError("ObservedPropertySimple value must not be an object")! + + // TODO undefined and null support remove this if statement + if (typeof localInitValue === "object") { + throw new SyntaxError("ObservedPropertySimple constructor: @State/@Provide value must not be an object")! } - this.setValueInternal(value); + + this.setValueInternal(localInitValue); } aboutToBeDeleted(unsubscribeMe?: IPropertySubscriber) { @@ -57,19 +60,23 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU called needs to do value change check and also notify with this.aboutToChange(); */ - private setValueInternal(newValue: T): void { - stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}'] new value is of simple type`); - this.wrappedValue_ = newValue; + private setValueInternal(newValue: T): boolean { + stateMgmtConsole.debug(`ObservedPropertySimplePU[${this.id__()}, '${this.info() || "unknown"}'] set new value`); + if (this.wrappedValue_ != newValue) { + this.wrappedValue_ = newValue; + return true; + } + return false; } public getUnmonitored(): T { - stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored returns '${JSON.stringify(this.wrappedValue_)}' .`); - // unmonitored get access , no call to otifyPropertyRead ! + stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); + // unmonitored get access , no call to notifyPropertyRead ! return this.wrappedValue_; } public get(): T { - stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: get returns '${JSON.stringify(this.wrappedValue_)}' .`); + stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: get.`); this.notifyPropertyHasBeenReadPU() return this.wrappedValue_; } @@ -79,9 +86,9 @@ class ObservedPropertySimplePU extends ObservedPropertySimpleAbstractPU stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: set with unchanged value - ignoring.`); return; } - stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: set, changed from '${JSON.stringify(this.wrappedValue_)}' to '${JSON.stringify(newValue)}.`); - this.setValueInternal(newValue); - this.notifyPropertyHasChangedPU(); - + stateMgmtConsole.debug(`ObservedPropertySimple[${this.id__()}, '${this.info() || "unknown"}']: set changed value.`); + if (this.setValueInternal(newValue)) { + this.notifyPropertyHasChangedPU(); + } } } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_nested.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_nested.ts index 2e8dc3a6cdb..2233d810409 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_nested.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_nested.ts @@ -68,7 +68,7 @@ class SynchedPropertyNesedObjectPU } public getUnmonitored(): C { - // stateMgmtConsole.debug(`SynchedPropertyNesedObject[${this.id()}, '${this.info() || "unknown"}']: getUnmonitored returns '${JSON.stringify(this.wrappedValue_)}' .`); + stateMgmtConsole.debug(`SynchedPropertyNesedObject[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); // unmonitored get access , no call to otifyPropertyRead ! return this.obsObject_; } @@ -76,7 +76,6 @@ class SynchedPropertyNesedObjectPU // get 'read through` from the ObservedProperty public get(): C { stateMgmtConsole.debug(`SynchedPropertyNesedObjectPU[${this.id__()}, '${this.info() || "unknown"}']: get`) - // this.notifyPropertyRead(); this.notifyPropertyHasBeenReadPU() return this.obsObject_; } @@ -84,11 +83,11 @@ class SynchedPropertyNesedObjectPU // set 'writes through` to the ObservedProperty public set(newValue: C): void { if (this.obsObject_ == newValue) { - stateMgmtConsole.debug(`SynchedPropertyNesedObjectPu[${this.id__()}IP, '${this.info() || "unknown"}']: set with unchanged value '${newValue}'- ignoring.`); + stateMgmtConsole.debug(`SynchedPropertyNesedObjectPu[${this.id__()}IP, '${this.info() || "unknown"}']: set @ObjectLink with unchanged value - nothing to do.`); return; } - stateMgmtConsole.debug(`SynchedPropertyNesedObjectPU[${this.id__()}, '${this.info() || "unknown"}']: set to newValue: '${newValue}'.`); + stateMgmtConsole.debug(`SynchedPropertyNesedObjectPU[${this.id__()}, '${this.info() || "unknown"}']: set: @ObjectLink set to new value: .`); // unsubscribe from the old value ObservedObject ObservedObject.removeOwningProperty(this.obsObject_, this); diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_one_way.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_one_way.ts index 6f0653093a0..ad4f20dbb2c 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_one_way.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_one_way.ts @@ -15,21 +15,60 @@ /** * SynchedPropertyObjectOneWayPU - * implementatio of @Prop decorated variables of type class object + * implementation of @Prop decorated variables of type class object * * all definitions in this file are framework internal * */ - -type SyncSourceHasChangedCb = (source : ObservedPropertyAbstract) => void; - +/** + * Initialisation scenarios: + * ------------------------- + * + * 1 - no local initialization, source provided (its ObservedObject value) + * wrap the ObservedObject into an ObservedPropertyObjectPU + * deep copy the ObservedObject into localCopyObservedObject_ + * + * 2 - local initialization, no source provided + * app transpiled code calls set + * leave source_ undefined + * no deep copy needed, but provided local init might need wrapping inside an ObservedObject to set to + * localCopyObservedObject_ + * + * 3 local initialization, source provided (its ObservedObject value) + * current app transpiled code is not optional + * sets source in constructor, as in case 1 + * calls set() to set the source value, but this will not deepcopy + * + * Update scenarios: + * ----------------- + * + * 1- assignment of a new Object value: this.aProp = new ClassA() + * rhs can be ObservedObject because of @Observed decoration or now + * notifyPropertryHasChangedPU + * + * 2- local ObservedObject member property change + * objectPropertyHasChangedPU called, eventSource is the ObservedObject stored in localCopyObservedObject_ + * no need to copy, notifyPropertryHasChangedPU + * + * 3- Rerender of the custom component triggered from the parent + * reset() is called (code generated by the transpiler), set the value of source_ , if that causes a change will call syncPeerHasChanged + * syncPeerHasChanged need to deep copy the ObservedObject from source to localCopyObservedObject_ + * notifyPropertryHasChangedPU + * + * 4- source_ ObservedObject member property change + * objectPropertyHasChangedPU called, eventSource is the ObservedObject stored source_.getUnmonitored + * notifyPropertryHasChangedPU + */ class SynchedPropertyObjectOneWayPU extends ObservedPropertyObjectAbstractPU implements PeerChangeEventReceiverPU, ObservedObjectEventsPUReceiver { - private wrappedValue_: C; + // the locally modified ObservedObject + private localCopyObservedObject_: C; + + // reference to the source variable in parent component private source_: ObservedPropertyAbstract; // true for @Prop code path, @@ -42,26 +81,28 @@ class SynchedPropertyObjectOneWayPU super(owningChildView, thisPropertyName); if (source && (typeof (source) === "object") && ("subscribeMe" in source)) { - // code path for @(Local)StorageProp, the souce is a ObservedPropertyObject in aLocalStorage) + // code path for @(Local)StorageProp, the source is a ObservedPropertyObject in a LocalStorage) this.source_ = source as ObservedPropertyAbstractPU; this.sourceIsOwnObject = false; // subscribe to receive value change updates from LocalStorage source property this.source_.subscribeMe(this); } else { - // code path for @Prop + // code path for + // 1- source is of same type C in parent, not that the value(!) is provided, not the ObservedPropertyAbstract + // 2- nested Object/Array inside observed another object/array in parent if (!ObservedObject.IsObservedObject(source)) { stateMgmtConsole.warn(`@Prop ${this.info()} Provided source object's class lacks @Observed class decorator. Object property changes will not be observed.`); } stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @Prop wrapping source in a new ObservedPropertyObjectPU`); - this.source_ = new ObservedPropertyObjectPU(source as C, this, thisPropertyName); + this.source_ = new ObservedPropertyObjectPU(source as C, this, this.getSourceObservedPropertyFakeName()); this.sourceIsOwnObject = true; } - // deep copy source Object and wrap it - this.setWrappedValue(this.source_.get()); - - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor ready with wrappedValue '${JSON.stringify(this.wrappedValue_)}'.`); + if (this.source_ != undefined) { + this.resetLocalValue(this.source_.get(), /* needCopyObject */ true); + } + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor ready with local copy.`); } /* @@ -81,18 +122,23 @@ class SynchedPropertyObjectOneWayPU super.aboutToBeDeleted(); } + private getSourceObservedPropertyFakeName() : string { + return `${this.info()}_source`; + } + public syncPeerHasChanged(eventSource: ObservedPropertyAbstractPU) { if (eventSource && this.source_ == eventSource) { // defensive programming: should always be the case! - stateMgmtConsole.debug(`SynchedPropertyNesedObjectPU[${this.id__()}]: syncPeerHasChanged(): Source '${eventSource.info()}' has changed'.`) + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}]: syncPeerHasChanged(): Source '${eventSource.info()}' has changed'.`) const newValue = this.source_.getUnmonitored(); if (typeof newValue == "object") { stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: hasChanged: newValue '${JSON.stringify(newValue)}'.`); - this.setWrappedValue(newValue); - this.notifyPropertyHasChangedPU(); + if (this.resetLocalValue(newValue, /* needCopyObject */ true)) { + this.notifyPropertyHasChangedPU(); + } } } else { - stateMgmtConsole.warn(`SynchedPropertyNesedObjectPU[${this.id__()}]: syncPeerHasChanged Unexpected situation. Ignorning event.`) + stateMgmtConsole.warn(`SynchedPropertyObjectOneWayPU[${this.id__()}]: syncPeerHasChanged Unexpected situation. Ignoring event.`) } } @@ -101,67 +147,220 @@ class SynchedPropertyObjectOneWayPU * @param souceObject * @param changedPropertyName */ - public objectPropertyHasChangedPU(souceObject: ObservedObject, changedPropertyName : string) { - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: \ - objectPropertyHasChangedPU: contained ObservedObject property '${changedPropertyName}' has changed.`) - this.notifyPropertyHasChangedPU(); + public objectPropertyHasChangedPU(sourceObject: ObservedObject, changedPropertyName: string) { + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: objectPropertyHasChangedPU '${changedPropertyName}' has changed.`); + this.notifyPropertyHasChangedPU(); } - public objectPropertyHasBeenReadPU(souceObject: ObservedObject, changedPropertyName : string) { + public objectPropertyHasBeenReadPU(sourceObject: ObservedObject, changedPropertyName : string) { stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: \ objectPropertyHasBeenReadPU: contained ObservedObject property '${changedPropertyName}' has been read.`); this.notifyPropertyHasBeenReadPU(); } public getUnmonitored(): C { - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored returns '${JSON.stringify(this.wrappedValue_)}'.`); + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); // unmonitored get access , no call to notifyPropertyRead ! - return this.wrappedValue_; + return this.localCopyObservedObject_; } - // get 'read through` from the ObservedObject public get(): C { - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: get returning ${JSON.stringify(this.wrappedValue_)}.`) + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: get.`) this.notifyPropertyHasBeenReadPU() - return this.wrappedValue_; + return this.localCopyObservedObject_; } // assignment to local variable in the form of this.aProp = // set 'writes through` to the ObservedObject public set(newValue: C): void { - if (this.wrappedValue_ == newValue) { - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}IP, '${this.info() || "unknown"}']: set with unchanged value '${JSON.stringify(newValue)}'- ignoring.`); + if (this.localCopyObservedObject_ == newValue) { + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}IP, '${this.info() || "unknown"}']: set with unchanged value - nothing to do.`); return; } - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: set to newValue: '${JSON.stringify(newValue)}'.`); + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: set to newV value.`); if (!ObservedObject.IsObservedObject(newValue)) { stateMgmtConsole.warn(`@Prop ${this.info()} Set: Provided new object's class - lacks @Observed class decorator. Object property changes will not be observed.`); + lacks '@Observed' class decorator. Object property changes will not be observed.`); } - this.setWrappedValue(newValue); - this.notifyPropertyHasChangedPU(); + if (this.resetLocalValue(newValue, /* needCopyObject */ false)) { + this.notifyPropertyHasChangedPU(); + } } + // called when updated from parent public reset(sourceChangedValue: C): void { - stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: reset from '${JSON.stringify(this.wrappedValue_)}' to '${JSON.stringify(sourceChangedValue)}'.`); - // if set causes an actual change, then, ObservedPropertyObject source_ will call syncPeerHasChanged - this.source_.set(sourceChangedValue); + stateMgmtConsole.debug(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: reset.`); + if (this.source_ !== undefined) { + // if this.source_.set causes an actual change, then, ObservedPropertyObject source_ will call syncPeerHasChanged method + this.source_.set(sourceChangedValue); + } else { + stateMgmtConsole.error(`SynchedPropertyObjectOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: reset @Prop --- No source_. Internal error!`); + } } + + /* + unsubscribe from previous wrappped ObjectObject + take a shallow or (TODO) deep copy + copied Object might already be an ObservedObject (e.g. becuse of @Observed decroator) or might be raw + Therefore, conditionally wrap the object, then subscribe + return value true only if localCopyObservedObject_ has been changed + */ + private resetLocalValue(newObservedObjectValue: C, needCopyObject : boolean): boolean { + // note: We can not test for newObservedObjectValue == this.localCopyObservedObject_ + // here because the object might still be the same, but some property of it has changed + + if (typeof newObservedObjectValue !== "object") { + // if not undefined or null, then the provided newObservedObjectValue must be an Object + stateMgmtConsole.error(`SynchedPropertyOneWayObjectPU[${this.id__()}]: setLocalValue new value must be an Object.`) + } + + // unsubscribe from old wrappedValue ObservedOject + ObservedObject.removeOwningProperty(this.localCopyObservedObject_, this); + + // shallow/deep copy value + // needed whenever newObservedObjectValue comes from source + // not needed on a local set (aka when called from set() method) + let copy = needCopyObject ? this.copyObject(newObservedObjectValue, this.info_) : newObservedObjectValue; - private setWrappedValue(value: C): void { + if (ObservedObject.IsObservedObject(copy)) { + // case: new ObservedObject + this.localCopyObservedObject_ = copy; + ObservedObject.addOwningProperty(this.localCopyObservedObject_, this); + } else { + // wrap newObservedObjectValue raw object as ObservedObject and subscribe to it + stateMgmtConsole.warn(`@Prop ${this.info()} Provided source object's class \ + lacks @Observed class decorator. Object property changes will not be observed.`); + this.localCopyObservedObject_ = ObservedObject.createNew(copy, this); + } + return true; + } + + private copyObject(value: C, propName: string): C { + // ViewStackProcessor.getApiVersion function is not present in API9 + // therefore shallowCopyObject will always be used in API version 9 and before + // but the code in this file is the same regardless of API version + return ((typeof ViewStackProcessor["getApiVersion"] == "function") && + (ViewStackProcessor["getApiVersion"]() >= 10)) + ? this.deepCopyObject(value, propName) + : this.shallowCopyObject(value, propName); + } + + // API 9 code path + private shallowCopyObject(value: C, propName: string) : C { let rawValue = ObservedObject.GetRawObject(value); let copy: C; - - // FIXME: Proper object deep copy missing here! - if (rawValue instanceof Array) { - copy = ObservedObject.createNew([ ...rawValue ], this) as unknown as C; - } else { + + if (rawValue == undefined || rawValue == null) { + copy = rawValue; + } else if (typeof rawValue != "object") { + // TODO would it be better to crash the app here? + stateMgmtConsole.error(`@Prop ${this.info()} shallowCopyObject: request to copy non-object but defined value of type '${typeof rawValue}'. Internal error! Setting copy=original value.`); + copy = rawValue; + } else if (rawValue instanceof Array) { + // case Array inside ObservedObject + copy = ObservedObject.createNew([...rawValue] as unknown as C, this); + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + } else if (rawValue instanceof Date) { + // case Date inside ObservedObject + let d = new Date(); + d.setTime((rawValue as Date).getTime()); + // subscribe, also Date gets wrapped / proxied by ObservedObject + copy = ObservedObject.createNew(d as unknown as C, this); + } else if (rawValue instanceof SubscribaleAbstract) { + // case SubscriabableAbstract, no wrapping inside ObservedObject + copy = { ...rawValue }; + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + if (copy instanceof SubscribaleAbstract) { + // subscribe + (copy as unknown as SubscribaleAbstract).addOwningProperty(this); + } + } else if (typeof rawValue == "object") { + // case Object that is not Array, not Date, not SubscribaleAbstract copy = ObservedObject.createNew({ ...rawValue }, this); + Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); + } else { + // TODO in PR "F": change to exception throwing: + stateMgmtConsole.error(`@Prop ${this.info()} shallow failed. Attempt to copy unsupported value type '${typeof rawValue}' .`); + copy = rawValue; + } + + return copy; + } + + // API 10 code path + private deepCopyObject(obj: C, variable?: string): C { + let copy = SynchedPropertyObjectOneWayPU.deepCopyObjectInternal(obj, variable); + + // this subscribe to the top level object/array of the copy + // same as shallowCopy does + if ((obj instanceof SubscribaleAbstract) && + (copy instanceof SubscribaleAbstract)) { + (copy as unknown as SubscribaleAbstract).addOwningProperty(this); + } else if (ObservedObject.IsObservedObject(obj) && ObservedObject.IsObservedObject(copy)) { + ObservedObject.addOwningProperty(copy, this); + } + + return copy;; + } + + + // do not use this function from outside unless it is for testing purposes. + public static deepCopyObjectInternal(obj: C, variable?: string): C { + if (!obj || typeof obj !== 'object') { + return obj; + } + + let stack = new Array<{ name: string }>(); + let copiedObjects = new Map(); + + return getDeepCopyOfObjectRecursive(obj); + + function getDeepCopyOfObjectRecursive(obj: any): any { + if (!obj || typeof obj !== 'object') { + return obj; + } + + const alreadyCopiedObject = copiedObjects.get(obj); + if (alreadyCopiedObject) { + let msg = `@Prop deepCopyObject: Found reference to already copied object: Path ${variable ? variable : 'unknown variable'}`; + stack.forEach(stackItem => msg += ` - ${stackItem.name}`) + stateMgmtConsole.debug(msg); + return alreadyCopiedObject; + } + + let copy; + if (obj instanceof Set) { + copy = new Set(); + for (const setKey of obj.keys()) { + stack.push({ name: setKey }); + copiedObjects.set(obj, copy); + copy.add(getDeepCopyOfObjectRecursive(setKey)); + stack.pop(); + } + } else if (obj instanceof Map) { + copy = new Map(); + for (const mapKey of obj.keys()) { + stack.push({ name: mapKey }); + copiedObjects.set(obj, copy); + copy.set(mapKey, getDeepCopyOfObjectRecursive(obj.get(mapKey))); + stack.pop(); + } + } else if (obj instanceof Date) { + copy = new Date() + copy.setTime(obj.getTime()); + } else if (obj instanceof Object) { + copy = Array.isArray(obj) ? [] : {}; + Object.setPrototypeOf(copy, Object.getPrototypeOf(obj)); + for (const objKey of Object.keys(obj)) { + stack.push({ name: objKey }); + copiedObjects.set(obj, copy); + Reflect.set(copy, objKey, getDeepCopyOfObjectRecursive(obj[objKey])); + stack.pop(); + } + } + return ObservedObject.IsObservedObject(obj) ? ObservedObject.createNew(copy, null) : copy; } - Object.setPrototypeOf(copy, Object.getPrototypeOf(rawValue)); - ObservedObject.addOwningProperty(this.wrappedValue_, this); - this.wrappedValue_ = copy; } } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_two_way.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_two_way.ts index e2555a8efb8..e1d9568671e 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_two_way.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_object_two_way.ts @@ -24,21 +24,23 @@ class SynchedPropertyObjectTwoWayPU implements PeerChangeEventReceiverPU, ObservedObjectEventsPUReceiver { - private linkedParentProperty_: ObservedPropertyObjectAbstract; + private source_: ObservedPropertyObjectAbstract; private changeNotificationIsOngoing_: boolean = false; - constructor(linkSource: ObservedPropertyObjectAbstract, + constructor(source: ObservedPropertyObjectAbstract, owningChildView: IPropertySubscriber, thisPropertyName: PropertyInfo) { super(owningChildView, thisPropertyName); - this.linkedParentProperty_ = linkSource; - if (this.linkedParentProperty_) { + this.source_ = source; + if (this.source_) { // register to the parent property - this.linkedParentProperty_.subscribeMe(this); - } + this.source_.subscribeMe(this); - // register to the ObservedObject - ObservedObject.addOwningProperty(this.linkedParentProperty_.get(), this); + // register to the ObservedObject + ObservedObject.addOwningProperty(this.source_.get(), this); + } else { + stateMgmtConsole.error(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @Link/@Consume source must not be undefined. Application error!`); + } } /* @@ -47,21 +49,28 @@ class SynchedPropertyObjectTwoWayPU */ aboutToBeDeleted() { // unregister from parent of this link - if (this.linkedParentProperty_) { - this.linkedParentProperty_.unlinkSuscriber(this.id__()); + if (this.source_) { + this.source_.unlinkSuscriber(this.id__()); // unregister from the ObservedObject - ObservedObject.removeOwningProperty(this.linkedParentProperty_.getUnmonitored(), this); + ObservedObject.removeOwningProperty(this.source_.getUnmonitored(), this); } super.aboutToBeDeleted(); } + private setObject(newValue: C): void { - if (!this.linkedParentProperty_) { - stateMgmtConsole.warn(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: setObject, no linked parent property.`); + if (!this.source_) { + stateMgmtConsole.warn(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: setObject (assign a new value), @Link/@Consume: no linked parent property. Likely a consequence of earlier application error.`); return; } - this.linkedParentProperty_.set(newValue) + + let oldValueObject = this.getUnmonitored(); + if (oldValueObject != undefined && oldValueObject != null) { + ObservedObject.removeOwningProperty(oldValueObject, this); + } + this.source_.set(newValue) + ObservedObject.addOwningProperty(this.getUnmonitored(), this); } /** @@ -95,9 +104,9 @@ class SynchedPropertyObjectTwoWayPU } public getUnmonitored(): C { - stateMgmtConsole.debug(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored returns '${(this.linkedParentProperty_ ? JSON.stringify(this.linkedParentProperty_.getUnmonitored()) : "undefined")}' .`); + stateMgmtConsole.debug(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); // unmonitored get access , no call to otifyPropertyRead ! - return (this.linkedParentProperty_ ? this.linkedParentProperty_.getUnmonitored() : undefined); + return (this.source_ ? this.source_.getUnmonitored() : undefined); } // get 'read through` from the ObservedProperty @@ -110,18 +119,15 @@ class SynchedPropertyObjectTwoWayPU // set 'writes through` to the ObservedProperty public set(newValue: C): void { if (this.getUnmonitored() == newValue) { - stateMgmtConsole.debug(`SynchedPropertyObjectTwoWayPU[${this.id__()}IP, '${this.info() || "unknown"}']: set with unchanged value '${newValue}'- ignoring.`); + stateMgmtConsole.debug(`SynchedPropertyObjectTwoWayPU[${this.id__()}IP, '${this.info() || "unknown"}']: set with unchanged value - nothing to do.`); return; } stateMgmtConsole.debug(`SynchedPropertyObjectTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: set to newValue: '${newValue}'.`); - - ObservedObject.removeOwningProperty(this.getUnmonitored(), this); // avoid circular notifications @Link -> source @State -> other but also back to same @Link this.changeNotificationIsOngoing_ = true; this.setObject(newValue); - ObservedObject.addOwningProperty(this.getUnmonitored(), this); this.notifyPropertyHasChangedPU(); this.changeNotificationIsOngoing_ = false; } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_one_way.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_one_way.ts index b9ea3dfe55a..18f85cf3536 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_one_way.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_one_way.ts @@ -36,7 +36,7 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU< // code path for @(Local)StorageProp this.source_ = source as ObservedPropertyAbstract; this.sourceIsOwnObject = false; - // subscribe to receive value chnage updates from LocalStorge source property + // subscribe to receive value change updates from LocalStorage source property this.source_.subscribeMe(this); } else { // code path for @Prop @@ -66,24 +66,19 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU< super.aboutToBeDeleted(); } - public syncPeerHasChanged(eventSource: ObservedPropertyAbstractPU) { + public syncPeerHasChanged(eventSource: ObservedPropertyAbstractPU) { if (eventSource && (eventSource == this.source_)) { // defensive, should always be the case stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: \ syncPeerHasChanged peer '${eventSource.info()}'.`); - this.sourceHasChanged(eventSource); - } + this.setWrappedValue(eventSource.getUnmonitored()); + this.notifyPropertyHasChangedPU(); + } } - protected sourceHasChanged(eventSource: ObservedPropertyAbstractPU): void { - stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: sourceHasChanged: source '${eventSource.info()}' has changed value to ${eventSource.getUnmonitored()}.`) - this.wrappedValue_ = eventSource.getUnmonitored(); - this.notifyPropertyHasChangedPU(); -} - public getUnmonitored(): T { - stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored returns '${JSON.stringify(this.wrappedValue_)}' .`); - // unmonitored get access , no call to otifyPropertyRead ! + stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: getUnmonitored.`); + // unmonitored get access, no call to notifyPropertyRead ! return this.wrappedValue_; } @@ -101,14 +96,21 @@ class SynchedPropertySimpleOneWayPU extends ObservedPropertySimpleAbstractPU< } stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: set from '${this.wrappedValue_} to '${newValue}'.`); - this.wrappedValue_ = newValue; + this.setWrappedValue(newValue); this.notifyPropertyHasChangedPU(); } public reset(sourceChangedValue: T): void { stateMgmtConsole.debug(`SynchedPropertySimpleOneWayPU[${this.id__()}, '${this.info() || "unknown"}']: reset from '${this.wrappedValue_} to '${sourceChangedValue}'.`); // if set causes an actual change, then, ObservedPropertySimple source_ will call hasChanged - this.source_.set(sourceChangedValue); + if (this.source_ !== undefined) { + // if set causes an actual change, then, ObservedPropertySimple source_ will call hasChanged + this.source_.set(sourceChangedValue); + } + } + + private setWrappedValue(newValue: T): void { + this.wrappedValue_ = newValue; } } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_two_way.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_two_way.ts index 6e4d1c14cd1..bb8f86ef811 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_two_way.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_synced_property_simple_two_way.ts @@ -29,7 +29,11 @@ class SynchedPropertySimpleTwoWayPU extends ObservedPropertySimpleAbstractPU< constructor(source: ObservedPropertyAbstract, owningView: IPropertySubscriber, owningViewPropNme: PropertyInfo) { super(owningView, owningViewPropNme); this.source_ = source; - this.source_.subscribeMe(this); + if (this.source_) { + this.source_.subscribeMe(this); + } else { + stateMgmtConsole.error(`SynchedPropertySimpleTwoWayPU[${this.id__()}, '${this.info() || "unknown"}']: constructor @Link/@Consume source must not be undefined. Application error!`); + } } /* diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_view.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_view.ts index bde95f357f5..b82d81869ee 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_view.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/partial_update/pu_view.ts @@ -53,6 +53,9 @@ abstract class ViewPU extends NativeViewPartialUpdate private parent_: ViewPU = undefined; private childrenWeakrefMap_ = new Map>(); + // flag for initgial rendering or re-render on-going. + private isRenderInProgress: boolean = false; + private watchedProps: Map void> = new Map void>(); @@ -222,21 +225,30 @@ abstract class ViewPU extends NativeViewPartialUpdate } protected initialRenderView(): void { + this.isRenderInProgress = true; this.initialRender(); + this.isRenderInProgress = false; } private UpdateElement(elmtId: number): void { + if (elmtId == this.id__()) { + // do not attempt to update itself. + // a @Prop can add a dependency of the ViewPU onto itself. Ignore it. + return; + } // do not process an Element that has been marked to be deleted const updateFunc: UpdateFunc = this.updateFuncByElmtId.get(elmtId); if ((updateFunc == undefined) || (typeof updateFunc !== "function")) { stateMgmtConsole.error(`${this.constructor.name}[${this.id__()}]: update function of ElementId ${elmtId} not found, internal error!`); } else { stateMgmtConsole.debug(`${this.constructor.name}[${this.id__()}]: updateDirtyElements: update function on elmtId ${elmtId} start ...`); + this.isRenderInProgress = true; updateFunc(elmtId, /* isFirstRender */ false); // continue in native JSView // Finish the Update in JSView::JsFinishUpdateFunc // this function appends no longer used elmtIds (as recrded by VSP) to the given allRmElmtIds array this.finishUpdateFunc(elmtId); + this.isRenderInProgress = false; stateMgmtConsole.debug(`View ${this.constructor.name} elmtId ${this.id__()}: ViewPU.updateDirtyElements: update function on ElementId ${elmtId} done`); } } @@ -313,6 +325,10 @@ abstract class ViewPU extends NativeViewPartialUpdate // implements IMultiPropertiesChangeSubscriber viewPropertyHasChanged(varName: PropertyInfo, dependentElmtIds: Set): void { stateMgmtTrace.scopedTrace(() => { + if (this.isRenderInProgress) { + stateMgmtConsole.error(`@Component '${this.constructor.name}' (id: ${this.id__()}) State variable '${varName}' has changed during render! It's illegal to change @Component state while build (initial render or re-render) is on-going. Application error!`); + } + stateMgmtConsole.debug(`${this.constructor.name}: viewPropertyHasChanged property '${varName}'. View needs ${dependentElmtIds.size ? 'update' : 'no update'}.`); this.syncInstanceId(); @@ -323,8 +339,9 @@ abstract class ViewPU extends NativeViewPartialUpdate this.markNeedUpdate(); } stateMgmtConsole.debug(`${this.constructor.name}: viewPropertyHasChanged property '${varName}': elmtIds affected by value change [${Array.from(dependentElmtIds).toString()}].`) - const union: Set = new Set([...this.dirtDescendantElementIds_, ...dependentElmtIds]); - this.dirtDescendantElementIds_ = union; + for (const elmtId of dependentElmtIds) { + this.dirtDescendantElementIds_.add(elmtId); + } stateMgmtConsole.debug(`${this.constructor.name}: viewPropertyHasChanged property '${varName}': all elmtIds need update [${Array.from(this.dirtDescendantElementIds_).toString()}].`) } @@ -426,6 +443,11 @@ abstract class ViewPU extends NativeViewPartialUpdate this.UpdateElement(elmtId); this.dirtDescendantElementIds_.delete(elmtId); }); + + if (this.dirtDescendantElementIds_.size) { + stateMgmtConsole.error(`@Component '${this.constructor.name}' (id: ${this.id__()}): New UINode objects added to update queue while re-render! \ + Likely caused by @Component state change during build phase, not allowed. Application error!`); + } } while(this.dirtDescendantElementIds_.size); } diff --git a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/sdk/environment.ts b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/sdk/environment.ts index 84e56b0e39d..fb15d77fc6f 100644 --- a/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/sdk/environment.ts +++ b/frameworks/bridge/declarative_frontend/state_mgmt/src/lib/sdk/environment.ts @@ -102,6 +102,7 @@ class Environment { prop = AppStorage.SetAndProp(key, tmp); this.props_.set(key, prop); stateMgmtConsole.debug(`Environment: envProp for '${key}' done.`); + return true; } private envProps(properties: { -- Gitee