From 841dc260bef77bdee2d3b3fed37cba308706c24f Mon Sep 17 00:00:00 2001 From: Pavel Zhukov Date: Fri, 27 Sep 2024 13:52:38 +0300 Subject: [PATCH] fix: swich to using useDeferredValue by default. Add { defer: false } support to useSub() and to observer() (effective for any useSub inside) options to not use it. --- packages/teamplay/index.js | 7 ++++- packages/teamplay/react/helpers.js | 6 ++++ packages/teamplay/react/useSub.js | 21 +++++++++---- packages/teamplay/react/wrapIntoSuspense.js | 2 ++ packages/teamplay/test_client/react.js | 34 ++++++++++----------- 5 files changed, 46 insertions(+), 24 deletions(-) diff --git a/packages/teamplay/index.js b/packages/teamplay/index.js index c1e0711..c3055d0 100644 --- a/packages/teamplay/index.js +++ b/packages/teamplay/index.js @@ -14,7 +14,12 @@ export { GLOBAL_ROOT_ID } from './orm/Root.js' export const $ = _getRootSignal({ rootId: GLOBAL_ROOT_ID, rootFunction: universal$ }) export default $ export { default as sub } from './orm/sub.js' -export { default as useSub, useAsyncSub, setUseDeferredValue as __setUseDeferredValue } from './react/useSub.js' +export { + default as useSub, + useAsyncSub, + setUseDeferredValue as __setUseDeferredValue, + setDefaultDefer as __setDefaultDefer +} from './react/useSub.js' export { default as observer } from './react/observer.js' export { connection, setConnection, getConnection, fetchOnly, setFetchOnly, publicOnly, setPublicOnly } from './orm/connection.js' export { useId, useNow, useScheduleUpdate, useTriggerUpdate } from './react/helpers.js' diff --git a/packages/teamplay/react/helpers.js b/packages/teamplay/react/helpers.js index 8476096..c5ae017 100644 --- a/packages/teamplay/react/helpers.js +++ b/packages/teamplay/react/helpers.js @@ -46,6 +46,12 @@ export function useScheduleUpdate () { return context.scheduleUpdate } +export function useDefer () { + const context = useContext(ComponentMetaContext) + if (!context) throw Error(ERRORS.useScheduleUpdate) + return context.defer +} + export function useCache (key) { const context = useContext(ComponentMetaContext) if (!context) throw Error(ERRORS.useCache) diff --git a/packages/teamplay/react/useSub.js b/packages/teamplay/react/useSub.js index 060a170..44514b5 100644 --- a/packages/teamplay/react/useSub.js +++ b/packages/teamplay/react/useSub.js @@ -1,13 +1,15 @@ import { useRef, useDeferredValue } from 'react' import sub from '../orm/sub.js' -import { useScheduleUpdate, useCache } from './helpers.js' +import { useScheduleUpdate, useCache, useDefer } from './helpers.js' import executionContextTracker from './executionContextTracker.js' let TEST_THROTTLING = false // experimental feature to leverage useDeferredValue() to handle re-subscriptions. // Currently it does lead to issues with extra rerenders and requires further investigation -let USE_DEFERRED_VALUE = false +let USE_DEFERRED_VALUE = true +// by default we want to defer stuff if possible instead of throwing promises +let DEFAULT_DEFER = true export function useAsyncSub (signal, params, options) { return useSub(signal, params, { ...options, async: true }) @@ -22,12 +24,16 @@ export default function useSub (signal, params, options) { } // version of sub() which works as a react hook and throws promise for Suspense -export function useSubDeferred (signal, params, { async = false } = {}) { +export function useSubDeferred (signal, params, { async = false, defer } = {}) { const $signalRef = useRef() // eslint-disable-line react-hooks/rules-of-hooks const scheduleUpdate = useScheduleUpdate() - signal = useDeferredValue(signal) - params = useDeferredValue(params ? JSON.stringify(params) : undefined) - params = params != null ? JSON.parse(params) : undefined + const observerDefer = useDefer() + defer ??= observerDefer ?? DEFAULT_DEFER + if (defer) { + signal = useDeferredValue(signal) // eslint-disable-line react-hooks/rules-of-hooks + params = useDeferredValue(params ? JSON.stringify(params) : undefined) // eslint-disable-line react-hooks/rules-of-hooks + params = params != null ? JSON.parse(params) : undefined + } const promiseOrSignal = params != null ? sub(signal, params) : sub(signal) // 1. if it's a promise, throw it so that Suspense can catch it and wait for subscription to finish if (promiseOrSignal.then) { @@ -97,6 +103,9 @@ export function resetTestThrottling () { export function setUseDeferredValue (value) { USE_DEFERRED_VALUE = value } +export function setDefaultDefer (value) { + DEFAULT_DEFER = value +} // throttle to simulate slow network function maybeThrottle (promise) { diff --git a/packages/teamplay/react/wrapIntoSuspense.js b/packages/teamplay/react/wrapIntoSuspense.js index 93db9a0..6a71d07 100644 --- a/packages/teamplay/react/wrapIntoSuspense.js +++ b/packages/teamplay/react/wrapIntoSuspense.js @@ -17,6 +17,7 @@ function destroyAdm (adm) { export default function wrapIntoSuspense ({ Component, forwardRef, + defer, suspenseProps = DEFAULT_SUSPENSE_PROPS } = {}) { if (!suspenseProps?.fallback) throw Error(ERRORS.noFallback) @@ -62,6 +63,7 @@ export default function wrapIntoSuspense ({ componentMetaRef.current = { componentId, createdAt: Date.now(), + defer, triggerUpdate: () => adm.onStoreChange?.(), scheduleUpdate: promise => adm.scheduleUpdate?.(promise), cache: { diff --git a/packages/teamplay/test_client/react.js b/packages/teamplay/test_client/react.js index d902e30..70d601e 100644 --- a/packages/teamplay/test_client/react.js +++ b/packages/teamplay/test_client/react.js @@ -380,27 +380,27 @@ describe('useSub() for subscribing to queries', () => { expect(container.textContent).toBe('John,Jane') fireEvent.click(container.querySelector('#active')) - expect(renders).toBe(3) + expect(renders).toBe(4) expect(container.textContent).toBe('John,Jane') await wait() - expect(renders).toBe(3) + expect(renders).toBe(4) expect(container.textContent).toBe('John,Jane') await throttledWait() - expect(renders).toBe(4) + expect(renders).toBe(5) expect(container.textContent).toBe('John') await wait() - expect(renders).toBe(4) + expect(renders).toBe(5) fireEvent.click(container.querySelector('#inactive')) - expect(renders).toBe(5) + expect(renders).toBe(7) expect(container.textContent).toBe('John') await throttledWait() - expect(renders).toBe(6) + expect(renders).toBe(8) expect(container.textContent).toBe('Jane') await throttledWait() - expect(renders).toBe(6) + expect(renders).toBe(8) resetTestThrottling() }) }) @@ -438,27 +438,27 @@ describe('useAsyncSub()', () => { expect(container.textContent).toBe('John,Jane') fireEvent.click(container.querySelector('#active')) - expect(renders).toBe(3) - expect(container.textContent).toBe('John,Jane') + expect(renders).toBe(4) + expect(container.textContent).toBe('Waiting for users to load...') await wait() - expect(renders).toBe(3) - expect(container.textContent).toBe('John,Jane') - await throttledWait() expect(renders).toBe(4) + expect(container.textContent).toBe('Waiting for users to load...') + await throttledWait() + expect(renders).toBe(5) expect(container.textContent).toBe('John') await wait() - expect(renders).toBe(4) + expect(renders).toBe(5) fireEvent.click(container.querySelector('#inactive')) - expect(renders).toBe(5) - expect(container.textContent).toBe('John') + expect(renders).toBe(7) + expect(container.textContent).toBe('Waiting for users to load...') await throttledWait() - expect(renders).toBe(6) + expect(renders).toBe(8) expect(container.textContent).toBe('Jane') await throttledWait() - expect(renders).toBe(6) + expect(renders).toBe(8) resetTestThrottling() }) })