diff --git a/src/runtime/component_node.ts b/src/runtime/component_node.ts index de925700..62babc23 100644 --- a/src/runtime/component_node.ts +++ b/src/runtime/component_node.ts @@ -3,14 +3,7 @@ import { BDom, VNode } from "./blockdom"; import { Component, ComponentConstructor, Props } from "./component"; import { fibersInError, OwlError } from "./error_handling"; import { Fiber, makeChildFiber, makeRootFiber, MountFiber, MountOptions } from "./fibers"; -import { - clearReactivesForCallback, - getSubscriptions, - NonReactive, - Reactive, - reactive, - TARGET, -} from "./reactivity"; +import { clearReactivesForCallback, getSubscriptions, reactive, targets } from "./reactivity"; import { STATUS } from "./status"; import { batched, Callback } from "./utils"; @@ -52,7 +45,7 @@ const batchedRenderFunctions = new WeakMap(); * relevant changes * @see reactive */ -export function useState(state: T): Reactive | NonReactive { +export function useState(state: T): T { const node = getCurrent(); let render = batchedRenderFunctions.get(node)!; if (!render) { @@ -116,7 +109,7 @@ export class ComponentNode

implements VNode implements VNode = T; + type Collection = Set | Map | WeakMap; type CollectionRawType = "Set" | "Map" | "WeakMap"; -export type Reactive = T & { - [TARGET]: any; -}; - -export type NonReactive = T & { - [SKIP]: any; -}; - const objectToString = Object.prototype.toString; const objectHasOwnProperty = Object.prototype.hasOwnProperty; @@ -61,15 +53,16 @@ function possiblyReactive(val: any, cb: Callback) { return canBeMadeReactive(val) ? reactive(val, cb) : val; } +const skipped = new WeakSet(); /** * Mark an object or array so that it is ignored by the reactivity system * * @param value the value to mark * @returns the object itself */ -export function markRaw(value: T): NonReactive { - (value as any)[SKIP] = true; - return value as NonReactive; +export function markRaw(value: T): T { + skipped.add(value); + return value; } /** @@ -78,8 +71,8 @@ export function markRaw(value: T): NonReactive { * @param value a reactive value * @returns the underlying value */ -export function toRaw(value: Reactive): T { - return value[TARGET] || value; +export function toRaw>(value: U | T): T { + return targets.has(value) ? (targets.get(value) as T) : value; } const targetToKeysToCallbacks = new WeakMap>>(); @@ -164,8 +157,9 @@ export function getSubscriptions(callback: Callback) { }; }); } - -const reactiveCache = new WeakMap>(); +// Maps reactive objects to the underlying target +export const targets = new WeakMap, Target>(); +const reactiveCache = new WeakMap>>(); /** * Creates a reactive proxy for an object. Reading data on the reactive object * subscribes to changes to the data. Writing data on the object will cause the @@ -180,7 +174,7 @@ const reactiveCache = new WeakMap>(); * Subscriptions: * + Reading a property on an object will subscribe you to changes in the value * of that property. - * + Accessing an object keys (eg with Object.keys or with `for..in`) will + * + Accessing an object's keys (eg with Object.keys or with `for..in`) will * subscribe you to the creation/deletion of keys. Checking the presence of a * key on the object with 'in' has the same effect. * - getOwnPropertyDescriptor does not currently subscribe you to the property. @@ -193,19 +187,16 @@ const reactiveCache = new WeakMap>(); * reactive has changed * @returns a proxy that tracks changes to it */ -export function reactive( - target: T, - callback: Callback = () => {} -): Reactive | NonReactive { +export function reactive(target: T, callback: Callback = () => {}): T { if (!canBeMadeReactive(target)) { throw new OwlError(`Cannot make the given value reactive`); } - if (SKIP in target) { - return target as NonReactive; + if (skipped.has(target)) { + return target; } - const originalTarget = (target as Reactive)[TARGET]; - if (originalTarget) { - return reactive(originalTarget, callback); + if (targets.has(target)) { + // target is reactive, create a reactive on the underlying object instead + return reactive(targets.get(target) as T, callback); } if (!reactiveCache.has(target)) { reactiveCache.set(target, new WeakMap()); @@ -218,6 +209,7 @@ export function reactive( : basicProxyHandler(callback); const proxy = new Proxy(target, handler as ProxyHandler) as Reactive; reactivesForTarget.set(callback, proxy); + targets.set(proxy, target); } return reactivesForTarget.get(callback) as Reactive; } @@ -229,29 +221,29 @@ export function reactive( */ function basicProxyHandler(callback: Callback): ProxyHandler { return { - get(target: any, key: PropertyKey, proxy: Reactive) { - if (key === TARGET) { - return target; - } + get(target, key, receiver) { // non-writable non-configurable properties cannot be made reactive const desc = Object.getOwnPropertyDescriptor(target, key); if (desc && !desc.writable && !desc.configurable) { - return Reflect.get(target, key, proxy); + return Reflect.get(target, key, receiver); } observeTargetKey(target, key, callback); - return possiblyReactive(Reflect.get(target, key, proxy), callback); + return possiblyReactive(Reflect.get(target, key, receiver), callback); }, - set(target, key, value, proxy) { - const isNewKey = !objectHasOwnProperty.call(target, key); - const originalValue = Reflect.get(target, key, proxy); - const ret = Reflect.set(target, key, value, proxy); - if (isNewKey) { + set(target, key, value, receiver) { + const hadKey = objectHasOwnProperty.call(target, key); + const originalValue = Reflect.get(target, key, receiver); + const ret = Reflect.set(target, key, value, receiver); + if (!hadKey && objectHasOwnProperty.call(target, key)) { notifyReactives(target, KEYCHANGES); } // While Array length may trigger the set trap, it's not actually set by this // method but is updated behind the scenes, and the trap is not called with the // new value. We disable the "same-value-optimization" for it because of that. - if (originalValue !== value || (Array.isArray(target) && key === "length")) { + if ( + originalValue !== Reflect.get(target, key, receiver) || + (key === "length" && Array.isArray(target)) + ) { notifyReactives(target, key); } return ret; @@ -444,10 +436,8 @@ function collectionsProxyHandler( // property is read. const specialHandlers = rawTypeToFuncHandlers[targetRawType](target, callback); return Object.assign(basicProxyHandler(callback), { + // FIXME: probably broken when part of prototype chain since we ignore the receiver get(target: any, key: PropertyKey) { - if (key === TARGET) { - return target; - } if (objectHasOwnProperty.call(specialHandlers, key)) { return (specialHandlers as any)[key]; } diff --git a/tests/reactivity.test.ts b/tests/reactivity.test.ts index 751ee5a7..a02648fa 100644 --- a/tests/reactivity.test.ts +++ b/tests/reactivity.test.ts @@ -9,7 +9,7 @@ import { markRaw, toRaw, } from "../src"; -import { reactive, Reactive } from "../src/runtime/reactivity"; +import { reactive } from "../src/runtime/reactivity"; import { batched } from "../src/runtime/utils"; import { makeDeferred, @@ -1142,6 +1142,46 @@ describe("Reactivity", () => { expect(() => state.a).not.toThrow(); expect(state.a).toBe(obj.a); }); + + test("writing on object with reactive in prototype chain doesn't notify", async () => { + let n = 0; + const state = createReactive({ val: 0 }, () => n++); + const nonReactive = Object.create(state); + nonReactive.val++; + expect(n).toBe(0); + expect(toRaw(state)).toEqual({ val: 0 }); + expect(toRaw(nonReactive)).toEqual({ val: 1 }); + state.val++; + expect(n).toBe(1); + expect(toRaw(state)).toEqual({ val: 1 }); + expect(toRaw(nonReactive)).toEqual({ val: 1 }); + }); + + test("creating key on object with reactive in prototype chain doesn't notify", async () => { + let n = 0; + const parent = createReactive({}, () => n++); + const child = Object.create(parent); + Object.keys(parent); // Subscribe to key changes + child.val = 0; + expect(n).toBe(0); + }); + + test("reactive of object with reactive in prototype chain is not the object from the prototype chain", async () => { + const cb = () => {}; + const parent = createReactive({ val: 0 }, cb); + const child = createReactive(Object.create(parent), cb); + expect(child).not.toBe(parent); + }); + + test("can create reactive of object with non-reactive in prototype chain", async () => { + let n = 0; + const parent = markRaw({ val: 0 }); + const child = createReactive(Object.create(parent), () => n++); + child.val++; + expect(n).toBe(1); + expect(parent).toEqual({ val: 0 }); + expect(child).toEqual({ val: 1 }); + }); }); describe("Collections", () => { @@ -1699,12 +1739,12 @@ describe("toRaw", () => { const obj = { value: 1 }; const reactiveObj = reactive(obj); expect(reactiveObj).not.toBe(obj); - expect(toRaw(reactiveObj as Reactive)).toBe(obj); + expect(toRaw(reactiveObj)).toBe(obj); }); test("giving a non reactive to toRaw return the object itself", () => { const obj = { value: 1 }; - expect(toRaw(obj as Reactive)).toBe(obj); + expect(toRaw(obj)).toBe(obj); }); });