Merge pull request #87 from HugoDF/fix-r-on-unwrappable-obj

Fix r(unwrappableObject) #75

src/__tests__/reactive.spec.ts

@@ -14,6 +14,98 @@ describe('reactive', () => {
     expect(data.x).toBe('foo')
   })
 
+  it('allows setting HTMLElement and NodeList without making them reactive', async () => {
+    const buttonClickHandler = vi.fn()
+    document.body.innerHTML = `<button data-testid="button">123</button>`
+    document.querySelector('button')!.onclick = buttonClickHandler
+
+    const data = reactive({
+      button: document.querySelector('button'),
+      buttons: document.querySelectorAll('button'),
+    })
+
+    // maintains property access
+    expect(data.button!.dataset.testid).toEqual('button')
+    expect(data.buttons.length).toEqual(1)
+
+    // references are to the correct DOM node
+    data.button!.click()
+    expect(buttonClickHandler).toHaveBeenCalledTimes(1)
+    data.buttons[0]!.click()
+    expect(buttonClickHandler).toHaveBeenCalledTimes(2)
+
+    const buttonPropertyWatcher = vi.fn()
+    data.$on('button', buttonPropertyWatcher)
+    data.$on('buttons', buttonPropertyWatcher)
+
+    // ensure the properties are not reactive
+    data.button!.autofocus = true
+    data.buttons[0].autofocus = true
+    await nextTick()
+    expect(buttonPropertyWatcher).not.toHaveBeenCalled()
+  })
+
+  it('allows updating a reactive field to a non-reactive HTMLElement/NodeList', async () => {
+    const buttonClickHandler = vi.fn()
+    document.body.innerHTML = `<button>123</button>`
+    document.querySelector('button')!.onclick = buttonClickHandler
+
+    const data = reactive({
+      button: {},
+      buttons: [],
+    })
+    data.button = document.querySelector('button')
+    data.buttons = document.querySelectorAll('button')
+
+    // check the references are to the correct DOM node
+    data.button!.click()
+    expect(buttonClickHandler).toHaveBeenCalledTimes(1)
+    data.buttons[0]!.click()
+    expect(buttonClickHandler).toHaveBeenCalledTimes(2)
+
+    const buttonPropertyWatcher = vi.fn()
+    data.$on('button', buttonPropertyWatcher)
+    data.$on('buttons', buttonPropertyWatcher)
+
+    // ensure the properties are not reactive
+    data.button!.autofocus = true
+    data.buttons[0].autofocus = true
+    await nextTick()
+    expect(buttonPropertyWatcher).not.toHaveBeenCalled()
+  })
+
+  it('common collection types', async () => {
+    const data = reactive({
+      map: new Map(),
+      set: new Set(),
+      weakMap: new WeakMap(),
+      weakSet: new WeakSet(),
+    })
+
+    expect(data.map.size).toEqual(0)
+    expect(data.set.size).toEqual(0)
+    expect(data.weakMap.size).toEqual(undefined)
+    expect(data.weakSet.size).toEqual(undefined)
+
+    const mockWatcher = vi.fn()
+    data.$on('map', mockWatcher)
+    data.$on('set', mockWatcher)
+    data.$on('weakSet', mockWatcher)
+    data.$on('weakMap', mockWatcher)
+
+    data.map.set('a', '123')
+    data.set.add('a')
+    data.weakMap.set(data.map, '123')
+    data.weakSet.add(data.map, '123')
+
+    // 'Map', 'Set', 'WeakMap', 'WeakSet' are not supported for watch yet
+    expect(mockWatcher).toHaveBeenCalledTimes(0)
+
+    expect(data.map.size).toEqual(1)
+    expect(data.set.size).toEqual(1)
+    expect(data.weakMap.get(data.map)).toEqual('123')
+  })
+
   it('can record dependencies', async () => {
     const data = reactive({
       a: 'foo',

src/common.ts

@@ -44,6 +44,20 @@ export function isR<T = DataSource>(obj: unknown): obj is ReactiveProxy<T> {
   )
 }
 
+/**
+ * Utility that ensures we only attempt to make reactive objects that _can_ be made reactive.
+ *
+ * Examples of objects that cause issues: NodeList, HTMLElement
+ * @see {@link https://github.com/vuejs/core/blob/8998afa42755cbdb3403cd6c0fe158980da8492c/packages/reactivity/src/reactive.ts#L43-L62}
+ */
+export function canReactiveWrap(maybeObj: any): boolean {
+  return ['Object', 'Array'].includes(
+    // from https://github.com/vuejs/core/blob/8998afa42755cbdb3403cd6c0fe158980da8492c/packages/shared/src/general.ts#L64-L67
+    // extracts "Type" from "[object Type]"
+    Object.prototype.toString.call(maybeObj).slice(8, -1)
+  )
+}
+
 export function isReactiveFunction(
   fn: CallableFunction
 ): fn is ReactiveFunction {

src/reactive.ts

@@ -1,4 +1,4 @@
-import { isR, queue, isReactiveFunction } from './common'
+import { isR, queue, isReactiveFunction, canReactiveWrap } from './common'
 
 /**
  * Available types of keys for a reactive object.
@@ -120,8 +120,9 @@ export function r<T extends DataSource>(
   data: T,
   state: ReactiveProxyState = {}
 ): ReactiveProxy<T> {
-  // If this is already reactive or a non object, just return it.
-  if (isR(data) || typeof data !== 'object') return data
+  // If this is already reactive, a non object, or an object than shouldn't be made reactive just return it.
+  if (isR(data) || typeof data !== 'object' || !canReactiveWrap(data))
+    return data
   // This is the observer registry itself, with properties as keys and callbacks as watchers.
   const observers: ReactiveProxyObservers = state.o || new Map()
   // This is a reverse map of observers with callbacks as keys and properties that callback is watching as values.