Skip to content

refactor(useRefEffect): Improve useRefEffect hook by adding generic types in docs and strict null check in implementation #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor(useRefEffect): Improve useRefEffect hook by adding generic types in docs and strict null check in implementation #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f910eb2
docs: add generic type
wo-o29 Sep 13, 2025
a8ecfc4
refactor: strict null check to effect callback
wo-o29 Sep 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/hooks/useRefEffect/ko/useRefEffect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
## 인터페이스

```ts
function useRefEffect(
callback: (element: Element) => CleanupCallback | void,
function useRefEffect<E extends HTMLElement>(
callback: (element: E) => CleanupCallback | void,
deps: DependencyList
): (element: Element | null) => void;
): (element: E | null) => void;
```

### 파라미터

<Interface
required
name="callback"
type="(element: Element) => CleanupCallback | void"
type="(element: E) => CleanupCallback | void"
description="요소가 설정될 때 실행되는 콜백 함수예요. 이 함수는 정리 함수를 반환할 수 있어요."
/>

Expand All @@ -31,7 +31,7 @@ function useRefEffect(

<Interface
name=""
type="(element: Element | null) => void"
type="(element: E | null) => void"
description="요소를 설정하는 함수예요. 이 함수를 <code>ref</code> 속성에 전달하면, 요소가 변경될 때마다 <code>callback</code>이 호출돼요."
/>

Expand Down
10 changes: 5 additions & 5 deletions src/hooks/useRefEffect/useRefEffect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
## Interface

```ts
function useRefEffect(
callback: (element: Element) => CleanupCallback | void,
function useRefEffect<E extends HTMLElement>(
callback: (element: E) => CleanupCallback | void,
deps: DependencyList
): (element: Element | null) => void;
): (element: E | null) => void;
```

### Parameters

<Interface
required
name="callback"
type="(element: Element) => CleanupCallback | void"
type="(element: E) => CleanupCallback | void"
description="A callback function that is executed when the element is set. This function can return a cleanup function."
/>

Expand All @@ -31,7 +31,7 @@ function useRefEffect(

<Interface
name=""
type="(element: Element | null) => void"
type="(element: E | null) => void"
description="function to set the element. Pass this function to the <code>ref</code> attribute, and the <code>callback</code> will be called whenever the element changes."
/>

Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useRefEffect/useRefEffect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function useRefEffect<Element extends HTMLElement = HTMLElement>(
cleanupCallbackRef.current();
cleanupCallbackRef.current = () => {};

if (element == null) {
if (element === null) {
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strict null check element === null will not catch undefined values, which could cause the callback to be executed with undefined. The original loose equality check == null was correct as it handles both null and undefined cases. This change could introduce bugs when the element is undefined.

Suggested change
if (element === null) {
if (element == null) {

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since instance is of type T | null as in React CallbackRef Type, I don't think we need to check for undefined. What do you think?

return;
}

Expand Down
Loading