From 437a3ee9d0561223b4cb41abcc148ae6c827249f Mon Sep 17 00:00:00 2001 From: sergeyteleshev Date: Wed, 17 Jul 2024 12:39:55 +0200 Subject: [PATCH] CB-5340 fixes unit tests --- .../src/useControlledScroll.test.ts | 31 +++++ .../core-blocks/src/useObjectRef.test.ts | 106 +++++++----------- .../packages/core-blocks/src/useObjectRef.ts | 12 +- .../core-blocks/src/useObservableRef.test.ts | 71 ++++++------ .../core-blocks/src/useObservableRef.ts | 16 +-- .../core-utils/src/bindFunctions.test.ts | 61 ++++++++++ .../packages/core-utils/src/bindFunctions.ts | 16 +++ webapp/packages/core-utils/src/index.ts | 1 + 8 files changed, 190 insertions(+), 124 deletions(-) create mode 100644 webapp/packages/core-utils/src/bindFunctions.test.ts create mode 100644 webapp/packages/core-utils/src/bindFunctions.ts diff --git a/webapp/packages/core-blocks/src/useControlledScroll.test.ts b/webapp/packages/core-blocks/src/useControlledScroll.test.ts index 826ee50abb..728ed0d905 100644 --- a/webapp/packages/core-blocks/src/useControlledScroll.test.ts +++ b/webapp/packages/core-blocks/src/useControlledScroll.test.ts @@ -82,6 +82,37 @@ describe('useControlledScroll', () => { expect(true).toBe(true); }); + it('should change element and apply the current state to it', () => { + const { rerender } = renderHook(({ el, state }) => useControlledScroll(el, state), { + initialProps: { el: element, state: scrollState }, + }); + + const newElement = document.createElement('div'); + + rerender({ el: newElement, state: scrollState }); + + jest.runAllTimers(); + + expect(newElement.scrollTop).toBe(0); + expect(newElement.scrollLeft).toBe(0); + }); + + it('should change element and state and apply the new state to the new element', () => { + const { rerender } = renderHook(({ el, state }) => useControlledScroll(el, state), { + initialProps: { el: element, state: scrollState }, + }); + + const newElement = document.createElement('div'); + const newState = { scrollTop: 75, scrollLeft: 25 }; + + rerender({ el: newElement, state: newState }); + + jest.runAllTimers(); + + expect(newElement.scrollTop).toBe(75); + expect(newElement.scrollLeft).toBe(25); + }); + it('should remove event listener if element becomes null', () => { const { rerender } = renderHook(({ el, state }) => useControlledScroll(el, state), { initialProps: { el: element as HTMLDivElement | null, state: scrollState }, diff --git a/webapp/packages/core-blocks/src/useObjectRef.test.ts b/webapp/packages/core-blocks/src/useObjectRef.test.ts index a00fa696e5..efcf62c1c5 100644 --- a/webapp/packages/core-blocks/src/useObjectRef.test.ts +++ b/webapp/packages/core-blocks/src/useObjectRef.test.ts @@ -7,9 +7,21 @@ */ import { renderHook } from '@testing-library/react'; +import * as coreUtils from '@cloudbeaver/core-utils'; + import { useObjectRef } from './useObjectRef'; +jest.mock('@cloudbeaver/core-utils', () => ({ + bindFunctions: jest.fn(), +})); + describe('useObjectRef', () => { + const bindFunctions = jest.spyOn(coreUtils, 'bindFunctions'); + + beforeAll(() => { + bindFunctions.mockClear(); + }); + test('should initialize', () => { const { result } = renderHook(() => useObjectRef({ @@ -30,20 +42,8 @@ describe('useObjectRef', () => { expect(result.current).toEqual({}); }); - test('should change ref value', () => { - const { result } = renderHook(() => - useObjectRef({ - count: 0, - }), - ); - - result.current.count = 123; - - expect(result.current.count).toBe(123); - }); - test('should bind ref functions', () => { - const { result } = renderHook(() => + renderHook(() => useObjectRef( () => ({ count: 0, @@ -55,76 +55,48 @@ describe('useObjectRef', () => { ['increment'], ), ); - const anotherContextObject = { - count: 123, - increment: result.current.increment, - }; - result.current.increment(); - - expect(result.current.count).toBe(1); - - anotherContextObject.increment(); - - expect(anotherContextObject.count).toBe(123); - expect(result.current.count).toBe(2); + expect(bindFunctions).toHaveBeenCalledTimes(1); }); - it('should not bind with empty bind array', () => { - const { result } = renderHook(() => - useObjectRef( - () => ({ + test('should update ref via initial state method', () => { + type TRef = { update: { count: number; increment?: (this: { count: number }) => void } }; + const init = () => ({ count: 0 }); + + const { result } = renderHook(({ update }: TRef) => useObjectRef(init, update, ['increment']), { + initialProps: { + update: { count: 0, - increment: function (this: { count: number }) { + increment: function () { this.count++; }, - }), - false, - [], - ), - ); - - expect(typeof result.current.increment).toBe('function'); - - const anotherContextObject = { - count: 123, - increment: result.current.increment, - }; - - anotherContextObject.increment(); - - expect(result.current.count).toBe(0); - expect(anotherContextObject.count).toBe(124); - }); - - test('should update ref', () => { - const { result, rerender } = renderHook( - ({ update }: { update: { count: number; increment?: (this: { count: number }) => void } }) => - useObjectRef(() => ({ count: 0 }), update, ['increment'] as const), - { - initialProps: { - update: { - count: 0, - }, }, }, - ); + }); expect(result.current.count).toBe(0); result.current?.increment?.(); - expect(result.current.count).toBe(0); + expect(result.current.count).toBe(1); + }); - rerender({ - update: { - count: 1, - increment: result.current.increment, - }, + test('should update ref', () => { + type TRef = { update: { count: number } }; + const init = () => ({ count: 0 }); + + const { result, rerender } = renderHook(({ update }: TRef) => useObjectRef(init, update), { + initialProps: { + update: { + count: 0, + }, + } as TRef, }); - result.current?.increment?.(); + expect(result.current.count).toBe(0); - expect(result.current.count).toBe(1); + rerender({ update: { count: 3 } }); + + expect(result.current.count).toBe(3); }); }); diff --git a/webapp/packages/core-blocks/src/useObjectRef.ts b/webapp/packages/core-blocks/src/useObjectRef.ts index 70922de855..89b46a9c08 100644 --- a/webapp/packages/core-blocks/src/useObjectRef.ts +++ b/webapp/packages/core-blocks/src/useObjectRef.ts @@ -7,6 +7,8 @@ */ import { useState } from 'react'; +import { bindFunctions } from '@cloudbeaver/core-utils'; + export function useObjectRef>(init: () => T & ThisType, update: false, bind?: Array): T; export function useObjectRef>(init: () => T & ThisType): T; export function useObjectRef, U extends Record>( @@ -58,13 +60,3 @@ export function useObjectRef, U extends Record(object: T, keys: Array): void { - for (const key of keys) { - const value = object[key]; - - if (typeof value === 'function') { - object[key] = value.bind(object); - } - } -} diff --git a/webapp/packages/core-blocks/src/useObservableRef.test.ts b/webapp/packages/core-blocks/src/useObservableRef.test.ts index 4b5c15770e..fc55534c08 100644 --- a/webapp/packages/core-blocks/src/useObservableRef.test.ts +++ b/webapp/packages/core-blocks/src/useObservableRef.test.ts @@ -8,10 +8,22 @@ import { renderHook } from '@testing-library/react'; import { action, computed, isObservable, observable, runInAction } from 'mobx'; +import * as coreUtils from '@cloudbeaver/core-utils'; + import { useObservableRef } from './useObservableRef'; +jest.mock('@cloudbeaver/core-utils', () => ({ + bindFunctions: jest.fn(), +})); + describe('useObservableRef', () => { - test('initializes with a function', () => { + const bindFunctions = jest.spyOn(coreUtils, 'bindFunctions'); + + beforeAll(() => { + bindFunctions.mockClear(); + }); + + test('should initialize with a function', () => { const init = () => ({ count: 0 }); const observed = { count: observable }; @@ -21,7 +33,7 @@ describe('useObservableRef', () => { expect(isObservable(result.current)).toBe(true); }); - test('initializes with an object', () => { + test('should initialize with an object', () => { const init = { count: 0 }; const observed = { count: observable }; @@ -31,21 +43,10 @@ describe('useObservableRef', () => { expect(isObservable(result.current)).toBe(true); }); - test('updates the object', () => { - const init = () => ({ count: 0 }); - const observed = { count: observable }; - const update = { count: 1 }; - - const { result } = renderHook(() => useObservableRef(init, observed, update)); - - expect(result.current.count).toBe(1); - expect(isObservable(result.current)).toBe(true); - }); - - test('binds functions', () => { + test('should bind functions', () => { const observed = { count: observable, increment: action }; - const { result } = renderHook(() => + renderHook(() => useObservableRef( () => ({ count: 0, @@ -59,16 +60,7 @@ describe('useObservableRef', () => { ), ); - expect(typeof result.current.increment).toBe('function'); - result.current.increment(); - expect(result.current.count).toBe(1); - - const anotherContext = { count: 0, increment: result.current.increment }; - - anotherContext.increment(); - - expect(anotherContext.count).toBe(0); - expect(result.current.count).toBe(2); + expect(bindFunctions).toHaveBeenCalledTimes(1); }); test('handles computed properties', () => { @@ -89,7 +81,7 @@ describe('useObservableRef', () => { expect(result.current.doubleCount).toBe(10); }); - test('handles partial updates', () => { + test('should merge update param to initial state', () => { const init = () => ({ count: 0, text: 'hello' }); const observed = { count: observable, text: observable }; const update = { count: 1 }; @@ -101,7 +93,7 @@ describe('useObservableRef', () => { expect(isObservable(result.current)).toBe(true); }); - test('handles update as bind array', () => { + test('should merge update to bind', () => { const init = () => ({ count: 0, increment: function (this: { count: number }) { @@ -111,17 +103,26 @@ describe('useObservableRef', () => { const observed = { count: observable, increment: action }; const update = ['increment']; - const { result } = renderHook(() => useObservableRef(init, observed, update)); + renderHook(() => useObservableRef(init, observed, update)); - expect(typeof result.current.increment).toBe('function'); - result.current.increment(); - expect(result.current.count).toBe(1); + expect(bindFunctions).toHaveBeenCalledTimes(2); + }); + + test('should update ref', () => { + interface Update { + count: number; + str?: string; + } + const init = () => ({ count: 0 }) as Update; + const observed = { count: observable }; + + const { result, rerender } = renderHook((update: Update) => useObservableRef(init, observed, update)); - const anotherContext = { count: 0, increment: result.current.increment }; + expect(result.current.count).toBe(0); - anotherContext.increment(); + rerender({ count: 3, str: 'hello' }); - expect(anotherContext.count).toBe(0); - expect(result.current.count).toBe(2); + expect(result.current.count).toBe(3); + expect(result.current.str).toBe('hello'); }); }); diff --git a/webapp/packages/core-blocks/src/useObservableRef.ts b/webapp/packages/core-blocks/src/useObservableRef.ts index 95cf4be874..55d6ed9c11 100644 --- a/webapp/packages/core-blocks/src/useObservableRef.ts +++ b/webapp/packages/core-blocks/src/useObservableRef.ts @@ -8,6 +8,8 @@ import { action, AnnotationsMap, makeObservable, runInAction, untracked } from 'mobx'; import { useLayoutEffect, useState } from 'react'; +import { bindFunctions } from '@cloudbeaver/core-utils'; + export function useObservableRef>( init: () => T & ThisType, observed: AnnotationsMap, @@ -70,12 +72,12 @@ export function useObservableRef>( update = undefined; } + state = makeObservable(state, observed, { deep: false, name }); + if (bind) { bindFunctions(state, bind as []); } - state = makeObservable(state, observed, { deep: false, name }); - return state; }); @@ -98,16 +100,6 @@ export function useObservableRef>( return state; } -function bindFunctions(object: T, keys: Array): void { - for (const key of keys) { - const value = object[key]; - - if (typeof value === 'function') { - object[key] = value.bind(object); - } - } -} - function assign(object: any, update: any): void { for (const [key, value] of Object.entries(update)) { if (!(key in object) || object[key] !== value) { diff --git a/webapp/packages/core-utils/src/bindFunctions.test.ts b/webapp/packages/core-utils/src/bindFunctions.test.ts new file mode 100644 index 0000000000..31282df79c --- /dev/null +++ b/webapp/packages/core-utils/src/bindFunctions.test.ts @@ -0,0 +1,61 @@ +/* + * CloudBeaver - Cloud Database Manager + * Copyright (C) 2020-2024 DBeaver Corp and others + * + * Licensed under the Apache License, Version 2.0. + * you may not use this file except in compliance with the License. + */ +import { bindFunctions } from './bindFunctions'; + +describe('bindFunctions', () => { + test('binds specified functions to the object', () => { + const obj = { + name: 'TestObject', + greet: function () { + return `Hello, I'm ${this.name}`; + }, + farewell: function () { + return `Goodbye from ${this.name}`; + }, + notAFunction: 'This is not a function', + }; + + const originalGreet = obj.greet; + const originalFarewell = obj.farewell; + + bindFunctions(obj, ['greet', 'farewell', 'notAFunction']); + + expect(obj.greet).not.toBe(originalGreet); + expect(obj.farewell).not.toBe(originalFarewell); + + expect(obj.greet()).toBe("Hello, I'm TestObject"); + expect(obj.farewell()).toBe('Goodbye from TestObject'); + + expect(obj.notAFunction).toBe('This is not a function'); + }); + + test('does not modify object if no function keys are provided', () => { + const obj = { + name: 'TestObject', + greet: function () { + return `Hello, I'm ${this.name}`; + }, + }; + + const originalGreet = obj.greet; + + bindFunctions(obj, []); + + expect(obj.greet).toBe(originalGreet); + }); + + test('handles objects with no matching keys', () => { + const obj = { + name: 'TestObject', + }; + + expect(() => { + bindFunctions(obj, ['nonexistentMethod' as never]); + }).not.toThrow(); + }); +}); diff --git a/webapp/packages/core-utils/src/bindFunctions.ts b/webapp/packages/core-utils/src/bindFunctions.ts new file mode 100644 index 0000000000..5ff089524e --- /dev/null +++ b/webapp/packages/core-utils/src/bindFunctions.ts @@ -0,0 +1,16 @@ +/* + * CloudBeaver - Cloud Database Manager + * Copyright (C) 2020-2024 DBeaver Corp and others + * + * Licensed under the Apache License, Version 2.0. + * you may not use this file except in compliance with the License. + */ +export function bindFunctions(object: T, keys: Array): void { + for (const key of keys) { + const value = object[key]; + + if (typeof value === 'function') { + object[key] = value.bind(object); + } + } +} diff --git a/webapp/packages/core-utils/src/index.ts b/webapp/packages/core-utils/src/index.ts index a54e923531..8f1c2d4534 100644 --- a/webapp/packages/core-utils/src/index.ts +++ b/webapp/packages/core-utils/src/index.ts @@ -86,3 +86,4 @@ export * from './withTimestamp'; export * from './toSafeHtmlString'; export * from './getProgressPercent'; export * from './types/UndefinedToNull'; +export * from './bindFunctions';