Skip to content

fix(core): apply named function pattern to useEffect calls#347

Merged
zztnrudzz13 merged 6 commits intotoss:mainfrom
eunwoo-levi:fix/useeffect-named-function-pattern
Mar 18, 2026
Merged

fix(core): apply named function pattern to useEffect calls#347
zztnrudzz13 merged 6 commits intotoss:mainfrom
eunwoo-levi:fix/useeffect-named-function-pattern

Conversation

@eunwoo-levi
Copy link
Contributor

@eunwoo-levi eunwoo-levi commented Mar 12, 2026

Overview

Apply the named function pattern to useEffect callbacks across multiple hooks to comply with the project's coding standards.

Summary

CLAUDE.md explicitly requires named functions in useEffect:

Named functions in useEffect — Improves stack traces and readability

useEffect(function handleResize() { ... }, []);  // ✅
useEffect(() => { ... }, []);                     // ❌

Changes

  • usePreservedCallback — function syncCallbackRef()
  • useInterval — function runImmediateCallback(), function startInterval()
  • useDebounce — function cancelDebouncedOnUnmount()
  • useDebouncedCallback — function clearDebouncedOnUnmount()
  • useLoading — function trackMountedState()

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

Copy link
Collaborator

@zztnrudzz13 zztnrudzz13 left a comment

Choose a reason for hiding this comment

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

This is a great PR — we’re planning to merge it soon. Could you please add a changeset?

Copilot AI review requested due to automatic review settings March 17, 2026 14:01
@eunwoo-levi
Copy link
Contributor Author

@zztnrudzz13 Added a changeset! Thank you for the review :)

@eunwoo-levi eunwoo-levi requested a review from zztnrudzz13 March 17, 2026 14:04
zztnrudzz13
zztnrudzz13 previously approved these changes Mar 17, 2026
Copy link
Collaborator

@zztnrudzz13 zztnrudzz13 left a comment

Choose a reason for hiding this comment

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

Great! Thank you for contributing!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates several core hooks to use named function expressions for useEffect callbacks, aligning with the project standard documented in CLAUDE.md and improving stack traces/readability.

Changes:

  • Replaced anonymous arrow callbacks in useEffect with named functions across multiple hooks.
  • Preserved existing effect logic while improving debuggability.
  • Added a patch changeset documenting the behavioral-neutral internal refactor.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/core/src/hooks/usePreservedCallback/usePreservedCallback.ts Replaces useEffect arrow callback with a named function to improve stack traces.
packages/core/src/hooks/useLoading/useLoading.ts Uses a named function for the mounted-state tracking effect.
packages/core/src/hooks/useInterval/useInterval.ts Names the immediate-run and interval-start effects for clearer stack traces.
packages/core/src/hooks/useDebouncedCallback/useDebouncedCallback.ts Converts the unmount cleanup effect to a named function.
packages/core/src/hooks/useDebounce/useDebounce.ts Converts the unmount cancel effect to a named function.
.changeset/pretty-candles-wink.md Adds a patch changeset entry describing the refactor.

@eunwoo-levi
Copy link
Contributor Author

eunwoo-levi commented Mar 18, 2026

@kimyouknow Rebased onto latest main after PR #345 was merged:) Thanks for the review!

@zztnrudzz13 zztnrudzz13 merged commit 9a358e2 into toss:main Mar 18, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants