diff --git a/.changeset/soft-fishes-warn.md b/.changeset/soft-fishes-warn.md new file mode 100644 index 000000000..a8c71ed32 --- /dev/null +++ b/.changeset/soft-fishes-warn.md @@ -0,0 +1,5 @@ +--- +"@suspensive/react": minor +--- + +feat(react): fix bugs in `lazy` and stabilize it diff --git a/docs/suspensive.org/src/content/en/docs/react/lazy.mdx b/docs/suspensive.org/src/content/en/docs/react/lazy.mdx index a428b0a02..d62dae0b3 100644 --- a/docs/suspensive.org/src/content/en/docs/react/lazy.mdx +++ b/docs/suspensive.org/src/content/en/docs/react/lazy.mdx @@ -2,12 +2,6 @@ import { Callout } from '@/components' # lazy - - -`lazy` is an experimental feature, so this interface may change. - - - The `lazy` function is a wrapper around React's `lazy` function that provides callbacks for component loading success and failure. It allows you to execute custom logic when a component loads successfully or fails, providing better user experience and debugging capabilities. ### Preloading Components diff --git a/docs/suspensive.org/src/content/ko/docs/react/lazy.mdx b/docs/suspensive.org/src/content/ko/docs/react/lazy.mdx index 8ea6784a2..6b19cd81a 100644 --- a/docs/suspensive.org/src/content/ko/docs/react/lazy.mdx +++ b/docs/suspensive.org/src/content/ko/docs/react/lazy.mdx @@ -2,12 +2,6 @@ import { Callout } from '@/components' # lazy - - -`lazy`는 실험 기능이므로 이 인터페이스는 변경될 수 있습니다. - - - `lazy` 함수는 React의 `lazy` 함수를 래핑하여 컴포넌트 로딩 성공과 실패에 대한 콜백을 제공합니다. 컴포넌트가 성공적으로 로드되거나 실패할 때 사용자 정의 로직을 실행할 수 있어 더 나은 사용자 경험과 디버깅을 제공합니다. ### 컴포넌트 사전 로딩 diff --git a/packages/react/src/lazy.spec.tsx b/packages/react/src/lazy.spec.tsx index 6e991d555..596d00a2c 100644 --- a/packages/react/src/lazy.spec.tsx +++ b/packages/react/src/lazy.spec.tsx @@ -23,11 +23,11 @@ class ImportCache { createImport(options: ImportOptions) { return vi - .fn<(path: string) => Promise<{ default: ComponentType }>>() + .fn<(path: string) => Promise<{ default: ComponentType<{ x: string }> }>>() .mockImplementation((path: string) => this.import(path, options)) } - private async import(path: string, options: ImportOptions): Promise<{ default: ComponentType }> { + private async import(path: string, options: ImportOptions): Promise<{ default: ComponentType<{ x: string }> }> { const data = this.pathData.get(path) || { cache: null, attempt: 0, failureCount: 0 } if (data.cache) { @@ -54,7 +54,7 @@ class ImportCache { path: string, attempt: number, { failureCount, failureDelay, successDelay }: ImportOptions - ): Promise<{ default: ComponentType }> { + ): Promise<{ default: ComponentType<{ x: string }> }> { if (attempt < failureCount) { await sleep(failureDelay) @@ -63,7 +63,13 @@ class ImportCache { await sleep(successDelay) - return { default: () =>
Component from {path}
} + return { + default: ({ x }: { x: string }) => ( +
+ Component from {path} x:{x} +
+ ), + } } clear() { @@ -146,7 +152,7 @@ describe('lazy', () => { {isShow ? ( loading...}> - + ) : (
not loaded
@@ -158,18 +164,18 @@ describe('lazy', () => { render() expect(screen.getByText('not loaded')).toBeInTheDocument() - expect(screen.queryByText('Component from /test-component')).not.toBeInTheDocument() + expect(screen.queryByText('Component from /test-component x:test')).not.toBeInTheDocument() screen.getByRole('button', { name: 'load' }).click() expect(screen.getByText('not loaded')).toBeInTheDocument() - expect(screen.queryByText('Component from /test-component')).not.toBeInTheDocument() + expect(screen.queryByText('Component from /test-component x:test')).not.toBeInTheDocument() screen.getByRole('button', { name: 'show' }).click() expect(screen.getByText('not loaded')).toBeInTheDocument() await act(() => vi.advanceTimersByTimeAsync(0)) expect(screen.getByText('loading...')).toBeInTheDocument() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(screen.queryByText('not loaded')).not.toBeInTheDocument() }) @@ -179,21 +185,21 @@ describe('lazy', () => { const Component1 = lazy(() => mockImport('/cached-component1')) const Component2 = lazy(() => mockImport('/cached-component2')) - render() + render() expect(importCache.getAttempt('/cached-component1')).toBe(1) expect(importCache.isCached('/cached-component1')).toBe(false) await act(() => vi.advanceTimersByTimeAsync(100)) expect(importCache.isCached('/cached-component1')).toBe(true) - render() + render() expect(importCache.getAttempt('/cached-component2')).toBe(1) expect(importCache.isCached('/cached-component2')).toBe(false) await act(() => vi.advanceTimersByTimeAsync(100)) expect(importCache.isCached('/cached-component2')).toBe(true) - render() + render() expect(importCache.getAttempt('/cached-component1')).toBe(1) expect(importCache.isCached('/cached-component1')).toBe(true) @@ -211,7 +217,7 @@ describe('lazy', () => { render( error}> loading...}> - + ) @@ -225,14 +231,14 @@ describe('lazy', () => { render( error}> loading...}> - + ) expect(screen.getByText('loading...')).toBeInTheDocument() await act(() => vi.advanceTimersByTimeAsync(200)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() }) it('should handle permanently failing imports', async () => { @@ -241,7 +247,7 @@ describe('lazy', () => { render( error}> - + ) @@ -258,10 +264,10 @@ describe('lazy', () => { const Component = lazy(() => mockImport('/test-component')) - render() + render() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(onSuccess).toHaveBeenCalledTimes(1) }) @@ -275,7 +281,7 @@ describe('lazy', () => { render( error}> - + ) @@ -293,10 +299,10 @@ describe('lazy', () => { const Component = lazy(() => mockImport('/test-component'), { onSuccess }) - render() + render() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(onSuccess).toHaveBeenCalledTimes(1) }) @@ -309,7 +315,7 @@ describe('lazy', () => { render( error}> - + ) @@ -337,7 +343,7 @@ describe('lazy', () => { render( error}> - + ) @@ -349,7 +355,7 @@ describe('lazy', () => { expect(callOrder).toEqual(['individual', 'factory']) }) - it('should execute default onSuccess first, then component onSuccess', async () => { + it('should execute component onSuccess first, then default onSuccess', async () => { const mockImport = importCache.createImport({ failureCount: 0, failureDelay: 50, successDelay: 100 }) const callOrder: string[] = [] const defaultOnSuccess = vi.fn().mockImplementation(() => { @@ -364,10 +370,10 @@ describe('lazy', () => { onSuccess: individualOnSuccess, }) - render() + render() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(defaultOnSuccess).toHaveBeenCalledTimes(1) expect(individualOnSuccess).toHaveBeenCalledTimes(1) @@ -381,10 +387,10 @@ describe('lazy', () => { const Component = lazy(() => mockImport('/test-component')) - render() + render() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(defaultOnSuccess).toHaveBeenCalledTimes(1) }) @@ -398,7 +404,7 @@ describe('lazy', () => { render( error}> - + ) @@ -414,10 +420,10 @@ describe('lazy', () => { const Component = lazy(() => mockImport('/test-component')) - expect(() => render()).not.toThrow() + expect(() => render()).not.toThrow() await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() }) }) @@ -434,7 +440,7 @@ describe('lazy', () => { render( error}> - + ) @@ -450,7 +456,7 @@ describe('lazy', () => { render( error}> - + ) @@ -470,7 +476,7 @@ describe('lazy', () => { render( error}> - + ) @@ -486,14 +492,14 @@ describe('lazy', () => { render( error}> - + ) expect(mockImport).toHaveBeenCalledTimes(2) await act(() => vi.advanceTimersByTimeAsync(50)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(mockReload).toHaveBeenCalledTimes(1) }) @@ -505,8 +511,8 @@ describe('lazy', () => { const Component = lazy(() => mockImport('/test-component')) const Component2 = lazy(() => mockImport('/test-component')) - expect(() => render()).not.toThrow() - expect(() => render()).not.toThrow() + expect(() => render()).not.toThrow() + expect(() => render()).not.toThrow() expect(mockReload).toHaveBeenCalledTimes(0) expect(storage.length).toBe(0) @@ -522,7 +528,7 @@ describe('lazy', () => { render( error}> - + ) @@ -549,7 +555,7 @@ describe('lazy', () => { render( error}> - + ) @@ -570,7 +576,7 @@ describe('lazy', () => { render( error}> - + ) @@ -595,7 +601,7 @@ describe('lazy', () => { render( error}> - + ) @@ -604,11 +610,10 @@ describe('lazy', () => { await act(() => vi.advanceTimersByTimeAsync(100)) expect(screen.getByText('error')).toBeInTheDocument() - // Should remove invalid value, but currentRetryCount becomes NaN, so it won't retry - // This is the actual behavior - when NaN is found, it's removed but currentRetryCount is still NaN - expect(storage.getItem(loadFunction.toString())).toBeNull() + // Should remove invalid value and reset retry count to 0, so it retries normally + expect(storage.getItem(loadFunction.toString())).toBe('1') await act(() => vi.advanceTimersByTimeAsync(1)) - expect(mockReload).toHaveBeenCalledTimes(0) + expect(mockReload).toHaveBeenCalledTimes(1) }) it('should not reload when retry count exceeds limit', async () => { @@ -622,7 +627,7 @@ describe('lazy', () => { render( error}> - + ) @@ -636,6 +641,39 @@ describe('lazy', () => { expect(mockReload).toHaveBeenCalledTimes(0) }) + it('should use separate storage keys for different lazy components', async () => { + const lazy = createLazy(reloadOnError({ storage, reload: mockReload, retry: 1 })) + const mockImport = importCache.createImport({ failureCount: 10, failureDelay: 50, successDelay: 50 }) + + const Component1 = lazy(() => mockImport('/component-1')) + const Component2 = lazy(() => mockImport('/component-2')) + + render( + error1}> + + + ) + + await act(() => vi.advanceTimersByTimeAsync(50)) + expect(screen.getByText('error1')).toBeInTheDocument() + + await act(() => vi.advanceTimersByTimeAsync(1)) + expect(mockReload).toHaveBeenCalledTimes(1) + + // Component2 should still be able to retry (not affected by Component1's retry count) + render( + error2}> + + + ) + + await act(() => vi.advanceTimersByTimeAsync(50)) + expect(screen.getByText('error2')).toBeInTheDocument() + + await act(() => vi.advanceTimersByTimeAsync(1)) + expect(mockReload).toHaveBeenCalledTimes(2) + }) + it('should throw error when storage is not provided and window.sessionStorage does not exist', () => { const originalWindow = global.window // @ts-expect-error - intentionally removing window @@ -674,7 +712,7 @@ describe('lazy', () => { render( error}> - + ) @@ -707,7 +745,7 @@ describe('lazy', () => { render( error}> - + ) @@ -745,7 +783,7 @@ describe('lazy', () => { render( error}> - + ) @@ -772,14 +810,14 @@ describe('lazy', () => { render( error}> - + ) expect(mockImport).toHaveBeenCalledTimes(2) await act(() => vi.advanceTimersByTimeAsync(100)) - expect(screen.getByText('Component from /test-component')).toBeInTheDocument() + expect(screen.getByText('Component from /test-component x:test')).toBeInTheDocument() expect(defaultOnError).toHaveBeenCalledTimes(1) expect(individualOnError).toHaveBeenCalledTimes(1) diff --git a/packages/react/src/lazy.ts b/packages/react/src/lazy.ts index b8f814b88..ccb0c0c58 100644 --- a/packages/react/src/lazy.ts +++ b/packages/react/src/lazy.ts @@ -1,9 +1,8 @@ import { type ComponentType, type LazyExoticComponent, lazy as originalLazy } from 'react' -import { noop } from './utils/noop' -interface LazyOptions { - onSuccess?: ({ load }: { load: () => Promise }) => void - onError?: ({ error, load }: { error: unknown; load: () => Promise }) => undefined +interface LazyOptions> { + onSuccess?: ({ load }: { load: () => Promise<{ default: TComponentType }> }) => void + onError?: ({ error, load }: { error: unknown; load: () => Promise<{ default: TComponentType }> }) => void } /** @@ -11,8 +10,6 @@ interface LazyOptions { * * The default `lazy` export is equivalent to `createLazy({})`. * - * @experimental This is experimental feature. - * * @description * The created lazy function will execute individual callbacks first, then default callbacks. * For onSuccess: individual onSuccess → default onSuccess @@ -42,38 +39,37 @@ interface LazyOptions { * ``` */ export const createLazy = - (defaultOptions: LazyOptions) => - >( - load: () => Promise<{ default: T }>, - options?: LazyOptions - ): LazyExoticComponent & { - load: () => Promise + (defaultOptions: LazyOptions>) => + >( + load: () => Promise<{ default: TComponentType }>, + options?: LazyOptions + ): LazyExoticComponent & { + load: () => Promise<{ default: TComponentType }> } => { - const composedOnSuccess = ({ load }: { load: () => Promise }) => { + const composedOnSuccess = () => { options?.onSuccess?.({ load }) defaultOptions.onSuccess?.({ load }) } - const composedOnError = ({ error, load }: { error: unknown; load: () => Promise }) => { + const composedOnError = (error: unknown) => { options?.onError?.({ error, load }) defaultOptions.onError?.({ error, load }) } - const loadNoReturn = () => load().then(noop) return Object.assign( originalLazy(() => load().then( (loaded) => { - composedOnSuccess({ load: loadNoReturn }) + composedOnSuccess() return loaded }, (error: unknown) => { - composedOnError({ error: error, load: loadNoReturn }) + composedOnError(error) throw error } ) ), - { load: loadNoReturn } + { load } ) } @@ -82,8 +78,6 @@ export const createLazy = * * This is equivalent to `createLazy({})` - a lazy function with no default options. * - * @experimental This is experimental feature. - * * @example * ```tsx * import { lazy, Suspense } from '@suspensive/react' @@ -121,7 +115,7 @@ export const createLazy = * ``` * * @returns A lazy component with additional `load` method for preloading - * @property {() => Promise} load - Preloads the component without rendering it. Useful for prefetching components in the background. + * @property load - Preloads the component without rendering it. Useful for prefetching components in the background. */ export const lazy = createLazy({}) @@ -131,7 +125,7 @@ interface ReloadOnErrorStorage { removeItem: (key: string) => void } -interface ReloadOnErrorOptions extends LazyOptions { +interface ReloadOnErrorOptions extends LazyOptions> { /** * The number of times to retry the loading of the component. \ * If `true`, the component will be retried indefinitely. @@ -165,8 +159,6 @@ interface ReloadOnErrorOptions extends LazyOptions { /** * Options for reloading page if the component fails to load. * - * @experimental This is experimental feature. - * * @example * ```tsx * import { createLazy, reloadOnError } from '@suspensive/react' @@ -181,7 +173,7 @@ export const reloadOnError = ({ storage, reload, ...options -}: ReloadOnErrorOptions): LazyOptions => { +}: ReloadOnErrorOptions): LazyOptions> => { const reloadStorage = (() => { if (storage) return storage if (typeof window !== 'undefined' && 'sessionStorage' in window) return window.sessionStorage @@ -209,8 +201,11 @@ export const reloadOnError = ({ const storedValue = reloadStorage.getItem(storageKey) if (storedValue) { const reloadCount = parseInt(storedValue, 10) - if (Number.isNaN(reloadCount)) reloadStorage.removeItem(storageKey) - currentRetryCount = reloadCount + if (Number.isNaN(reloadCount)) { + reloadStorage.removeItem(storageKey) + } else { + currentRetryCount = reloadCount + } } }