Skip to content

Commit

Permalink
Merge pull request #541 from ckeditor/ck/fix-undefined-behavior
Browse files Browse the repository at this point in the history
Fix: Behavior of `useCKEditorCloud` hook is now consistent on `Vite` and `Next` runtimes while changing properties.
Internal: Fix incorrect typings of `useAsyncCallback` and `useAsyncValue` callback arguments types.
Tests: Add missing cleanup methods to few tests.
  • Loading branch information
Mati365 authored Sep 30, 2024
2 parents 8f5232c + bd261f1 commit 8886ca3
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 29 deletions.
6 changes: 3 additions & 3 deletions src/hooks/useAsyncCallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { useRefSafeCallback } from './useRefSafeCallback.js';
* ```
*/
export const useAsyncCallback = <A extends Array<unknown>, R>(
callback: ( ...args: Array<A> ) => Promise<R>
callback: ( ...args: A ) => Promise<R>
): AsyncCallbackHookResult<A, R> => {
// The state of the asynchronous callback.
const [ asyncState, setAsyncState ] = useState<AsyncCallbackState<R>>( {
Expand All @@ -50,7 +50,7 @@ export const useAsyncCallback = <A extends Array<unknown>, R>(
const prevExecutionUIDRef = useRef<string | null>( null );

// The asynchronous executor function, which is a wrapped version of the original callback.
const asyncExecutor = useRefSafeCallback( async ( ...args: Array<any> ) => {
const asyncExecutor = useRefSafeCallback( async ( ...args: A ) => {
if ( unmountedRef.current || isSSR() ) {
return null;
}
Expand Down Expand Up @@ -101,7 +101,7 @@ export const useAsyncCallback = <A extends Array<unknown>, R>(
* Represents the result of the `useAsyncCallback` hook.
*/
export type AsyncCallbackHookResult<A extends Array<unknown>, R> = [
( ...args: Array<A> ) => Promise<R | null>,
( ...args: A ) => Promise<R | null>,
AsyncCallbackState<R>
];

Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useAsyncValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { useAsyncCallback, type AsyncCallbackState } from './useAsyncCallback.js
* ```
*/
export const useAsyncValue = <A extends Array<unknown>, R>(
callback: ( ...args: Array<A> ) => Promise<R>,
callback: ( ...args: A ) => Promise<R>,
deps: DependencyList
): AsyncValueHookResult<R> => {
const [ asyncCallback, asyncState ] = useAsyncCallback( callback );
Expand Down
8 changes: 4 additions & 4 deletions src/hooks/useInstantEffect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md.
*/

import { useRef, type DependencyList } from 'react';
import { useState, type DependencyList } from 'react';
import { shallowCompareArrays } from '@ckeditor/ckeditor5-integrations-common';

/**
Expand All @@ -13,10 +13,10 @@ import { shallowCompareArrays } from '@ckeditor/ckeditor5-integrations-common';
* @param deps The dependency list.
*/
export const useInstantEffect = ( fn: VoidFunction, deps: DependencyList ): void => {
const prevDeps = useRef<any>( null );
const [ prevDeps, setDeps ] = useState<any>( null );

if ( !shallowCompareArrays( prevDeps.current, deps ) ) {
prevDeps.current = [ ...deps ];
if ( !shallowCompareArrays( prevDeps, deps ) ) {
fn();
setDeps( [ ...deps ] );
}
};
9 changes: 6 additions & 3 deletions tests/cloud/useCKEditorCloud.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
* For licensing, see LICENSE.md.
*/

import { beforeEach, describe, expect, expectTypeOf, it } from 'vitest';
import { renderHook, waitFor, act } from '@testing-library/react';
import { afterEach, describe, expect, expectTypeOf, it } from 'vitest';
import { renderHook, waitFor, act, cleanup } from '@testing-library/react';

import type { CKEditorCloudConfig } from '@ckeditor/ckeditor5-integrations-common';
import { removeAllCkCdnResources } from '@ckeditor/ckeditor5-integrations-common/test-utils';

import useCKEditorCloud from '../../src/cloud/useCKEditorCloud.js';

describe( 'useCKEditorCloud', () => {
beforeEach( removeAllCkCdnResources );
afterEach( () => {
cleanup();
removeAllCkCdnResources();
} );

it( 'should load CKEditor bundles from CDN', async () => {
const { result } = renderHook( () => useCKEditorCloud( {
Expand Down
7 changes: 3 additions & 4 deletions tests/cloud/withCKEditorCloud.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React, { type MutableRefObject } from 'react';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, render } from '@testing-library/react';

import { createDefer } from '@ckeditor/ckeditor5-integrations-common';
Expand All @@ -17,9 +17,8 @@ describe( 'withCKEditorCloud', () => {
current: null
};

afterEach( cleanup );

beforeEach( () => {
afterEach( () => {
cleanup();
removeAllCkCdnResources();
lastRenderedMockProps.current = null;
} );
Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useAsyncCallback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it, vi } from 'vitest';
import { renderHook, act, waitFor } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { renderHook, act, waitFor, cleanup } from '@testing-library/react';
import { useAsyncCallback } from '../../src/hooks/useAsyncCallback.js';
import { timeout } from '../_utils/timeout.js';

describe( 'useAsyncCallback', () => {
afterEach( cleanup );

it( 'should execute the callback and update the state correctly when the callback resolves', async () => {
const fetchData = vi.fn().mockResolvedValue( 'data' );

Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useAsyncValue.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it } from 'vitest';
import { renderHook, waitFor } from '@testing-library/react';
import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook, waitFor } from '@testing-library/react';
import { useAsyncValue } from '../../src/hooks/useAsyncValue.js';

describe( 'useAsyncValue', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', async () => {
const { result } = renderHook( () => useAsyncValue( async () => 123, [] ) );

Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useInstantEditorEffect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it, vi } from 'vitest';
import { renderHook } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { useInstantEditorEffect } from '../../src/hooks/useInstantEditorEffect.js';

describe( 'useInstantEditorEffect', () => {
afterEach( cleanup );

it( 'should execute the provided function after mounting of editor', () => {
const semaphore = {
runAfterMount: vi.fn()
Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useInstantEffect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it, vi } from 'vitest';
import { renderHook } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { useInstantEffect } from '../../src/hooks/useInstantEffect.js';

describe( 'useInstantEffect', () => {
afterEach( cleanup );

it( 'should call the effect function when dependencies change', () => {
const effectFn = vi.fn();
const { rerender } = renderHook( deps => useInstantEffect( effectFn, deps ), {
Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useIsMountedRef.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it } from 'vitest';
import { renderHook } from '@testing-library/react';
import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { useIsMountedRef } from '../../src/hooks/useIsMountedRef.js';

describe( 'useIsMountedRef', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', () => {
const { result } = renderHook( () => useIsMountedRef() );

Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useIsUnmountedRef.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { describe, expect, it } from 'vitest';
import { renderHook } from '@testing-library/react';
import { afterEach, describe, expect, it } from 'vitest';
import { cleanup, renderHook } from '@testing-library/react';
import { useIsUnmountedRef } from '../../src/hooks/useIsUnmountedRef.js';

describe( 'useIsUnmountedRef', () => {
afterEach( cleanup );

it( 'should return a mutable ref object', () => {
const { result } = renderHook( () => useIsUnmountedRef() );

Expand Down
6 changes: 4 additions & 2 deletions tests/hooks/useRefSafeCallback.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
* For licensing, see LICENSE.md.
*/

import { expect, it, describe, vi } from 'vitest';
import { renderHook, act } from '@testing-library/react';
import { expect, it, describe, vi, afterEach } from 'vitest';
import { renderHook, act, cleanup } from '@testing-library/react';
import { useRefSafeCallback } from '../../src/hooks/useRefSafeCallback.js';

describe( 'useRefSafeCallback', () => {
afterEach( cleanup );

it( 'should return a function', () => {
const { result } = renderHook( () => useRefSafeCallback( () => {} ) );

Expand Down

0 comments on commit 8886ca3

Please sign in to comment.