From 776caa6182826ea6735c8faed8691ab39312f16a Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 6 Jul 2022 11:17:52 +0200 Subject: [PATCH 1/9] feat: rewrite internals to use setState for `ref` --- .storybook/main.js | 2 +- src/stories/Hooks.story.tsx | 20 ++++++++++-- src/useInView.tsx | 64 ++++++++++++++++++++++++------------- 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/.storybook/main.js b/.storybook/main.js index dd1404ae..4c2991e9 100644 --- a/.storybook/main.js +++ b/.storybook/main.js @@ -30,7 +30,7 @@ module.exports = { async viteFinal(config) { // The build fails to correctly minify the `ansi-to-html` module with esbuild, so we fallback to Terser. // It's a package used by "Storybook" for the Webpreview, so it's interesting why it fails. - config.build.minify = 'terser'; + if (config.build) config.build.minify = 'terser'; if (config.optimizeDeps) { config.optimizeDeps.include = [ diff --git a/src/stories/Hooks.story.tsx b/src/stories/Hooks.story.tsx index 33f9108f..fd116d75 100644 --- a/src/stories/Hooks.story.tsx +++ b/src/stories/Hooks.story.tsx @@ -28,6 +28,7 @@ type Props = IntersectionOptions & { style?: CSSProperties; className?: string; lazy?: boolean; + inlineRef?: boolean; }; const story: Meta = { @@ -88,7 +89,13 @@ const story: Meta = { export default story; -const Template: Story = ({ style, className, lazy, ...rest }) => { +const Template: Story = ({ + style, + className, + lazy, + inlineRef, + ...rest +}) => { const { options, error } = useValidateOptions(rest); const { ref, inView, entry } = useInView(!error ? options : {}); const [isLoading, setIsLoading] = useState(lazy); @@ -109,7 +116,11 @@ const Template: Story = ({ style, className, lazy, ...rest }) => { return ( - + ref(node) : ref} + inView={inView} + style={style} + > @@ -127,6 +138,11 @@ LazyHookRendering.args = { lazy: true, }; +export const InlineRef = Template.bind({}); +LazyHookRendering.args = { + inlineRef: true, +}; + export const StartInView = Template.bind({}); StartInView.args = { initialInView: true, diff --git a/src/useInView.tsx b/src/useInView.tsx index c8dd8689..85d8e12a 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -45,29 +45,27 @@ export function useInView({ fallbackInView, onChange, }: IntersectionOptions = {}): InViewHookResponse { + const [ref, setRef] = React.useState(null); const unobserve = React.useRef(); const callback = React.useRef(); const [state, setState] = React.useState({ inView: !!initialInView, + entry: undefined, }); + // Store the onChange callback in a `ref`, so we can access the latest instance inside the `useCallback`. callback.current = onChange; - const setRef = React.useCallback( - (node: Element | null) => { - if (unobserve.current !== undefined) { - unobserve.current(); - unobserve.current = undefined; - } - - // Skip creating the observer - if (skip) return; - - if (node) { + React.useEffect( + () => { + if (ref && !skip) { unobserve.current = observe( - node, + ref, (inView, entry) => { - setState({ inView, entry }); + setState({ + inView, + entry, + }); if (callback.current) callback.current(inView, entry); if (entry.isIntersecting && triggerOnce && unobserve.current) { @@ -87,12 +85,31 @@ export function useInView({ }, fallbackInView, ); + } else if (!ref && !triggerOnce && !skip) { + // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) + // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView + setState((prevState) => { + if (prevState.entry) { + return { + inView: !!initialInView, + entry: undefined, + }; + } + return prevState; + }); } + + return () => { + if (unobserve.current) { + unobserve.current(); + unobserve.current = undefined; + } + }; }, // We break the rule here, because we aren't including the actual `threshold` variable // eslint-disable-next-line react-hooks/exhaustive-deps [ - // If the threshold is an array, convert it to a string so it won't change between renders. + // If the threshold is an array, convert it to a string, so it won't change between renders. // eslint-disable-next-line react-hooks/exhaustive-deps Array.isArray(threshold) ? threshold.toString() : threshold, root, @@ -102,19 +119,20 @@ export function useInView({ trackVisibility, fallbackInView, delay, + ref, ], ); /* eslint-disable-next-line */ - React.useEffect(() => { - if (!unobserve.current && state.entry && !triggerOnce && !skip) { - // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) - // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView - setState({ - inView: !!initialInView, - }); - } - }); + // React.useEffect(() => { + // if (!unobserve.current && state.entry && !triggerOnce && !skip) { + // // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) + // // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView + // setState({ + // inView: !!initialInView, + // }); + // } + // }); const result = [setRef, state.inView, state.entry] as InViewHookResponse; From a3e2be51062af784f87d89ba7950ab83ea319b05 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 6 Jul 2022 21:36:17 +0200 Subject: [PATCH 2/9] fix: optimize re-renders in useInView --- README.md | 10 +++--- src/useInView.tsx | 89 +++++++++++++++++++++-------------------------- 2 files changed, 45 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 1db3e8ba..628ad3a4 100644 --- a/README.md +++ b/README.md @@ -164,9 +164,9 @@ Provide these as the options argument in the `useInView` hook or as props on the The **``** component also accepts the following props: -| Name | Type | Default | Description | -| ------------ | ---------------------------------------------------- | ----------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **as** | `string` | `'div'` | Render the wrapping element as this element. Defaults to `div`. | +| Name | Type | Default | Description | +| ------------ | ---------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **as** | `string` | `'div'` | Render the wrapping element as this element. Defaults to `div`. | | **children** | `({ref, inView, entry}) => ReactNode` or `ReactNode` | `undefined` | Children expects a function that receives an object containing the `inView` boolean and a `ref` that should be assigned to the element root. Alternatively pass a plain child, to have the `` deal with the wrapping element. You will also get the `IntersectionObserverEntry` as `entry`, giving you more details. | ### Intersection Observer v2 🧪 @@ -217,9 +217,9 @@ import { useInView } from 'react-intersection-observer'; function Component(props) { const ref = useRef(); - const [inViewRef, inView] = useInView(); + const { ref: inViewRef, inView } = useInView(); - // Use `useCallback` so we don't recreate the function on each render - Could result in infinite loop + // Use `useCallback` so we don't recreate the function on each render const setRefs = useCallback( (node) => { // Ref's from useRef needs to have the node assigned to `current` diff --git a/src/useInView.tsx b/src/useInView.tsx index 85d8e12a..cf9a0a28 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -58,46 +58,35 @@ export function useInView({ React.useEffect( () => { - if (ref && !skip) { - unobserve.current = observe( - ref, - (inView, entry) => { - setState({ - inView, - entry, - }); - if (callback.current) callback.current(inView, entry); + // Ensure we have node ref, and that we shouldn't skip observing + if (skip || !ref) return; - if (entry.isIntersecting && triggerOnce && unobserve.current) { - // If it should only trigger once, unobserve the element after it's inView - unobserve.current(); - unobserve.current = undefined; - } - }, - { - root, - rootMargin, - threshold, - // @ts-ignore - trackVisibility, - // @ts-ignore - delay, - }, - fallbackInView, - ); - } else if (!ref && !triggerOnce && !skip) { - // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) - // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView - setState((prevState) => { - if (prevState.entry) { - return { - inView: !!initialInView, - entry: undefined, - }; + unobserve.current = observe( + ref, + (inView, entry) => { + setState({ + inView, + entry, + }); + if (callback.current) callback.current(inView, entry); + + if (entry.isIntersecting && triggerOnce && unobserve.current) { + // If it should only trigger once, unobserve the element after it's inView + unobserve.current(); + unobserve.current = undefined; } - return prevState; - }); - } + }, + { + root, + rootMargin, + threshold, + // @ts-ignore + trackVisibility, + // @ts-ignore + delay, + }, + fallbackInView, + ); return () => { if (unobserve.current) { @@ -112,6 +101,7 @@ export function useInView({ // If the threshold is an array, convert it to a string, so it won't change between renders. // eslint-disable-next-line react-hooks/exhaustive-deps Array.isArray(threshold) ? threshold.toString() : threshold, + ref, root, rootMargin, triggerOnce, @@ -119,20 +109,21 @@ export function useInView({ trackVisibility, fallbackInView, delay, - ref, ], ); - /* eslint-disable-next-line */ - // React.useEffect(() => { - // if (!unobserve.current && state.entry && !triggerOnce && !skip) { - // // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) - // // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView - // setState({ - // inView: !!initialInView, - // }); - // } - // }); + const entryTarget = state.entry?.target; + + React.useEffect(() => { + if (!ref && entryTarget && !triggerOnce && !skip) { + // If we don't have a node ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) + // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView + setState({ + inView: !!initialInView, + entry: undefined, + }); + } + }, [ref, entryTarget, triggerOnce, skip, initialInView]); const result = [setRef, state.inView, state.entry] as InViewHookResponse; From 8f4badd72e10c23f13828022ae454949702887c0 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 7 Jul 2022 09:00:57 +0200 Subject: [PATCH 3/9] docs: improve comment --- src/useInView.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/useInView.tsx b/src/useInView.tsx index cf9a0a28..26a17259 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -53,7 +53,8 @@ export function useInView({ entry: undefined, }); - // Store the onChange callback in a `ref`, so we can access the latest instance inside the `useCallback`. + // Store the onChange callback in a `ref`, so we can access the latest instance + // inside the `useEffect`, but without triggering a rerender. callback.current = onChange; React.useEffect( From 0f52b0ad68e4b5b956c2902b2bf90721d9e7c606 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 7 Jul 2022 09:09:22 +0200 Subject: [PATCH 4/9] chore: fix eslint file --- .eslintrc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 7cbe9d0e..2385fdd2 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -91,7 +91,7 @@ module.exports = { 'no-this-before-super': 'warn', 'no-throw-literal': 'warn', 'no-unexpected-multiline': 'warn', - 'no-unreachable': 'wa' + 'rn', + 'no-unreachable': 'warn', 'no-unused-expressions': [ 'error', { From 0ae3f1c9bebde3456b60c67a93aaceb9e2f9adb6 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Sat, 9 Jul 2022 23:52:10 +0200 Subject: [PATCH 5/9] test: fix hook story --- src/stories/Hooks.story.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stories/Hooks.story.tsx b/src/stories/Hooks.story.tsx index fd116d75..639fa2ee 100644 --- a/src/stories/Hooks.story.tsx +++ b/src/stories/Hooks.story.tsx @@ -139,7 +139,7 @@ LazyHookRendering.args = { }; export const InlineRef = Template.bind({}); -LazyHookRendering.args = { +InlineRef.args = { inlineRef: true, }; From f597a4f1c14da5e9fc27d0136f44ff8930a893bc Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Sun, 10 Jul 2022 00:07:10 +0200 Subject: [PATCH 6/9] refactor: revert to useCallback for ref Change it so cleanup is now done inside a `useEffect`. This allows the `setRef` to be called multiple times before the hook triggers the cleanup, ensuring the current `ref` is settled. --- src/useInView.tsx | 50 ++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/useInView.tsx b/src/useInView.tsx index 26a17259..5dd0a57f 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -45,7 +45,7 @@ export function useInView({ fallbackInView, onChange, }: IntersectionOptions = {}): InViewHookResponse { - const [ref, setRef] = React.useState(null); + const ref = React.useRef(null); const unobserve = React.useRef(); const callback = React.useRef(); const [state, setState] = React.useState({ @@ -54,21 +54,28 @@ export function useInView({ }); // Store the onChange callback in a `ref`, so we can access the latest instance - // inside the `useEffect`, but without triggering a rerender. + // inside the `useCallback`, but without triggering a rerender. callback.current = onChange; - React.useEffect( - () => { + const setRef = React.useCallback( + (node: Element) => { + ref.current = node; // Ensure we have node ref, and that we shouldn't skip observing - if (skip || !ref) return; + if (skip || !node) return; + // Store a reference the current unobserve function, so we can destroy it later + const previousObserver = unobserve.current; + + // Create a new IntersectionObserver, and store the `unobserve` function. unobserve.current = observe( - ref, + node, (inView, entry) => { setState({ inView, entry, }); + + // Trigger any onChange callback function if (callback.current) callback.current(inView, entry); if (entry.isIntersecting && triggerOnce && unobserve.current) { @@ -89,12 +96,11 @@ export function useInView({ fallbackInView, ); - return () => { - if (unobserve.current) { - unobserve.current(); - unobserve.current = undefined; - } - }; + if (previousObserver) { + // Was already observing a node - Make sure we destroy the previous observer. + // Do it after we create the new one, so the IntersectionObserver instance can be reused. + previousObserver(); + } }, // We break the rule here, because we aren't including the actual `threshold` variable // eslint-disable-next-line react-hooks/exhaustive-deps @@ -102,7 +108,6 @@ export function useInView({ // If the threshold is an array, convert it to a string, so it won't change between renders. // eslint-disable-next-line react-hooks/exhaustive-deps Array.isArray(threshold) ? threshold.toString() : threshold, - ref, root, rootMargin, triggerOnce, @@ -113,18 +118,27 @@ export function useInView({ ], ); - const entryTarget = state.entry?.target; - + // We break the rule here, since we want to ensure we check the `ref` instances on every render. + // eslint-disable-next-line react-hooks/exhaustive-deps React.useEffect(() => { - if (!ref && entryTarget && !triggerOnce && !skip) { + if (!ref.current && state.entry?.target && !triggerOnce && !skip) { // If we don't have a node ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) - // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView + // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView. setState({ inView: !!initialInView, entry: undefined, }); } - }, [ref, entryTarget, triggerOnce, skip, initialInView]); + + return () => { + if (!ref.current && unobserve.current) { + // We no longer have a valid ref. Destroy the observer + unobserve.current(); + unobserve.current = undefined; + ref.current = null; + } + }; + }); const result = [setRef, state.inView, state.entry] as InViewHookResponse; From 79b4b6448e264a4bd712862b3f5ad4c02ae23d2d Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Sun, 10 Jul 2022 19:31:15 +0200 Subject: [PATCH 7/9] Revert "refactor: revert to useCallback for ref" This reverts commit f597a4f1c14da5e9fc27d0136f44ff8930a893bc. --- src/useInView.tsx | 50 +++++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/src/useInView.tsx b/src/useInView.tsx index 5dd0a57f..26a17259 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -45,7 +45,7 @@ export function useInView({ fallbackInView, onChange, }: IntersectionOptions = {}): InViewHookResponse { - const ref = React.useRef(null); + const [ref, setRef] = React.useState(null); const unobserve = React.useRef(); const callback = React.useRef(); const [state, setState] = React.useState({ @@ -54,28 +54,21 @@ export function useInView({ }); // Store the onChange callback in a `ref`, so we can access the latest instance - // inside the `useCallback`, but without triggering a rerender. + // inside the `useEffect`, but without triggering a rerender. callback.current = onChange; - const setRef = React.useCallback( - (node: Element) => { - ref.current = node; + React.useEffect( + () => { // Ensure we have node ref, and that we shouldn't skip observing - if (skip || !node) return; + if (skip || !ref) return; - // Store a reference the current unobserve function, so we can destroy it later - const previousObserver = unobserve.current; - - // Create a new IntersectionObserver, and store the `unobserve` function. unobserve.current = observe( - node, + ref, (inView, entry) => { setState({ inView, entry, }); - - // Trigger any onChange callback function if (callback.current) callback.current(inView, entry); if (entry.isIntersecting && triggerOnce && unobserve.current) { @@ -96,11 +89,12 @@ export function useInView({ fallbackInView, ); - if (previousObserver) { - // Was already observing a node - Make sure we destroy the previous observer. - // Do it after we create the new one, so the IntersectionObserver instance can be reused. - previousObserver(); - } + return () => { + if (unobserve.current) { + unobserve.current(); + unobserve.current = undefined; + } + }; }, // We break the rule here, because we aren't including the actual `threshold` variable // eslint-disable-next-line react-hooks/exhaustive-deps @@ -108,6 +102,7 @@ export function useInView({ // If the threshold is an array, convert it to a string, so it won't change between renders. // eslint-disable-next-line react-hooks/exhaustive-deps Array.isArray(threshold) ? threshold.toString() : threshold, + ref, root, rootMargin, triggerOnce, @@ -118,27 +113,18 @@ export function useInView({ ], ); - // We break the rule here, since we want to ensure we check the `ref` instances on every render. - // eslint-disable-next-line react-hooks/exhaustive-deps + const entryTarget = state.entry?.target; + React.useEffect(() => { - if (!ref.current && state.entry?.target && !triggerOnce && !skip) { + if (!ref && entryTarget && !triggerOnce && !skip) { // If we don't have a node ref, then reset the state (unless the hook is set to only `triggerOnce` or `skip`) - // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView. + // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView setState({ inView: !!initialInView, entry: undefined, }); } - - return () => { - if (!ref.current && unobserve.current) { - // We no longer have a valid ref. Destroy the observer - unobserve.current(); - unobserve.current = undefined; - ref.current = null; - } - }; - }); + }, [ref, entryTarget, triggerOnce, skip, initialInView]); const result = [setRef, state.inView, state.entry] as InViewHookResponse; From c84989ecb08dc855b31842b728356689f8fe3973 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Sun, 10 Jul 2022 19:47:09 +0200 Subject: [PATCH 8/9] refactor: use action when `onChange` is called --- src/stories/Hooks.story.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/stories/Hooks.story.tsx b/src/stories/Hooks.story.tsx index 639fa2ee..5f90754b 100644 --- a/src/stories/Hooks.story.tsx +++ b/src/stories/Hooks.story.tsx @@ -1,4 +1,3 @@ -import { action } from '@storybook/addon-actions'; import { Meta, Story } from '@storybook/react'; import { IntersectionOptions, InView, useInView } from '../index'; import { motion } from 'framer-motion'; @@ -80,6 +79,7 @@ const story: Meta = { table: { disable: true, }, + action: 'InView', }, }, args: { @@ -96,10 +96,12 @@ const Template: Story = ({ inlineRef, ...rest }) => { + // const onChange: IntersectionOptions['onChange'] = (inView, entry) => { + // action('InView')(inView, entry); + // } const { options, error } = useValidateOptions(rest); - const { ref, inView, entry } = useInView(!error ? options : {}); + const { ref, inView } = useInView(!error ? { ...options } : {}); const [isLoading, setIsLoading] = useState(lazy); - action('InView')(inView, entry); useEffect(() => { if (isLoading) setIsLoading(false); From b6cecf6e2e07022a1a415b6cbd1681cef4896bd6 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 22 Jul 2022 16:00:24 +0200 Subject: [PATCH 9/9] refactor: get rid of unobserve ref The flow is now contained in the useEffect --- src/useInView.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/useInView.tsx b/src/useInView.tsx index 26a17259..a7a89a04 100644 --- a/src/useInView.tsx +++ b/src/useInView.tsx @@ -46,7 +46,6 @@ export function useInView({ onChange, }: IntersectionOptions = {}): InViewHookResponse { const [ref, setRef] = React.useState(null); - const unobserve = React.useRef(); const callback = React.useRef(); const [state, setState] = React.useState({ inView: !!initialInView, @@ -62,7 +61,7 @@ export function useInView({ // Ensure we have node ref, and that we shouldn't skip observing if (skip || !ref) return; - unobserve.current = observe( + let unobserve: (() => void) | undefined = observe( ref, (inView, entry) => { setState({ @@ -71,10 +70,10 @@ export function useInView({ }); if (callback.current) callback.current(inView, entry); - if (entry.isIntersecting && triggerOnce && unobserve.current) { + if (entry.isIntersecting && triggerOnce && unobserve) { // If it should only trigger once, unobserve the element after it's inView - unobserve.current(); - unobserve.current = undefined; + unobserve(); + unobserve = undefined; } }, { @@ -90,9 +89,8 @@ export function useInView({ ); return () => { - if (unobserve.current) { - unobserve.current(); - unobserve.current = undefined; + if (unobserve) { + unobserve(); } }; },