From 483bf9b6f071a04a366bfaf8aaaddd2cf5d0b893 Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Tue, 11 Jun 2024 10:09:58 +0300 Subject: [PATCH] fix(useAfterFirstRender): hook is currently behaving the opposite than its purpose (#2168) --- packages/core/.storybook/preview.tsx | 3 +- packages/core/plop/hooks/index.js | 2 +- .../core/src/components/Counter/Counter.tsx | 2 +- .../core/src/hooks/{index.js => index.ts} | 0 .../__stories__/useAfterFirstRender.mdx | 2 +- .../useAfterFirstRender.stories.tsx | 46 ++++++++++++++++--- .../__tests__/useAfterFirstRender.test.tsx | 35 ++++++++++++++ .../src/hooks/useAfterFirstRender/index.ts | 11 +++-- .../components/LiveContent/LiveContent.tsx | 3 +- 9 files changed, 88 insertions(+), 16 deletions(-) rename packages/core/src/hooks/{index.js => index.ts} (100%) create mode 100644 packages/core/src/hooks/useAfterFirstRender/__tests__/useAfterFirstRender.test.tsx diff --git a/packages/core/.storybook/preview.tsx b/packages/core/.storybook/preview.tsx index 18976087e0..886922b4e8 100644 --- a/packages/core/.storybook/preview.tsx +++ b/packages/core/.storybook/preview.tsx @@ -1,6 +1,7 @@ import React from "react"; import * as VibeComponents from "../src/components"; import * as VibeComponentsNext from "../src/next"; +import * as VibeHooks from "../src/hooks"; import * as VibeIcons from "../src/components/Icon/Icons"; import { Preview } from "@storybook/react"; import isChromatic from "chromatic/isChromatic"; @@ -113,7 +114,7 @@ const preview: Preview = { }, playground: { storyId: "playground", - components: { ...VibeComponents, VibeIcons, VibeNext: VibeComponentsNext }, + components: { ...VibeComponents, VibeIcons, VibeNext: VibeComponentsNext, ...VibeHooks }, introCode, autocompletions: generateAutocompletion(reactDocgenOutput) } diff --git a/packages/core/plop/hooks/index.js b/packages/core/plop/hooks/index.js index 24cc949996..61e812a83c 100644 --- a/packages/core/plop/hooks/index.js +++ b/packages/core/plop/hooks/index.js @@ -37,7 +37,7 @@ module.exports = plop => { }, { type: "append", - path: "src/hooks/index.js", + path: "src/hooks/index.ts", pattern: /(\n$)/gm, separator: "", template: `export { default as {{camelCase hookName}} } from "./{{camelCase hookName}}";\n` diff --git a/packages/core/src/components/Counter/Counter.tsx b/packages/core/src/components/Counter/Counter.tsx index 3d675067ef..af2ccabe4e 100644 --- a/packages/core/src/components/Counter/Counter.tsx +++ b/packages/core/src/components/Counter/Counter.tsx @@ -97,7 +97,7 @@ const Counter: React.FC & { // Effects useEffect(() => { - if (!isAfterFirstRender.current) { + if (isAfterFirstRender.current) { setCountChangedAnimationActive(); } }, [count, isAfterFirstRender, setCountChangedAnimationActive]); diff --git a/packages/core/src/hooks/index.js b/packages/core/src/hooks/index.ts similarity index 100% rename from packages/core/src/hooks/index.js rename to packages/core/src/hooks/index.ts diff --git a/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.mdx b/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.mdx index d9278a7c3c..a00c68fa6e 100644 --- a/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.mdx +++ b/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.mdx @@ -1,4 +1,4 @@ -import { Canvas, Meta } from "@storybook/blocks"; +import { Meta } from "@storybook/blocks"; import { FunctionArgument, FunctionArguments, UsageGuidelines } from "vibe-storybook-components"; import * as UseAfterFirstRenderStories from "./useAfterFirstRender.stories"; diff --git a/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.stories.tsx b/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.stories.tsx index ecec1d5e8e..9a14f9c644 100644 --- a/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.stories.tsx +++ b/packages/core/src/hooks/useAfterFirstRender/__stories__/useAfterFirstRender.stories.tsx @@ -1,15 +1,47 @@ -import React from "react"; +import React, { useState } from "react"; import useAfterFirstRender from ".."; +import Button from "../../../components/Button/Button"; +import { Meta, StoryObj } from "@storybook/react"; +import Flex from "../../../components/Flex/Flex"; +import Heading from "../../../components/Heading/Heading"; + +type Story = StoryObj; export default { title: "Hooks/useAfterFirstRender" -}; +} satisfies Meta; -export const Overview = { +export const Overview: Story = { render: () => { - const isRendered = useAfterFirstRender(); - return
{isRendered ? "Rendered!" : "Not yet!"}
; - }, + const isAfterFirstRender = useAfterFirstRender(); + const [renderCount, setRenderCount] = useState(0); - name: "Overview" + const handleRerender = () => { + setRenderCount(prevCount => prevCount + 1); + }; + + return ( + <> + + {isAfterFirstRender.current ? "It is after the first render!" : "This is the first render!"} + +

Rerender count: {renderCount}

+ + + ); + }, + decorators: [ + Story => ( + + + + ) + ], + parameters: { + docs: { + liveEdit: { + scope: { Heading } + } + } + } }; diff --git a/packages/core/src/hooks/useAfterFirstRender/__tests__/useAfterFirstRender.test.tsx b/packages/core/src/hooks/useAfterFirstRender/__tests__/useAfterFirstRender.test.tsx new file mode 100644 index 0000000000..b009e11a59 --- /dev/null +++ b/packages/core/src/hooks/useAfterFirstRender/__tests__/useAfterFirstRender.test.tsx @@ -0,0 +1,35 @@ +import { renderHook } from "@testing-library/react-hooks"; +import useAfterFirstRender from "../"; + +describe("useAfterFirstRender", () => { + it("should initially set isAfterFirstRender to false", () => { + const { result } = renderHook(() => useAfterFirstRender()); + expect(result.current.current).toBeFalsy(); + }); + + describe("when re-renders", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("should set isAfterFirstRender to true after a delay", async () => { + const { result } = renderHook(() => useAfterFirstRender()); + jest.advanceTimersByTime(1); + expect(result.current.current).toBeTruthy(); + }); + + it("should maintain isAfterFirstRender as true on subsequent renders", () => { + const { result, rerender } = renderHook(() => useAfterFirstRender()); + + jest.advanceTimersByTime(1); + expect(result.current.current).toBeTruthy(); + + rerender(); + expect(result.current.current).toBeTruthy(); + }); + }); +}); diff --git a/packages/core/src/hooks/useAfterFirstRender/index.ts b/packages/core/src/hooks/useAfterFirstRender/index.ts index 47fcbfcf48..08e5f2e9d2 100644 --- a/packages/core/src/hooks/useAfterFirstRender/index.ts +++ b/packages/core/src/hooks/useAfterFirstRender/index.ts @@ -1,11 +1,14 @@ import { RefObject, useEffect, useRef } from "react"; export default function useAfterFirstRender(): RefObject { - const isAfterFirstRender = useRef(true); + const isAfterFirstRender = useRef(false); + useEffect(() => { - window.requestAnimationFrame(() => { - isAfterFirstRender.current = false; - }); + const timer = setTimeout(() => { + isAfterFirstRender.current = true; + }, 0); + return () => clearTimeout(timer); }, []); + return isAfterFirstRender; } diff --git a/packages/core/src/storybook/decorators/withLiveEdit/components/LiveContent/LiveContent.tsx b/packages/core/src/storybook/decorators/withLiveEdit/components/LiveContent/LiveContent.tsx index e54eae06f6..1ddeecc25b 100644 --- a/packages/core/src/storybook/decorators/withLiveEdit/components/LiveContent/LiveContent.tsx +++ b/packages/core/src/storybook/decorators/withLiveEdit/components/LiveContent/LiveContent.tsx @@ -5,10 +5,11 @@ import useApplyDecorators from "../../hooks/useApplyDecorators"; import { LiveContentProps } from "./LiveContent.types"; import styles from "./LiveContent.module.scss"; import * as VibeComponents from "../../../../../components"; +import * as VibeHooks from "../../../../../hooks"; import * as VibeIcons from "../../../../../components/Icon/Icons"; import * as VibeComponentsNext from "../../../../../next"; -const vibeScope = { ...VibeComponents, VibeIcons, VibeNext: VibeComponentsNext }; +const vibeScope = { ...VibeComponents, VibeIcons, VibeNext: VibeComponentsNext, ...VibeHooks }; const reactCommonHooksScope = { useState: React.useState, useEffect: React.useEffect,