fix: refactor reactive membrane to use weakmap instead of targetslot (#…

…34)

* fix: refactor reactive membrane to use weakmap instead of targetslot

* fix: minor variable name changes

src/reactive-dev-formatter.ts

@@ -1,12 +1,12 @@
 import {
-    TargetSlot,
     ArrayPush,
     ArrayConcat,
     isArray,
     ObjectCreate,
     getPrototypeOf,
     getOwnPropertyNames,
     getOwnPropertySymbols,
+    unwrap
 } from './shared';
 
 // Define globalThis since it's not current defined in by typescript.
@@ -19,15 +19,11 @@ interface DevToolFormatter {
     body: (object: any, config: any) => any;
 }
 
-function getTarget(item: any) {
-    return item && item[TargetSlot];
-}
-
 function extract(objectOrArray: any): any {
     if (isArray(objectOrArray)) {
         return objectOrArray.map((item) => {
-            const original = getTarget(item);
-            if (original) {
+            const original = unwrap(item);
+            if (original !== item) {
                 return extract(original);
             }
             return item;
@@ -39,8 +35,8 @@ function extract(objectOrArray: any): any {
     return ArrayConcat.call(names, getOwnPropertySymbols(objectOrArray))
         .reduce((seed: any, key: PropertyKey) => {
             const item = objectOrArray[key];
-            const original = getTarget(item);
-            if (original) {
+            const original = unwrap(item);
+            if (original !== item) {
                 seed[key] = extract(original);
             } else {
                 seed[key] = item;
@@ -51,8 +47,9 @@ function extract(objectOrArray: any): any {
 
 const formatter: DevToolFormatter = {
     header: (plainOrProxy) => {
-        const originalTarget = plainOrProxy[TargetSlot];
-        if (!originalTarget) {
+        const originalTarget = unwrap(plainOrProxy);
+        // if originalTarget is falsy or not unwrappable, exit
+        if (!originalTarget || originalTarget === plainOrProxy) {
             return null;
         }
 

src/reactive-handler.ts

@@ -1,7 +1,6 @@
 import {
     toString,
     isUndefined,
-    TargetSlot,
     unwrap,
     ArrayConcat,
     isArray,
@@ -25,8 +24,11 @@ function wrapValue(membrane: ReactiveMembrane, value: any): any {
     return membrane.valueIsObservable(value) ? membrane.getProxy(value) : value;
 }
 
-// Unwrap property descriptors
-// We only need to unwrap if value is specified
+/**
+ * Unwrap property descriptors will set value on original descriptor
+ * We only need to unwrap if value is specified
+ * @param descriptor external descrpitor provided to define new property on original value
+ */
 function unwrapDescriptor(descriptor: PropertyDescriptor): PropertyDescriptor {
     if (hasOwnProperty.call(descriptor, 'value')) {
         descriptor.value = unwrap(descriptor.value);
@@ -63,9 +65,6 @@ export class ReactiveProxyHandler {
     }
     get(shadowTarget: ReactiveMembraneShadowTarget, key: PropertyKey): any {
         const { originalTarget, membrane } = this;
-        if (key === TargetSlot) {
-            return originalTarget;
-        }
         const value = originalTarget[key];
         const { valueObserved } = membrane;
         valueObserved(originalTarget, key);

src/reactive-membrane.ts

@@ -7,6 +7,7 @@ import {
     getPrototypeOf,
     isFunction,
     hasOwnProperty,
+    registerProxy,
 } from './shared';
 import { ReactiveProxyHandler } from './reactive-handler';
 import { ReadOnlyHandler } from './read-only-handler';
@@ -118,9 +119,10 @@ export class ReactiveMembrane {
     }
 
     getProxy(value: any) {
-        const distorted = this.valueDistortion(value);
+        const unwrappedValue = unwrap(value);
+        const distorted = this.valueDistortion(unwrappedValue);
         if (this.valueIsObservable(distorted)) {
-            const o = this.getReactiveState(distorted);
+            const o = this.getReactiveState(unwrappedValue, distorted);
             // when trying to extract the writable version of a readonly
             // we return the readonly.
             return o.readOnly === value ? value : o.reactive;
@@ -129,9 +131,10 @@ export class ReactiveMembrane {
     }
 
     getReadOnlyProxy(value: any) {
+        value = unwrap(value);
         const distorted = this.valueDistortion(value);
         if (this.valueIsObservable(distorted)) {
-            return this.getReactiveState(distorted).readOnly;
+            return this.getReactiveState(value, distorted).readOnly;
         }
         return distorted;
     }
@@ -140,34 +143,35 @@ export class ReactiveMembrane {
         return unwrap(p);
     }
 
-    private getReactiveState(value: any): ReactiveState {
+    private getReactiveState(value: any, distortedValue: any): ReactiveState {
         const {
             objectGraph,
         } = this;
-        value = unwrap(value);
-        let reactiveState = objectGraph.get(value);
+        let reactiveState = objectGraph.get(distortedValue);
         if (reactiveState) {
             return reactiveState;
         }
         const membrane = this;
         reactiveState = {
             get reactive() {
-                const reactiveHandler = new ReactiveProxyHandler(membrane, value);
+                const reactiveHandler = new ReactiveProxyHandler(membrane, distortedValue);
                 // caching the reactive proxy after the first time it is accessed
-                const proxy = new Proxy(createShadowTarget(value), reactiveHandler);
+                const proxy = new Proxy(createShadowTarget(distortedValue), reactiveHandler);
+                registerProxy(proxy, value);
                 ObjectDefineProperty(this, 'reactive', { value: proxy });
                 return proxy;
             },
             get readOnly() {
-                const readOnlyHandler = new ReadOnlyHandler(membrane, value);
+                const readOnlyHandler = new ReadOnlyHandler(membrane, distortedValue);
                 // caching the readOnly proxy after the first time it is accessed
-                const proxy = new Proxy(createShadowTarget(value), readOnlyHandler);
+                const proxy = new Proxy(createShadowTarget(distortedValue), readOnlyHandler);
+                registerProxy(proxy, value);
                 ObjectDefineProperty(this, 'readOnly', { value: proxy });
                 return proxy;
             }
         } as ReactiveState;
 
-        objectGraph.set(value, reactiveState);
+        objectGraph.set(distortedValue, reactiveState);
         return reactiveState;
     }
 

src/read-only-handler.ts

@@ -1,6 +1,5 @@
 import {
     isUndefined,
-    TargetSlot,
     ArrayConcat,
     ObjectDefineProperty,
     getOwnPropertyDescriptor,
@@ -29,9 +28,6 @@ export class ReadOnlyHandler {
     }
     get(shadowTarget: ReactiveMembraneShadowTarget, key: PropertyKey): any {
         const { membrane, originalTarget } = this;
-        if (key === TargetSlot) {
-            return originalTarget;
-        }
         const value = originalTarget[key];
         const { valueObserved } = membrane;
         valueObserved(originalTarget, key);

src/shared.ts

@@ -55,17 +55,14 @@ export function isFunction(obj: any): obj is Function {
     return typeof obj === 'function';
 }
 
-export const TargetSlot = Symbol();
-
-// TODO: we are using a funky and leaky abstraction here to try to identify if
-// the proxy is a compat proxy, and define the unwrap method accordingly.
-// @ts-ignore
-const { getKey } = Proxy;
-
-export const unwrap = getKey ?
-    (replicaOrAny: any): any => (replicaOrAny && getKey(replicaOrAny, TargetSlot)) || replicaOrAny
-    : (replicaOrAny: any): any => (replicaOrAny && replicaOrAny[TargetSlot]) || replicaOrAny;
-
 export function isObject(obj: any): obj is object {
     return typeof obj === 'object';
 }
+
+const proxyToValueMap: WeakMap<object, any> = new WeakMap();
+
+export function registerProxy(proxy: object, value: any) {
+    proxyToValueMap.set(proxy, value);
+}
+
+export const unwrap = (replicaOrAny: any): any => proxyToValueMap.get(replicaOrAny) || replicaOrAny;