Guarantee immediate scheduling of async hook effects (#10252)

* Throw error when useComponent is given UndefinedEntity

This is always a mistake, as it causes a reactor to suspend forever.

* Gaurantee immediate scheduling of async hook effects

* license data

* Update MeshComponent.ts

* Update ComponentFunctions.ts

* Update resourceLoaderHooks.test.tsx

* Update QueryFunctions.tsx

* simplify useQuery

* Update QueryFunctions.test.tsx

* Update QueryFunctions.tsx

* Move useImmediateEffect into hyperflux

* Update QueryFunctions.tsx

* Cleanup

packages/ecs/src/QueryFunctions.test.tsx

@@ -0,0 +1,116 @@
+/*
+CPAL-1.0 License
+
+The contents of this file are subject to the Common Public Attribution License
+Version 1.0. (the "License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+https://github.com/EtherealEngine/etherealengine/blob/dev/LICENSE.
+The License is based on the Mozilla Public License Version 1.1, but Sections 14
+and 15 have been added to cover use of software over a computer network and 
+provide for limited attribution for the Original Developer. In addition, 
+Exhibit A has been modified to be consistent with Exhibit B.
+
+Software distributed under the License is distributed on an "AS IS" basis,
+WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for the
+specific language governing rights and limitations under the License.
+
+The Original Code is Ethereal Engine.
+
+The Original Developer is the Initial Developer. The Initial Developer of the
+Original Code is the Ethereal Engine team.
+
+All portions of the code written by the Ethereal Engine team are Copyright © 2021-2023 
+Ethereal Engine. All Rights Reserved.
+*/
+
+import { renderHook } from '@testing-library/react'
+import assert from 'assert'
+import { ComponentMap, defineComponent, hasComponent, removeComponent, setComponent } from './ComponentFunctions'
+import { destroyEngine, startEngine } from './Engine'
+import { createEntity } from './EntityFunctions'
+import { defineQuery, useQuery } from './QueryFunctions'
+
+const ComponentA = defineComponent({ name: 'ComponentA' })
+const ComponentB = defineComponent({ name: 'ComponentB' })
+
+describe('QueryFunctions', () => {
+  beforeEach(() => {
+    startEngine()
+  })
+
+  afterEach(() => {
+    ComponentMap.clear()
+    return destroyEngine()
+  })
+
+  describe('defineQuery', () => {
+    it('should define a query with the given components', () => {
+      const query = defineQuery([ComponentA, ComponentB])
+      assert.ok(query)
+      let entities = query()
+      assert.ok(entities)
+      assert.strictEqual(entities.length, 0) // No entities yet
+
+      const e1 = createEntity()
+      const e2 = createEntity()
+      setComponent(e1, ComponentA)
+      setComponent(e1, ComponentB)
+      setComponent(e2, ComponentA)
+      setComponent(e2, ComponentB)
+      setComponent(createEntity(), ComponentA)
+      setComponent(createEntity(), ComponentB)
+
+      entities = query()
+      assert.strictEqual(entities.length, 2)
+      assert.strictEqual(entities[0], e1)
+      assert.strictEqual(entities[1], e2)
+      assert.ok(hasComponent(entities[0], ComponentA))
+      assert.ok(hasComponent(entities[0], ComponentB))
+    })
+  })
+
+  describe('useQuery', () => {
+    it('should return entities that match the query', () => {
+      const e1 = createEntity()
+      const e2 = createEntity()
+      setComponent(e1, ComponentA)
+      setComponent(e1, ComponentB)
+      setComponent(e2, ComponentA)
+      setComponent(e2, ComponentB)
+      const { result } = renderHook(() => useQuery([ComponentA, ComponentB])) // return correct results the first time
+      const entities = result.current
+      assert.strictEqual(entities.length, 2)
+      assert.strictEqual(entities[0], e1)
+      assert.strictEqual(entities[1], e2)
+      assert.ok(hasComponent(entities[0], ComponentA))
+      assert.ok(hasComponent(entities[0], ComponentB))
+      assert.ok(hasComponent(entities[1], ComponentA))
+      assert.ok(hasComponent(entities[1], ComponentB))
+    })
+
+    it('should update the entities when components change', () => {
+      const e1 = createEntity()
+      const e2 = createEntity()
+      setComponent(e1, ComponentA)
+      setComponent(e1, ComponentB)
+      setComponent(e2, ComponentA)
+      setComponent(e2, ComponentB)
+      const { result, rerender } = renderHook(() => useQuery([ComponentA, ComponentB]))
+      let entities = result.current
+      assert.strictEqual(entities.length, 2)
+      assert.strictEqual(entities[0], e1)
+      assert.strictEqual(entities[1], e2)
+      assert.ok(hasComponent(entities[0], ComponentA))
+      assert.ok(hasComponent(entities[0], ComponentB))
+      assert.ok(hasComponent(entities[1], ComponentA))
+      assert.ok(hasComponent(entities[1], ComponentB))
+      removeComponent(e1, ComponentB)
+      rerender()
+      entities = result.current
+      assert.strictEqual(entities.length, 1)
+      assert.strictEqual(entities[0], e2)
+      assert.ok(hasComponent(entities[0], ComponentA))
+      assert.ok(hasComponent(entities[0], ComponentB))
+    })
+  })
+})

packages/ecs/src/QueryFunctions.tsx

@@ -24,10 +24,10 @@ Ethereal Engine. All Rights Reserved.
 */
 
 import * as bitECS from 'bitecs'
-import React, { ErrorInfo, FC, memo, Suspense, useEffect, useLayoutEffect, useMemo } from 'react'
+import React, { ErrorInfo, FC, memo, Suspense, useLayoutEffect, useMemo } from 'react'
 
 import { useForceUpdate } from '@etherealengine/common/src/utils/useForceUpdate'
-import { getState, HyperFlux, startReactor, useHookstate } from '@etherealengine/hyperflux'
+import { getState, HyperFlux, startReactor, useImmediateEffect } from '@etherealengine/hyperflux'
 
 import { Component, useOptionalComponent } from './ComponentFunctions'
 import { Entity } from './Entity'
@@ -69,12 +69,10 @@ export const ReactiveQuerySystem = defineSystem({
   uuid: 'ee.hyperflux.ReactiveQuerySystem',
   insert: { after: PresentationSystemGroup },
   execute: () => {
-    for (const { query, result } of getState(SystemState).reactiveQueryStates) {
+    for (const { query, forceUpdate } of getState(SystemState).reactiveQueryStates) {
       const entitiesAdded = query.enter().length
       const entitiesRemoved = query.exit().length
-      if (entitiesAdded || entitiesRemoved) {
-        result.set(query())
-      }
+      if (entitiesAdded || entitiesRemoved) forceUpdate()
     }
   }
 })
@@ -84,17 +82,19 @@ export const ReactiveQuerySystem = defineSystem({
  * - "components" argument must not change
  */
 export function useQuery(components: QueryComponents) {
-  const result = useHookstate([] as Entity[])
+  const query = defineQuery(components)
+  const eids = query()
+  removeQuery(query)
+
   const forceUpdate = useForceUpdate()
 
-  // Use an immediate (layout) effect to ensure that `queryResult`
+  // Use a layout effect to ensure that `queryResult`
   // is deleted from the `reactiveQueryStates` map immediately when the current
   // component is unmounted, before any other code attempts to set it
   // (component state can't be modified after a component is unmounted)
   useLayoutEffect(() => {
     const query = defineQuery(components)
-    result.set(query())
-    const queryState = { query, result, components }
+    const queryState = { query, forceUpdate, components }
     getState(SystemState).reactiveQueryStates.add(queryState)
     return () => {
       removeQuery(query)
@@ -103,21 +103,47 @@ export function useQuery(components: QueryComponents) {
   }, [])
 
   // create an effect that forces an update when any components in the query change
-  useEffect(() => {
-    const entities = [...result.value]
-    const root = startReactor(function useQueryReactor() {
-      for (const entity of entities) {
-        components.forEach((C) => ('isComponent' in C ? useOptionalComponent(entity, C as any)?.value : undefined))
-      }
+  // use an immediate effect to ensure that the reactor is initialized even if this component becomes suspended during this render
+  useImmediateEffect(() => {
+    function UseQueryEntityReactor({ entity }: { entity: Entity }) {
+      return (
+        <>
+          {components.map((C) => {
+            const Component = ('isComponent' in C ? C : (C as any)()[0]) as Component
+            return (
+              <UseQueryComponentReactor
+                entity={entity}
+                key={Component.name}
+                Component={Component}
+              ></UseQueryComponentReactor>
+            )
+          })}
+        </>
+      )
+    }
+
+    function UseQueryComponentReactor(props: { entity: Entity; Component: Component }) {
+      useOptionalComponent(props.entity, props.Component)
       forceUpdate()
       return null
+    }
+
+    const root = startReactor(function UseQueryReactor() {
+      return (
+        <>
+          {eids.map((entity) => (
+            <UseQueryEntityReactor key={entity} entity={entity}></UseQueryEntityReactor>
+          ))}
+        </>
+      )
     })
+
     return () => {
       root.stop()
     }
-  }, [result])
+  }, [JSON.stringify(eids)])
 
-  return result.value
+  return eids
 }
 
 export type Query = ReturnType<typeof defineQuery>

packages/ecs/src/SystemFunctions.ts

@@ -25,12 +25,12 @@ Ethereal Engine. All Rights Reserved.
 
 /** Functions to provide system level functionalities. */
 
-import { FC, useEffect } from 'react'
+import { FC } from 'react'
 import { v4 as uuidv4 } from 'uuid'
 
 import { OpaqueType } from '@etherealengine/common/src/interfaces/OpaqueType'
 import multiLogger from '@etherealengine/common/src/logger'
-import { getMutableState, getState, startReactor } from '@etherealengine/hyperflux'
+import { getMutableState, getState, startReactor, useImmediateEffect } from '@etherealengine/hyperflux'
 
 import { SystemState } from './SystemState'
 import { nowMilliseconds } from './Timer'
@@ -217,7 +217,7 @@ export function defineSystem(systemConfig: SystemArgs) {
 }
 
 export const useExecute = (execute: () => void, insert: InsertSystem) => {
-  useEffect(() => {
+  useImmediateEffect(() => {
     const handle = defineSystem({ uuid: uuidv4(), execute, insert })
     return () => {
       destroySystem(handle)

packages/ecs/src/SystemState.ts

@@ -24,9 +24,8 @@ Ethereal Engine. All Rights Reserved.
 */
 
 import { isDev } from '@etherealengine/common/src/config'
-import { defineState, ReactorRoot, State } from '@etherealengine/hyperflux'
+import { defineState, ReactorRoot } from '@etherealengine/hyperflux'
 
-import { Entity } from './Entity'
 import { Query, QueryComponents } from './QueryFunctions'
 import { SystemUUID } from './SystemFunctions'
 
@@ -36,6 +35,6 @@ export const SystemState = defineState({
     performanceProfilingEnabled: isDev,
     activeSystemReactors: new Map<SystemUUID, ReactorRoot>(),
     currentSystemUUID: '__null__' as SystemUUID,
-    reactiveQueryStates: new Set<{ query: Query; result: State<Entity[]>; components: QueryComponents }>()
+    reactiveQueryStates: new Set<{ query: Query; forceUpdate: () => void; components: QueryComponents }>()
   })
 })

packages/engine/src/assets/functions/resourceLoaderHooks.test.tsx

@@ -245,4 +245,16 @@ describe('ResourceLoaderHooks', () => {
       })
     })
   })
+
+  it('useGLTF calls loadResource synchronously', () => {
+    const resourceState = getState(ResourceState)
+    const entity = createEntity()
+    // use renderHook to render the hook
+    renderHook(() => {
+      // call the useGLTF hook
+      useGLTF(gltfURL, entity)
+    })
+    // ensure that the loadResource function is synchronously called when the hook is rendered
+    assert(resourceState.resources[gltfURL])
+  })
 })

packages/engine/src/assets/functions/resourceLoaderHooks.ts

@@ -24,12 +24,12 @@ Ethereal Engine. All Rights Reserved.
 */
 
 import { GLTF } from '@gltf-transform/core'
-import { useEffect, useLayoutEffect } from 'react'
+import { useEffect } from 'react'
 import { Texture } from 'three'
 import { v4 as uuidv4 } from 'uuid'
 
 import { Entity, UndefinedEntity } from '@etherealengine/ecs'
-import { NO_PROXY, State, useHookstate } from '@etherealengine/hyperflux'
+import { NO_PROXY, State, useHookstate, useImmediateEffect } from '@etherealengine/hyperflux'
 import { ResourceAssetType, ResourceManager, ResourceType } from '@etherealengine/spatial/src/resources/ResourceState'
 
 import { ResourcePendingComponent } from '../../gltf/ResourcePendingComponent'
@@ -59,7 +59,7 @@ function useLoader<T extends ResourceAssetType>(
     return unload
   }, [])
 
-  useLayoutEffect(() => {
+  useImmediateEffect(() => {
     const controller = new AbortController()
     if (url !== urlState.value) {
       if (urlState.value) {
@@ -145,7 +145,7 @@ function useBatchLoader<T extends ResourceAssetType>(
     return unload
   }, [])
 
-  useEffect(() => {
+  useImmediateEffect(() => {
     const completedArr = new Array(urls.length).fill(false) as boolean[]
     const controller = new AbortController()
 

packages/hyperflux/functions/useImmediateEffect.test.ts

@@ -0,0 +1,79 @@
+/*
+CPAL-1.0 License
+
+The contents of this file are subject to the Common Public Attribution License
+Version 1.0. (the "License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+https://github.com/EtherealEngine/etherealengine/blob/dev/LICENSE.
+The License is based on the Mozilla Public License Version 1.1, but Sections 14
+and 15 have been added to cover use of software over a computer network and 
+provide for limited attribution for the Original Developer. In addition, 
+Exhibit A has been modified to be consistent with Exhibit B.
+
+Software distributed under the License is distributed on an "AS IS" basis,
+WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for the
+specific language governing rights and limitations under the License.
+
+The Original Code is Ethereal Engine.
+
+The Original Developer is the Initial Developer. The Initial Developer of the
+Original Code is the Ethereal Engine team.
+
+All portions of the code written by the Ethereal Engine team are Copyright © 2021-2023 
+Ethereal Engine. All Rights Reserved.
+*/
+
+import { renderHook } from '@testing-library/react'
+import assert from 'assert'
+import { useImmediateEffect } from './useImmediateEffect'
+
+describe('useImmediateEffect', () => {
+  it('should call the effect function immediately', () => {
+    let effectCalled = false
+    const effect = () => {
+      effectCalled = true
+    }
+
+    const { rerender } = renderHook((deps: number[]) => useImmediateEffect(effect, deps), {
+      initialProps: []
+    })
+
+    rerender([])
+
+    assert(effectCalled)
+  })
+
+  it('should call the cleanup function when dependencies change', () => {
+    let cleanupCalled = false
+    const effect = () => {
+      return () => {
+        cleanupCalled = true
+      }
+    }
+
+    const { rerender } = renderHook((deps: number[]) => useImmediateEffect(effect, deps), {
+      initialProps: []
+    })
+
+    rerender([1, 2, 3])
+
+    assert(cleanupCalled)
+  })
+
+  it('should not call the cleanup function when dependencies do not change', () => {
+    let cleanupCalled = false
+    const effect = () => {
+      return () => {
+        cleanupCalled = true
+      }
+    }
+
+    const { rerender } = renderHook((deps: number[]) => useImmediateEffect(effect, deps), {
+      initialProps: [1, 2, 3]
+    })
+
+    rerender([1, 2, 3])
+
+    assert(!cleanupCalled)
+  })
+})