Skip to content

fix: Apply forwardRef to ImpressionArea#241

Merged
seungrodotlee merged 1 commit into
mainfrom
fix/impression-area-forward-ref
May 21, 2025
Merged

fix: Apply forwardRef to ImpressionArea#241
seungrodotlee merged 1 commit into
mainfrom
fix/impression-area-forward-ref

Conversation

@jungpaeng
Copy link
Copy Markdown
Member

Overview

In React versions below 18, it is not possible to pass a ref without using forwardRef.

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?

Copilot AI review requested due to automatic review settings May 21, 2025 06:09
Copy link
Copy Markdown

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 pull request applies a fix to the ImpressionArea component by wrapping it in forwardRef to support older versions of React that require it for ref forwarding.

  • Updated the ImpressionArea implementation to use forwardRef with an arrow function.
  • Ensured the component's display name is set for easier debugging.

export const ImpressionArea = forwardRef(
<T extends ElementType = 'div'>(
{ as, rootMargin, areaThreshold, timeThreshold, onImpressionStart, onImpressionEnd, ...props }: Props<T>,
ref: React.Ref<Element>
Copy link

Copilot AI May 21, 2025

Choose a reason for hiding this comment

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

Consider using React.Ref instead of React.Ref for improved type safety, since the component's ref is expected to reference an HTMLElement.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Size Change: +230 B (+0.43%)

Total Size: 54.1 kB

Filename Size Change
./dist/components/ImpressionArea/index.cjs 2.23 kB +53 B (+2.44%)
./dist/index.cjs 6.5 kB +58 B (+0.9%)
./esm/components/ImpressionArea/index.js 1.84 kB +55 B (+3.09%)
./esm/index.js 5.99 kB +64 B (+1.08%)
ℹ️ View Unchanged
Filename Size
./dist/components/Separated/index.cjs 708 B
./dist/components/SwitchCase/index.cjs 580 B
./dist/hooks/useAsyncEffect/index.cjs 603 B
./dist/hooks/useBooleanState/index.cjs 653 B
./dist/hooks/useCallbackOncePerRender/index.cjs 724 B
./dist/hooks/useControlledState/index.cjs 784 B
./dist/hooks/useCounter/index.cjs 841 B
./dist/hooks/useDebounce/index.cjs 1.13 kB
./dist/hooks/useDebouncedCallback/index.cjs 1.25 kB
./dist/hooks/useDoubleClick/index.cjs 854 B
./dist/hooks/useImpressionRef/index.cjs 1.95 kB
./dist/hooks/useInputState/index.cjs 669 B
./dist/hooks/useIntersectionObserver/index.cjs 945 B
./dist/hooks/useInterval/index.cjs 808 B
./dist/hooks/useIsomorphicLayoutEffect/index.cjs 580 B
./dist/hooks/useLoading/index.cjs 774 B
./dist/hooks/useOutsideClickEffect/index.cjs 843 B
./dist/hooks/usePreservedCallback/index.cjs 623 B
./dist/hooks/usePreservedReference/index.cjs 672 B
./dist/hooks/usePrevious/index.cjs 675 B
./dist/hooks/useRefEffect/index.cjs 804 B
./dist/hooks/useStorageState/index.cjs 1.66 kB
./dist/hooks/useThrottle/index.cjs 1.31 kB
./dist/hooks/useTimeout/index.cjs 711 B
./dist/hooks/useToggle/index.cjs 578 B
./dist/hooks/useVisibilityEvent/index.cjs 691 B
./dist/utils/buildContext/index.cjs 870 B
./dist/utils/mergeProps/index.cjs 779 B
./dist/utils/mergeRefs/index.cjs 586 B
./esm/components/Separated/index.js 297 B
./esm/components/SwitchCase/index.js 173 B
./esm/hooks/useAsyncEffect/index.js 184 B
./esm/hooks/useBooleanState/index.js 239 B
./esm/hooks/useCallbackOncePerRender/index.js 318 B
./esm/hooks/useControlledState/index.js 388 B
./esm/hooks/useCounter/index.js 440 B
./esm/hooks/useDebounce/index.js 715 B
./esm/hooks/useDebouncedCallback/index.js 860 B
./esm/hooks/useDoubleClick/index.js 458 B
./esm/hooks/useImpressionRef/index.js 1.56 kB
./esm/hooks/useInputState/index.js 258 B
./esm/hooks/useIntersectionObserver/index.js 540 B
./esm/hooks/useInterval/index.js 407 B
./esm/hooks/useIsomorphicLayoutEffect/index.js 161 B
./esm/hooks/useLoading/index.js 365 B
./esm/hooks/useOutsideClickEffect/index.js 439 B
./esm/hooks/usePreservedCallback/index.js 205 B
./esm/hooks/usePreservedReference/index.js 264 B
./esm/hooks/usePrevious/index.js 259 B
./esm/hooks/useRefEffect/index.js 407 B
./esm/hooks/useStorageState/index.js 1.29 kB
./esm/hooks/useThrottle/index.js 914 B
./esm/hooks/useTimeout/index.js 305 B
./esm/hooks/useToggle/index.js 160 B
./esm/hooks/useVisibilityEvent/index.js 285 B
./esm/utils/buildContext/index.js 455 B
./esm/utils/mergeProps/index.js 393 B
./esm/utils/mergeRefs/index.js 182 B

compressed-size-action

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (c76f1fe) to head (fcafbc8).
Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #241   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           33        33           
  Lines          839       836    -3     
  Branches       254       254           
=========================================
- Hits           839       836    -3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@seungrodotlee seungrodotlee left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@seungrodotlee seungrodotlee merged commit bea638d into main May 21, 2025
8 checks passed
@seungrodotlee seungrodotlee deleted the fix/impression-area-forward-ref branch May 21, 2025 13:49
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.

4 participants