Merge pull request #12 from lensapp/fix-invalid-refs-multiple-times

fix: Ensure that callback style refs on ElementComponents are only called when changed

Author: jansav

packages/element-component/src/element-component.test.tsx

@@ -126,10 +126,10 @@ describe('element', () => {
     const Div = getElementComponent('div');
 
     const TestComponent = () => {
-      const testRef = React.useRef(null);
+      const testRef = React.useRef<HTMLDivElement>(null);
 
       useEffect(() => {
-        testRef.current.classList.add('some-class-from-ref');
+        testRef.current!.classList.add('some-class-from-ref');
       });
 
       return <Div ref={testRef} data-some-element-test />;
@@ -167,35 +167,43 @@ describe('element', () => {
     ).toBe(true);
   });
 
+  it('given functional ref, when rendered, when rerendered with a different prop changed, does not call again', () => {
+    const Div = getElementComponent('div');
+    const someRef = jest.fn();
+
+    const rendered = render(<Div ref={someRef} data-some-element-test />);
+
+    someRef.mockClear();
+    rendered.rerender(<Div ref={someRef} />);
+
+    expect(someRef).not.toHaveBeenCalled();
+  });
+
   describe('given multiple plugins with ref', () => {
     let rendered: RenderResult;
     let discover: Discover;
 
     beforeEach(() => {
-      const somePlugin1 = getPlugin<{ $somePlugin1Input?: boolean }>(
-        (props: any): any => {
-          const ref = useRef(null);
-
-          useEffect(() => {
-            ref.current.classList.add('some-class-from-hook-ref');
-          }, []);
-
-          return {
-            $somePlugin1Input: undefined,
-            ref,
-          };
-        },
-      );
+      const somePlugin1 = getPlugin<{ $somePlugin1Input?: boolean }>(() => {
+        const ref = useRef<HTMLElement>(null);
 
-      const somePlugin2 = getPlugin<{ $somePlugin2Input?: boolean }>(
-        (props): any => ({
-          $somePlugin2Input: undefined,
+        useEffect(() => {
+          ref.current!.classList.add('some-class-from-hook-ref');
+        }, []);
 
-          ref: node => {
-            node?.classList.add('some-class-from-functional-ref');
-          },
-        }),
-      );
+        return {
+          $somePlugin1Input: undefined,
+          ref,
+        };
+      });
+
+      const somePlugin2 = getPlugin<{ $somePlugin2Input?: boolean }>(() => ({
+        $somePlugin2Input: undefined,
+
+        ref: node => {
+          node?.classList.add('some-class-from-functional-ref');
+        },
+      }));
 
       const Div = getElementComponent('div', somePlugin2, somePlugin1);
 

packages/element-component/src/element-component.tsx

@@ -32,37 +32,29 @@ export function getElementComponent<TagName extends TagNames>(
     .map(withMappedRef)
     .map(withMergeOutputOverInput);
 
-  return forwardRef((unprocessedProps: any, ref) => {
+  return forwardRef((unprocessedProps, ref) => {
     const { $$ref = [], ...processedProps }: any = fastPipeline(
       unprocessedProps,
       ...processedPlugins,
     ) as JSX.IntrinsicElements[TagName];
-
+    const callbackRef = React.useCallback(
+      (node: HTMLElement) => [ref, ...$$ref].forEach(handleRefFor(node)),
+      $$ref,
+    );
     const TagName = tagName as React.ElementType;
 
-    return (
-      <TagName
-        {...processedProps}
-        ref={(node: HTMLElement) => {
-          const handleRef = handleRefFor(node);
-
-          if (ref) {
-            handleRef(ref);
-          }
-
-          $$ref.forEach(handleRef);
-        }}
-      />
-    );
+    return <TagName {...processedProps} ref={callbackRef} />;
   });
 }
 
 const handleRefFor =
   (node: HTMLElement) =>
-  (ref: ((node: HTMLElement) => void) | MutableRefObject<unknown>) => {
+  (
+    ref: ((node: HTMLElement) => void) | MutableRefObject<unknown> | undefined,
+  ) => {
     if (typeof ref === 'function') {
       ref(node);
-    } else {
+    } else if (ref) {
       ref.current = node;
     }
   };