diff --git a/.size-limit.json b/.size-limit.json index e98c98c..c910d8b 100644 --- a/.size-limit.json +++ b/.size-limit.json @@ -1,32 +1,32 @@ [ { "path": "dist/bundle.esm.js", - "limit": "323 B", + "limit": "353 B", "gzip": true }, { "path": "dist/bundle.esm.js", - "limit": "243 B", + "limit": "266 B", "brotli": true }, { "path": "dist/bundle.cjs.js", - "limit": "313 B", + "limit": "341 B", "gzip": true }, { "path": "dist/bundle.cjs.js", - "limit": "233 B", + "limit": "258 B", "brotli": true }, { "path": "polyfilled.js", - "limit": "2644 B", + "limit": "2673 B", "gzip": true }, { "path": "polyfilled.js", - "limit": "2353 B", + "limit": "2381 B", "brotli": true } ] diff --git a/CHANGELOG.md b/CHANGELOG.md index 03dce22..1d7bd21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # CHANGELOG +## 6.1.0-alpha2 + +- ResizeObserver instances are no longer created unnecessarily when the onResize + callback changes. (Fixes #32) +- Written new tests in [react testing library](https://github.com/testing-library/react-testing-library). + ## 6.1.0-alpha1 - Rewrote the source in TypeScript. (Feedback is welcome.) diff --git a/README.md b/README.md index 3d12b9d..a2d76e3 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A React hook that allows you to use a ResizeObserver to measure an element's siz ## Highlights - Written in **TypeScript**. -- **Tiny**: 323 B (minified, gzipped) Monitored by [size-limit](https://github.com/ai/size-limit). +- **Tiny**: 353 B (minified, gzipped) Monitored by [size-limit](https://github.com/ai/size-limit). - Exposes an **onResize callback** if you need more control. - [Throttle / Debounce](#throttle--debounce) - Works with **SSR**. diff --git a/karma.conf.js b/karma.conf.js index 19eb75b..8b17e8f 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -1,14 +1,24 @@ -module.exports = function(config) { +module.exports = function (config) { const browsers = (process.env.KARMA_BROWSERS || "ChromeHeadless").split(","); + const testFilePattern = "tests/*.tsx"; + config.set({ basePath: ".", frameworks: ["jasmine"], - files: ["tests/dist/index.js"], + files: [ + { + pattern: testFilePattern, + watched: true, + }, + ], autoWatch: true, browsers, reporters: ["spec"], + preprocessors: { + [testFilePattern]: ["webpack", "sourcemap"], + }, // Max concurrency for SauceLabs OS plan concurrency: 5, @@ -16,8 +26,35 @@ module.exports = function(config) { client: { jasmine: { // Order of the tests matter, so don't randomise it - random: false - } - } + random: false, + }, + }, + + webpack: { + mode: "development", + devtool: "inline-source-map", + module: { + rules: [ + { + test: /\.(ts|tsx|js|jsx)$/, + exclude: /node_modules/, + use: { + loader: "babel-loader", + options: { + presets: [ + ["@babel/preset-env", { loose: true, modules: false }], + "@babel/preset-react", + "@babel/preset-typescript", + ], + plugins: [["@babel/transform-runtime", { useESModules: true }]], + }, + }, + }, + ], + }, + resolve: { + extensions: [".ts", ".tsx", ".js", ".jsx"], + }, + }, }); }; diff --git a/package.json b/package.json index fa68134..a285dc8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "use-resize-observer", - "version": "6.1.0-alpha.1", + "version": "6.1.0-alpha.2", "main": "dist/bundle.cjs.js", "module": "dist/bundle.esm.js", "types": "dist/index.d.ts", @@ -10,22 +10,19 @@ "author": "Viktor Hubert ", "license": "MIT", "scripts": { - "build": "run-s src:build test:build", - "watch": "KARMA_BROWSERS=Chrome run-p src:watch test:watch karma:watch", - "src:build": "rollup -c && tsc && cp dist/index.d.ts polyfilled.d.ts", + "build": "rollup -c && tsc && cp dist/index.d.ts polyfilled.d.ts", + "watch": "KARMA_BROWSERS=Chrome run-p 'src:watch' 'karma:watch'", "src:watch": "rollup -c -w", - "tsc": "tsc", - "size-limit": "size-limit", - "test": "run-s 'build' 'size-limit' 'tsc -p tests' 'test:chrome:headless' 'test:firefox:headless'", - "test:build": "parcel build tests/index.tsx --out-dir tests/dist", - "test:watch": "parcel watch tests/index.ts --out-dir tests/dist", + "check:size": "size-limit", + "check:tests": "tsc -p tests", + "test": "run-s 'build' 'check:size' 'check:tests' 'test:headless:*'", "test:chrome": "KARMA_BROWSERS=Chrome yarn karma:run", - "test:chrome:headless": "KARMA_BROWSERS=ChromeHeadless yarn karma:run", + "test:headless:chrome": "KARMA_BROWSERS=ChromeHeadless yarn karma:run", "test:firefox": "KARMA_BROWSERS=Firefox yarn karma:run", - "test:firefox:headless": "KARMA_BROWSERS=FirefoxHeadless yarn karma:run", + "test:headless:firefox": "KARMA_BROWSERS=FirefoxHeadless yarn karma:run", "karma:run": "karma start --singleRun", "karma:watch": "karma start", - "prepublish": "yarn src:build" + "prepublish": "yarn build" }, "husky": { "hooks": { @@ -43,25 +40,29 @@ }, "devDependencies": { "@babel/core": "^7.7.7", + "@babel/plugin-transform-runtime": "^7.9.0", "@babel/preset-env": "^7.7.7", + "@babel/preset-react": "^7.9.4", "@babel/preset-typescript": "^7.9.0", "@rollup/plugin-inject": "^4.0.1", "@size-limit/preset-small-lib": "^4.4.5", + "@testing-library/react": "^10.0.2", "@types/karma": "^5.0.0", "@types/karma-jasmine": "^3.1.0", "@types/react": "^16.9.34", "@types/react-dom": "^16.9.6", - "babel-regenerator-runtime": "^6.5.0", + "babel-loader": "^8.1.0", "delay": "^4.1.0", "husky": "^4.2.5", "karma": "^5.0.1", "karma-chrome-launcher": "^3.0.0", "karma-firefox-launcher": "^1.3.0", "karma-jasmine": "^3.1.1", + "karma-sourcemap-loader": "^0.3.7", "karma-spec-reporter": "^0.0.32", + "karma-webpack": "^4.0.2", "lint-staged": "^10.1.3", "npm-run-all": "^4.1.5", - "parcel-bundler": "^1.10.3", "prettier": "^2.0.4", "react": "^16.9.0", "react-dom": "^16.9.0", diff --git a/src/index.ts b/src/index.ts index 56cfd37..a7b602b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,11 @@ -import { useEffect, useState, useRef, useMemo, RefObject } from "react"; +import { + useEffect, + useState, + useRef, + useMemo, + RefObject, + MutableRefObject, +} from "react"; type ObservedSize = { width: number | undefined; @@ -29,8 +36,51 @@ function useResizeObserver( // @see https://reactjs.org/docs/hooks-rules.html#explanation const defaultRef = useRef(null); - const ref = opts.ref || defaultRef; + // Saving the callback as a ref. With this, I don't need to put onResize in the + // effect dep array, and just passing in an anonymous function without memoising + // will not reinstantiate the hook's ResizeObserver const onResize = opts.onResize; + const onResizeRef = useRef(undefined); + onResizeRef.current = onResize; + + // Using a single instance throughought the hook's lifetime + const resizeObserverRef = useRef() as MutableRefObject< + ResizeObserver + >; + if (!resizeObserverRef.current) { + resizeObserverRef.current = new ResizeObserver((entries) => { + if (!Array.isArray(entries)) { + return; + } + + // Since we only observe the one element, we don't need to loop over the + // array + if (!entries.length) { + return; + } + + const entry = entries[0]; + + // `Math.round` is in line with how CSS resolves sub-pixel values + const newWidth = Math.round(entry.contentRect.width); + const newHeight = Math.round(entry.contentRect.height); + if ( + previous.current.width !== newWidth || + previous.current.height !== newHeight + ) { + const newSize = { width: newWidth, height: newHeight }; + if (onResizeRef.current) { + onResizeRef.current(newSize); + } else { + previous.current.width = newWidth; + previous.current.height = newHeight; + setSize(newSize); + } + } + }); + } + + const ref = opts.ref || defaultRef; const [size, setSize] = useState<{ width?: number; height?: number; @@ -60,41 +110,11 @@ function useResizeObserver( } const element = ref.current; - const resizeObserver = new ResizeObserver((entries) => { - if (!Array.isArray(entries)) { - return; - } - - // Since we only observe the one element, we don't need to loop over the - // array - if (!entries.length) { - return; - } - - const entry = entries[0]; - - // `Math.round` is in line with how CSS resolves sub-pixel values - const newWidth = Math.round(entry.contentRect.width); - const newHeight = Math.round(entry.contentRect.height); - if ( - previous.current.width !== newWidth || - previous.current.height !== newHeight - ) { - const newSize = { width: newWidth, height: newHeight }; - if (onResize) { - onResize(newSize); - } else { - previous.current.width = newWidth; - previous.current.height = newHeight; - setSize(newSize); - } - } - }); - resizeObserver.observe(element); + resizeObserverRef.current.observe(element); - return () => resizeObserver.unobserve(element); - }, [ref, onResize]); + return () => resizeObserverRef.current.unobserve(element); + }, [ref]); return useMemo(() => ({ ref, width: size.width, height: size.height }), [ ref, diff --git a/tests/index.tsx b/tests/basic.tsx similarity index 89% rename from tests/index.tsx rename to tests/basic.tsx index befe189..172ce04 100644 --- a/tests/index.tsx +++ b/tests/basic.tsx @@ -1,9 +1,10 @@ +// todo test for SSR import React, { useEffect, useState, useRef, RefObject, - FunctionComponent + FunctionComponent, } from "react"; import useResizeObserver from "../"; import useResizeObserverPolyfilled from "../polyfilled"; @@ -16,13 +17,8 @@ import { ObservedSize, MultiHandlerResolverComponentProps, ComponentHandler, - HandlerResolverComponentProps + HandlerResolverComponentProps, } from "./utils"; -// Using the following to support async/await in tests. -// I'm intentionally not using babel/polyfill, as that would introduce polyfills -// the actual lib might not have, giving the false impression that something -// works while it might actually not, if you use the lib without babel-polyfill. -import "babel-regenerator-runtime"; it("should render with undefined sizes at first", async () => { const handler = await render(Observed); @@ -35,7 +31,7 @@ it("should render with custom defaults", async () => { {}, { defaultWidth: 24, - defaultHeight: 42 + defaultHeight: 42, } ); @@ -49,7 +45,7 @@ it("should follow size changes correctly with appropriate render count and witho setAndAssertSize, setSize, assertSize, - assertRenderCount + assertRenderCount, } = await render(Observed, { waitForFirstMeasurement: true }); // Default render + first measurement @@ -66,18 +62,18 @@ it("should follow size changes correctly with appropriate render count and witho it("should handle multiple instances", async () => { const Test: FunctionComponent = ({ - resolveHandler + resolveHandler, }) => { let resolveHandler1: HandlerReceiver = () => {}; let resolveHandler2: HandlerReceiver = () => {}; const handlersPromise = Promise.all([ new Promise( - resolve => (resolveHandler1 = resolve as HandlerReceiver) + (resolve) => (resolveHandler1 = resolve as HandlerReceiver) ), new Promise( - resolve => (resolveHandler2 = resolve as HandlerReceiver) - ) + (resolve) => (resolveHandler2 = resolve as HandlerReceiver) + ), ]); useEffect(() => { @@ -99,7 +95,7 @@ it("should handle multiple instances", async () => { await Promise.all([ handler1.setAndAssertSize({ width: 100, height: 200 }), - handler2.setAndAssertSize({ width: 300, height: 400 }) + handler2.setAndAssertSize({ width: 300, height: 400 }), ]); handler1.assertRenderCount(2); @@ -116,7 +112,7 @@ it("should handle multiple instances", async () => { it("should handle custom refs", async () => { const Test: FunctionComponent = ({ - resolveHandler + resolveHandler, }) => { const ref = useRef(null); const { width, height } = useResizeObserver({ ref }); @@ -160,7 +156,7 @@ it("should be able to reuse the same ref to measure different elements", async ( const { width, height } = useResizeObserver({ ref: stateRef }); const currentSizeRef = useRef({ width: undefined, - height: undefined + height: undefined, }); currentSizeRef.current.width = width; currentSizeRef.current.height = height; @@ -237,7 +233,7 @@ it("should keep the same response instance between renders if nothing changed", }; const Test: FunctionComponent = ({ - resolveHandler + resolveHandler, }) => { const previousResponseRef = useRef< | ({ @@ -282,12 +278,12 @@ it("should keep the same response instance between renders if nothing changed", it("should ignore invalid custom refs", async () => { const Test: FunctionComponent = ({ - resolveHandler + resolveHandler, }) => { // Passing in an invalid custom ref. // Same should be work if "null" or something similar gets passed in. const { width, height } = useResizeObserver({ - ref: {} as RefObject + ref: {} as RefObject, }); const currentSizeRef = useRef({} as ObservedSize); currentSizeRef.current.width = width; @@ -314,7 +310,7 @@ it("should ignore invalid custom refs", async () => { it("should work with the polyfilled version", async () => { const Test: FunctionComponent = ({ - resolveHandler + resolveHandler, }) => { const { ref, width, height } = useResizeObserverPolyfilled< HTMLDivElement @@ -373,7 +369,7 @@ it("should handle if the onResize handler changes properly with the correct rend }) => { const [onResizeHandler, setOnResizeHandler] = useState(() => () => {}); - changeOnResizeHandler = handler => setOnResizeHandler(() => handler); + changeOnResizeHandler = (handler) => setOnResizeHandler(() => handler); return ( { + cleanup(); +}); + +type ComponentController = { + setSize: (width: number, height: number) => void; + getRenderCount: () => number; + getWidth: () => number | undefined; + getHeight: () => number | undefined; + triggerRender: () => void; + switchToExplicitRef: () => void; +}; + +type TestProps = { + onResize?: (size: ObservedSize) => void; + resolveController: (controller: ComponentController) => void; +}; + +const Test = ({ onResize, resolveController }: TestProps) => { + const [, setRenderTrigger] = useState(false); + const [useExplicitRef, setUseExplicitRef] = useState(false); + const explicitRef = useRef(null); + const { ref, width = 0, height = 0 } = useResizeObserver({ + // We intentionally create a new function instance here if onResize is given. + // The hook is supposed to handle it and not recreate ResizeObserver instances on each render for example. + onResize: onResize ? (size: ObservedSize) => onResize(size) : undefined, + ...(useExplicitRef ? { ref: explicitRef } : {}), + }); + const controllerStateRef = useRef<{ renderCount: number } & ObservedSize>({ + renderCount: 0, + width: undefined, + height: undefined, + }); + + controllerStateRef.current.renderCount++; + controllerStateRef.current.width = width; + controllerStateRef.current.height = height; + + useEffect(() => { + resolveController({ + setSize: (width: number, height: number) => { + if (!ref.current) { + throw new Error(`Expected "ref.current" to be set.`); + } + + ref.current.style.width = `${width}px`; + ref.current.style.height = `${height}px`; + }, + getRenderCount: () => controllerStateRef.current.renderCount, + getWidth: () => controllerStateRef.current.width, + getHeight: () => controllerStateRef.current.height, + triggerRender: () => setRenderTrigger((value) => !value), + switchToExplicitRef: () => setUseExplicitRef(true), + }); + }, []); + + return
; +}; + +const awaitNextFrame = () => + new Promise((resolve) => setTimeout(resolve, 1000 / 60)); + +const renderTest = ( + props: Omit = {} +): Promise<[ComponentController, RenderResult]> => + new Promise((resolve) => { + const tools = render( + resolve([controller, tools])} + > + ); + }); + +it("should measure the right sizes", async () => { + const [controller] = await renderTest(); + + // Default response on the first render before an actual measurement took place + expect(controller.getWidth()).toBe(0); + expect(controller.getHeight()).toBe(0); + expect(controller.getRenderCount()).toBe(1); + + // Should react to component size changes. + controller.setSize(100, 200); + await awaitNextFrame(); + expect(controller.getWidth()).toBe(100); + expect(controller.getHeight()).toBe(200); + expect(controller.getRenderCount()).toBe(2); +}); + +describe("Resize Observer Instance Counting Block", () => { + let resizeObserverInstanceCount = 0; + let resizeObserverObserveCount = 0; + let resizeObserverUnobserveCount = 0; + const NativeResizeObserver = (window as any).ResizeObserver; + + beforeAll(() => { + (window as any).ResizeObserver = function PatchedResizeObserver( + cb: Function + ) { + resizeObserverInstanceCount++; + + const ro = new NativeResizeObserver(cb) as ResizeObserver; + + const mock = { + observe: (element: Element) => { + resizeObserverObserveCount++; + return ro.observe(element); + }, + unobserve: (element: Element) => { + resizeObserverUnobserveCount++; + return ro.unobserve(element); + }, + }; + + return mock; + }; + }); + + beforeEach(() => { + resizeObserverInstanceCount = 0; + resizeObserverObserveCount = 0; + resizeObserverUnobserveCount = 0; + }); + + afterAll(() => { + (window as any).ResizeObserver = NativeResizeObserver; + }); + + it("should use a single ResizeObserver instance even if the onResize callback is not memoised", async () => { + const [controller] = await renderTest({ + // This is only here so that each render passes a different callback + // instance through to the hook. + onResize: (size) => {}, + }); + + await awaitNextFrame(); + + controller.triggerRender(); + + await awaitNextFrame(); + + // Different onResize instances used to trigger the hook's internal useEffect, + // resulting in the hook using a new ResizeObserver instance on each render + // regardless of what triggered it. + expect(resizeObserverInstanceCount).toBe(1); + expect(resizeObserverObserveCount).toBe(1); + expect(resizeObserverUnobserveCount).toBe(0); + }); + + it("should not reinstantiate if the hook is the same but the observed element changes", async () => { + const [controller] = await renderTest(); + + // Default behaviour on initial mount with the explicit ref + expect(resizeObserverInstanceCount).toBe(1); + expect(resizeObserverObserveCount).toBe(1); + expect(resizeObserverUnobserveCount).toBe(0); + + // Switching to a different ref / element causes the hook to unobserve the + // previous element, and observe the new one, but it should not recreate the + // ResizeObserver instance. + controller.switchToExplicitRef(); + await awaitNextFrame(); + expect(resizeObserverInstanceCount).toBe(1); + expect(resizeObserverObserveCount).toBe(2); + expect(resizeObserverUnobserveCount).toBe(1); + }); +}); diff --git a/tests/tsconfig.json b/tests/tsconfig.json index 6737d4f..1ca89bc 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -9,5 +9,5 @@ "jsx": "react", "noEmit": true }, - "include": ["."] + "include": [".", "../src"] } diff --git a/tests/utils.tsx b/tests/utils/index.tsx similarity index 88% rename from tests/utils.tsx rename to tests/utils/index.tsx index 28a93c3..0257833 100644 --- a/tests/utils.tsx +++ b/tests/utils/index.tsx @@ -1,6 +1,6 @@ import React, { useEffect, useRef, RefObject, FunctionComponent } from "react"; import ReactDOM from "react-dom"; -import useResizeObserver from "../"; +import useResizeObserver from "../.."; import delay from "delay"; export type Size = { @@ -48,14 +48,14 @@ export function createComponentHandler(opts: { export function createComponentHandler({ currentSizeRef, measuredElementRef, - renderCountRef + renderCountRef, }: { currentSizeRef: RefObject; measuredElementRef?: RefObject; renderCountRef?: RefObject; }): BaseComponentHandler { let handler = { - assertSize: function({ width, height }: ObservedSize) { + assertSize: function ({ width, height }: ObservedSize) { if (currentSizeRef.current === null) { throw new Error(`currentSizeRef.current is not set.`); } @@ -63,9 +63,9 @@ export function createComponentHandler({ expect(currentSizeRef.current.width).toBe(width); expect(currentSizeRef.current.height).toBe(height); }, - assertDefaultSize: function() { + assertDefaultSize: function () { return this.assertSize({ width: undefined, height: undefined }); - } + }, } as ComponentHandler; if (measuredElementRef) { @@ -77,7 +77,7 @@ export function createComponentHandler({ measuredElementRef.current.style.width = `${width}px`; measuredElementRef.current.style.height = `${height}px`; }; - handler.setAndAssertSize = async size => { + handler.setAndAssertSize = async (size) => { handler.setSize(size); await delay(50); handler.assertSize(size); @@ -85,7 +85,7 @@ export function createComponentHandler({ } if (renderCountRef) { - handler.assertRenderCount = count => { + handler.assertRenderCount = (count) => { expect(renderCountRef.current).toBe(count); }; } @@ -101,20 +101,22 @@ export type MultiHandlerResolverComponentProps = { resolveHandler: MultiHandlerReceiver; }; -export const Observed: FunctionComponent void; -}> = ({ resolveHandler, defaultWidth, defaultHeight, onResize, ...props }) => { +export const Observed: FunctionComponent< + HandlerResolverComponentProps & { + defaultWidth?: number; + defaultHeight?: number; + onResize?: (size: ObservedSize) => void; + } +> = ({ resolveHandler, defaultWidth, defaultHeight, onResize, ...props }) => { const renderCountRef = useRef(0); const { ref: measuredElementRef, width = defaultWidth, - height = defaultHeight + height = defaultHeight, } = useResizeObserver({ onResize }); const currentSizeRef = useRef({ width: undefined, - height: undefined + height: undefined, }); currentSizeRef.current.width = width; currentSizeRef.current.height = height; @@ -129,7 +131,7 @@ export const Observed: FunctionComponent @@ -183,11 +185,11 @@ export function render( | FunctionComponent | FunctionComponent, { - waitForFirstMeasurement = false + waitForFirstMeasurement = false, }: { waitForFirstMeasurement?: boolean } = {}, props?: {} ) { - return new Promise(resolve => { + return new Promise((resolve) => { async function resolveHandler>( handler: T | T[] ): Promise {