From 7f192b4f607470c0d9eb4d578abf5f4c3928ec90 Mon Sep 17 00:00:00 2001 From: Rene Date: Sat, 6 Feb 2021 16:17:55 +0100 Subject: [PATCH] direction observer performance improvements, cache new feature and typing improvements --- .../src/observers/domObserver.ts | 10 +- .../src/observers/sizeObserver.ts | 52 +++++- .../src/observers/trinsicObserver.ts | 2 +- .../src/support/cache/cache.ts | 28 ++- .../observers/sizeObserver/index.browser.ts | 9 +- .../tests/jsdom/support/cache/cache.test.ts | 173 ++++++++++++++---- 6 files changed, 207 insertions(+), 67 deletions(-) diff --git a/packages/overlayscrollbars/src/observers/domObserver.ts b/packages/overlayscrollbars/src/observers/domObserver.ts index 4a801b2..03ddfd4 100644 --- a/packages/overlayscrollbars/src/observers/domObserver.ts +++ b/packages/overlayscrollbars/src/observers/domObserver.ts @@ -16,26 +16,24 @@ import { isFunction, } from 'support'; -type TruthyOrFalsy = boolean | '' | 0 | null | undefined; type StringNullUndefined = string | null | undefined; export type DOMObserverEventContentChange = | Array<[StringNullUndefined, ((elms: Node[]) => string) | StringNullUndefined] | null | undefined> | false - | '' | null | undefined; export type DOMObserverIgnoreContentChange = ( mutation: MutationRecord, - isNestedTarget: TruthyOrFalsy, + isNestedTarget: boolean, domObserverTarget: HTMLElement, domObserverOptions: DOMObserverOptions | undefined -) => TruthyOrFalsy; +) => boolean; export type DOMObserverIgnoreTargetAttrChange = ( target: Node, attributeName: string, oldAttributeValue: string | null, newAttributeValue: string | null -) => TruthyOrFalsy; +) => boolean; export interface DOMObserverOptions { _observeContent?: boolean; // do observe children and trigger content change _attributes?: string[]; // observed attributes @@ -190,7 +188,7 @@ export const createDOMObserver = ( const baseAssertion = isNestedTarget ? !ignoreTargetChange(mutationTarget, attributeName!, oldValue, attributeValue as string | null) : notOnlyAttrChanged || contentAttrChanged; - const contentFinalChanged = baseAssertion && !ignoreContentChange(mutation, isNestedTarget, target, options); + const contentFinalChanged = baseAssertion && !ignoreContentChange(mutation, !!isNestedTarget, target, options); push(totalAddedNodes, addedNodes); diff --git a/packages/overlayscrollbars/src/observers/sizeObserver.ts b/packages/overlayscrollbars/src/observers/sizeObserver.ts index 806b20c..db40c1f 100644 --- a/packages/overlayscrollbars/src/observers/sizeObserver.ts +++ b/packages/overlayscrollbars/src/observers/sizeObserver.ts @@ -14,12 +14,12 @@ import { preventDefault, stopPropagation, addClass, - isString, equalWH, push, cAF, rAF, ResizeObserverConstructor, + isArray, } from 'support'; import { CSSDirection } from 'typings'; import { getEnvironment } from 'environment'; @@ -36,28 +36,60 @@ const animationStartEventName = 'animationstart'; const scrollEventName = 'scroll'; const scrollAmount = 3333333; const getDirection = (elm: HTMLElement): CSSDirection => style(elm, 'direction') as CSSDirection; +const domRectHasDimensions = (rect?: DOMRectReadOnly) => rect && (rect.height > 0 || rect.width > 0); +interface SizeObserverEntry { + contentRect: DOMRectReadOnly; +} export type SizeObserverOptions = { _direction?: boolean; _appear?: boolean }; export const createSizeObserver = ( target: HTMLElement, onSizeChangedCallback: (directionCache?: Cache) => any, options?: SizeObserverOptions ): (() => void) => { - const { _direction: direction = false, _appear: appear = false } = options || {}; + const { _direction: observeDirectionChange = false, _appear: observeAppearChange = false } = options || {}; const rtlScrollBehavior = getEnvironment()._rtlScrollBehavior; const baseElements = createDOM(`
`); const sizeObserver = baseElements[0] as HTMLElement; const listenerElement = sizeObserver.firstChild as HTMLElement; - const onSizeChangedCallbackProxy = (directionCache?: Cache) => { - if (direction) { - const rtl = getDirection(sizeObserver) === 'rtl'; + const updateResizeObserverContentRectCache = createCache(0, { + _alwaysUpdateValues: true, + _equal: (currVal, newVal) => + !( + !currVal || // if no initial value + // if from display: none to display: block + (!domRectHasDimensions(currVal) && domRectHasDimensions(newVal)) + ), + }); + const onSizeChangedCallbackProxy = (sizeChangedContext?: Cache | SizeObserverEntry[] | Event) => { + const directionCacheValue = sizeChangedContext && (sizeChangedContext as Cache)._value; + + let skip: boolean = false; + let doDirectionScroll = true; // always true if sizeChangedContext is Event + + // if triggered from RO. + if (isArray(sizeChangedContext) && sizeChangedContext.length > 0) { + const { _previous, _value, _changed } = updateResizeObserverContentRectCache(0, sizeChangedContext.pop()!.contentRect); + skip = !_previous || !domRectHasDimensions(_value); // skip on initial RO. call or if display is none + doDirectionScroll = !skip && _changed; // direction scroll when not skipping and changing from display: none to block, false otherwise + } + // else if its triggered with DirectionCache + else if (directionCacheValue) { + doDirectionScroll = (sizeChangedContext as Cache)._changed; // direction scroll when DirectionCache changed, false toherwise + } + + if (observeDirectionChange && doDirectionScroll) { + const rtl = (directionCacheValue || getDirection(sizeObserver)) === 'rtl'; scrollLeft(sizeObserver, rtl ? (rtlScrollBehavior.n ? -scrollAmount : rtlScrollBehavior.i ? 0 : scrollAmount) : scrollAmount); scrollTop(sizeObserver, scrollAmount); } - onSizeChangedCallback(isString((directionCache || {})._value) ? directionCache : undefined); + + if (!skip) { + onSizeChangedCallback(directionCacheValue ? (sizeChangedContext as Cache) : undefined); + } }; const offListeners: (() => void)[] = []; - let appearCallback: ((...args: any) => any) | null = appear ? onSizeChangedCallbackProxy : null; + let appearCallback: ((...args: any) => any) | false = observeAppearChange ? onSizeChangedCallbackProxy : false; if (ResizeObserverConstructor) { const resizeObserverInstance = new ResizeObserverConstructor(onSizeChangedCallbackProxy); @@ -120,10 +152,10 @@ export const createSizeObserver = ( height: scrollAmount, }); reset(); - appearCallback = appear ? () => onScroll() : reset; + appearCallback = observeAppearChange ? () => onScroll() : reset; } - if (direction) { + if (observeDirectionChange) { const updateDirectionCache = createCache(() => getDirection(sizeObserver)); push( offListeners, @@ -152,7 +184,7 @@ export const createSizeObserver = ( push( offListeners, on(sizeObserver, animationStartEventName, appearCallback, { - // Fire only once for "CSS is ready" event + // Fire only once for "CSS is ready" event if ResizeObserver strategy is used _once: !!ResizeObserverConstructor, }) ); diff --git a/packages/overlayscrollbars/src/observers/trinsicObserver.ts b/packages/overlayscrollbars/src/observers/trinsicObserver.ts index ec17524..5c5a33f 100644 --- a/packages/overlayscrollbars/src/observers/trinsicObserver.ts +++ b/packages/overlayscrollbars/src/observers/trinsicObserver.ts @@ -20,7 +20,7 @@ export const createTrinsicObserver = ( const trinsicObserver = createDOM(`
`)[0] as HTMLElement; const offListeners: (() => void)[] = []; const updateHeightIntrinsicCache = createCache>( - (ioEntryOrSize) => + (ioEntryOrSize: IntersectionObserverEntry | WH) => (ioEntryOrSize! as WH).h === 0 || (ioEntryOrSize! as IntersectionObserverEntry).isIntersecting || (ioEntryOrSize! as IntersectionObserverEntry).intersectionRatio > 0, diff --git a/packages/overlayscrollbars/src/support/cache/cache.ts b/packages/overlayscrollbars/src/support/cache/cache.ts index fa754c3..7c0015f 100644 --- a/packages/overlayscrollbars/src/support/cache/cache.ts +++ b/packages/overlayscrollbars/src/support/cache/cache.ts @@ -5,26 +5,38 @@ export interface Cache { } export interface CacheOptions { + // Custom comparison function if shallow compare isn't enough. Returns true if nothing changed. _equal?: EqualCachePropFunction; + // Initial value for _value _initialValue?: T; + // If true updates always _value and _previous, otherwise they update only when changed + _alwaysUpdateValues?: boolean; } -export type CacheUpdate = (force?: boolean | 0, context?: C) => Cache; +export type CacheUpdate = undefined extends C ? (force?: boolean | 0, context?: C) => Cache : (force: boolean | 0, context: C) => Cache; -export type UpdateCachePropFunction = (context?: C, current?: T, previous?: T) => T; +export type UpdateCachePropFunction = undefined extends C + ? (context?: C, current?: T, previous?: T) => T + : C extends T + ? ((context: C, current?: T, previous?: T) => T) | 0 + : (context: C, current?: T, previous?: T) => T; export type EqualCachePropFunction = (currentVal?: T, newVal?: T) => boolean; export const createCache = (update: UpdateCachePropFunction, options?: CacheOptions): CacheUpdate => { - const { _equal, _initialValue } = options || {}; + const { _equal, _initialValue, _alwaysUpdateValues } = options || {}; let _value: T | undefined = _initialValue; let _previous: T | undefined; - return (force, context) => { + + const cacheUpdate = ((force?: boolean | 0, context?: C) => { const curr = _value; - const newVal = update(context, _value, _previous); + // @ts-ignore + // update can only not be a function if C extends T as described in "UpdateCachePropFunction" type definition + // if C extends T the cast (context as T) is perfectly valid + const newVal = update ? update(context, _value, _previous) : (context as T); const changed = force || (_equal ? !_equal(curr, newVal) : curr !== newVal); - if (changed) { + if (changed || _alwaysUpdateValues) { _value = newVal; _previous = curr; } @@ -34,5 +46,7 @@ export const createCache = (update: UpdateCachePropFunction; + + return cacheUpdate; }; diff --git a/packages/overlayscrollbars/tests/browser/observers/sizeObserver/index.browser.ts b/packages/overlayscrollbars/tests/browser/observers/sizeObserver/index.browser.ts index 9b942ca..eb4f4d8 100644 --- a/packages/overlayscrollbars/tests/browser/observers/sizeObserver/index.browser.ts +++ b/packages/overlayscrollbars/tests/browser/observers/sizeObserver/index.browser.ts @@ -3,7 +3,8 @@ import './index.scss'; import should from 'should'; import { generateClassChangeSelectCallback, iterateSelect } from '@/testing-browser/Select'; import { setTestResult, waitForOrFailTest } from '@/testing-browser/TestResult'; -import { hasDimensions, offsetSize, WH, style, ResizeObserverConstructor } from 'support'; +import { timeout } from '@/testing-browser/timeout'; +import { hasDimensions, offsetSize, WH, style } from 'support'; import { createSizeObserver } from 'observers/sizeObserver'; @@ -65,8 +66,7 @@ const iterate = async (select: HTMLSelectElement | null, afterEach?: () => any) const offsetSizeChanged = currOffsetSize.w !== newOffsetSize.w || currOffsetSize.h !== newOffsetSize.h; const contentSizeChanged = currContentSize.w !== newContentSize.w || currContentSize.h !== newContentSize.h; const dirChanged = currDir !== newDir; - // ResizeObserver Polyfill doesn't react on display: none, so make an exception there - const dimensions = ResizeObserverConstructor ? true : hasDimensions(targetElm as HTMLElement); + const dimensions = hasDimensions(targetElm as HTMLElement); const observerElm = targetElm?.firstElementChild as HTMLElement; // no overflow if not needed @@ -87,6 +87,9 @@ const iterate = async (select: HTMLSelectElement | null, afterEach?: () => any) } }); } + if (!dimensions) { + await timeout(100); + } }, afterEach, }); diff --git a/packages/overlayscrollbars/tests/jsdom/support/cache/cache.test.ts b/packages/overlayscrollbars/tests/jsdom/support/cache/cache.test.ts index 35bd26e..17abad6 100644 --- a/packages/overlayscrollbars/tests/jsdom/support/cache/cache.test.ts +++ b/packages/overlayscrollbars/tests/jsdom/support/cache/cache.test.ts @@ -30,50 +30,88 @@ describe('cache', () => { expect(_changed).toBe(true); }); - test('creates and updates cache with context', () => { - interface ContextObj { - test: string; - even: number; - } - const updateFn = jest.fn(); - const updater = (context?: ContextObj, current?: boolean, previous?: boolean) => { - updateFn(context, current, previous); - return context!.test === 'test' || context!.even % 2 === 0; - }; - const update = createCache(updater); - const firstCtx = { test: 'test', even: 2 }; + describe('context', () => { + test('creates and updates cache with context', () => { + interface ContextObj { + test: string; + even: number; + } + const updateFn = jest.fn(); + const updater = (context?: ContextObj, current?: boolean, previous?: boolean) => { + updateFn(context, current, previous); + return context!.test === 'test' || context!.even % 2 === 0; + }; + const update = createCache(updater); + const firstCtx = { test: 'test', even: 2 }; - let { _value, _previous, _changed } = update(0, firstCtx); - expect(updateFn).toHaveBeenLastCalledWith(firstCtx, undefined, undefined); - expect(_value).toBe(true); - expect(_previous).toBe(undefined); - expect(_changed).toBe(true); + let { _value, _previous, _changed } = update(0, firstCtx); + expect(updateFn).toHaveBeenLastCalledWith(firstCtx, undefined, undefined); + expect(_value).toBe(true); + expect(_previous).toBe(undefined); + expect(_changed).toBe(true); - ({ _value, _previous, _changed } = update(0, firstCtx)); - expect(updateFn).toHaveBeenLastCalledWith(firstCtx, true, undefined); - expect(_value).toBe(true); - expect(_previous).toBe(undefined); - expect(_changed).toBe(false); + ({ _value, _previous, _changed } = update(0, firstCtx)); + expect(updateFn).toHaveBeenLastCalledWith(firstCtx, true, undefined); + expect(_value).toBe(true); + expect(_previous).toBe(undefined); + expect(_changed).toBe(false); - const scndCtx = { test: 'nah', even: 1 }; + const scndCtx = { test: 'nah', even: 1 }; - ({ _value, _previous, _changed } = update(0, scndCtx)); - expect(updateFn).toHaveBeenLastCalledWith(scndCtx, true, undefined); - expect(_value).toBe(false); - expect(_previous).toBe(true); - expect(_changed).toBe(true); + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(updateFn).toHaveBeenLastCalledWith(scndCtx, true, undefined); + expect(_value).toBe(false); + expect(_previous).toBe(true); + expect(_changed).toBe(true); - ({ _value, _previous, _changed } = update(0, scndCtx)); - expect(updateFn).toHaveBeenLastCalledWith(scndCtx, false, true); - expect(_value).toBe(false); - expect(_previous).toBe(true); - expect(_changed).toBe(false); + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(updateFn).toHaveBeenLastCalledWith(scndCtx, false, true); + expect(_value).toBe(false); + expect(_previous).toBe(true); + expect(_changed).toBe(false); - ({ _value, _previous, _changed } = update(true, scndCtx)); - expect(updateFn).toHaveBeenLastCalledWith(scndCtx, false, true); - expect(_value).toBe(false); - expect(_previous).toBe(false); - expect(_changed).toBe(true); + ({ _value, _previous, _changed } = update(true, scndCtx)); + expect(updateFn).toHaveBeenLastCalledWith(scndCtx, false, true); + expect(_value).toBe(false); + expect(_previous).toBe(false); + expect(_changed).toBe(true); + }); + + test('creates and updates cache with context shorthand', () => { + interface ContextObj { + test: string; + even: number; + } + const update = createCache(0); + const firstCtx = { test: 'test', even: 2 }; + + let { _value, _previous, _changed } = update(0, firstCtx); + expect(_value).toBe(firstCtx); + expect(_previous).toBe(undefined); + expect(_changed).toBe(true); + + ({ _value, _previous, _changed } = update(0, firstCtx)); + expect(_value).toBe(firstCtx); + expect(_previous).toBe(undefined); + expect(_changed).toBe(false); + + const scndCtx = { test: 'nah', even: 1 }; + + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(firstCtx); + expect(_changed).toBe(true); + + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(firstCtx); + expect(_changed).toBe(false); + + ({ _value, _previous, _changed } = update(true, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(scndCtx); + expect(_changed).toBe(true); + }); }); describe('equal', () => { @@ -131,7 +169,7 @@ describe('cache', () => { }); describe('inital value', () => { - test('creates and updates cache with inital value', () => { + test('creates and updates cache with initialValue', () => { const [fn, updater] = createUpdater((i) => i); const update = createCache(updater, { _initialValue: 0 }); @@ -148,7 +186,7 @@ describe('cache', () => { expect(_changed).toBe(true); }); - test('creates and updates cache with inital value and equal', () => { + test('creates and updates cache with initialValue and equal', () => { const obj = { a: -1, b: -1 }; const [fn, updater] = createUpdater((i) => ({ a: i, b: i + 1 })); const update = createCache(updater, { _initialValue: obj, _equal: (a, b) => a?.a === b?.a && a?.b === b?.b }); @@ -167,6 +205,61 @@ describe('cache', () => { }); }); + describe('always update values', () => { + test('creates and updates cache with alwaysUpdateValues and equal always true', () => { + const [fn, updater] = createUpdater((i) => i); + const update = createCache(updater, { _alwaysUpdateValues: true, _equal: () => true }); + + let { _value, _previous, _changed } = update(); + expect(fn).toHaveBeenLastCalledWith(undefined, undefined, undefined); + expect(_value).toBe(1); + expect(_previous).toBe(undefined); + expect(_changed).toBe(false); + + ({ _value, _previous, _changed } = update()); + expect(fn).toHaveBeenLastCalledWith(undefined, 1, undefined); + expect(_value).toBe(2); + expect(_previous).toBe(1); + expect(_changed).toBe(false); + }); + + test('creates and updates cache with context shorthand and alwaysUpdateValues', () => { + interface ContextObj { + test: string; + even: number; + } + const update = createCache(0, { _alwaysUpdateValues: true }); + const firstCtx = { test: 'test', even: 2 }; + + let { _value, _previous, _changed } = update(0, firstCtx); + expect(_value).toBe(firstCtx); + expect(_previous).toBe(undefined); + expect(_changed).toBe(true); + + ({ _value, _previous, _changed } = update(0, firstCtx)); + expect(_value).toBe(firstCtx); + expect(_previous).toBe(firstCtx); + expect(_changed).toBe(false); + + const scndCtx = { test: 'nah', even: 1 }; + + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(firstCtx); + expect(_changed).toBe(true); + + ({ _value, _previous, _changed } = update(0, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(scndCtx); + expect(_changed).toBe(false); + + ({ _value, _previous, _changed } = update(true, scndCtx)); + expect(_value).toBe(scndCtx); + expect(_previous).toBe(scndCtx); + expect(_changed).toBe(true); + }); + }); + describe('constant', () => { test('updates constant initially without intial value', () => { const [fn, updater] = createUpdater(() => true);