[Avatar] Fix flashing when image is already cached

.yarn/versions/4905f29b.yml

@@ -0,0 +1,5 @@
+releases:
+  "@radix-ui/react-avatar": patch
+
+declined:
+  - primitives

packages/react/avatar/src/Avatar.stories.tsx

@@ -1,9 +1,11 @@
 import { css } from '../../../../stitches.config';
 import * as Avatar from '@radix-ui/react-avatar';
+import React from 'react';
 
 export default { title: 'Components/Avatar' };
 
 const src = 'https://picsum.photos/id/1005/400/400';
+const srcAlternative = 'https://picsum.photos/id/1006/400/400';
 const srcBroken = 'https://broken.link.com/broken-pic.jpg';
 
 export const Styled = () => (
@@ -33,6 +35,18 @@ export const Styled = () => (
         <AvatarIcon />
       </Avatar.Fallback>
     </Avatar.Root>
+
+    <h1>Changing image src</h1>
+    <SourceChanger sources={[src, srcAlternative, srcBroken]}>
+      {(src) => (
+        <Avatar.Root className={rootClass()}>
+          <Avatar.Image className={imageClass()} alt="John Smith" src={src} />
+          <Avatar.Fallback delayMs={300} className={fallbackClass()}>
+            JS
+          </Avatar.Fallback>
+        </Avatar.Root>
+      )}
+    </SourceChanger>
   </>
 );
 
@@ -58,6 +72,18 @@ export const Chromatic = () => (
         <AvatarIcon />
       </Avatar.Fallback>
     </Avatar.Root>
+
+    <h1>Changing image src</h1>
+    <SourceChanger sources={[src, srcAlternative, srcBroken]}>
+      {(src) => (
+        <Avatar.Root className={rootClass()}>
+          <Avatar.Image className={imageClass()} alt="John Smith" src={src} />
+          <Avatar.Fallback delayMs={300} className={fallbackClass()}>
+            JS
+          </Avatar.Fallback>
+        </Avatar.Root>
+      )}
+    </SourceChanger>
   </>
 );
 Chromatic.parameters = { chromatic: { disable: false, delay: 1000 } };
@@ -113,3 +139,21 @@ const AvatarIcon = () => (
     />
   </svg>
 );
+
+function SourceChanger({
+  sources,
+  children,
+}: {
+  sources: string[];
+  children: (src: string) => React.ReactElement;
+}) {
+  const [src, setSrc] = React.useState(sources[0]);
+  React.useEffect(() => {
+    const interval = setInterval(() => {
+      const nextIndex = (sources.indexOf(src) + 1) % sources.length;
+      setSrc(sources[nextIndex]);
+    }, 1000);
+    return () => clearInterval(interval);
+  }, [sources, src]);
+  return children(src);
+}

packages/react/avatar/src/Avatar.test.tsx

@@ -2,57 +2,60 @@ import { axe } from 'jest-axe';
 import type { RenderResult } from '@testing-library/react';
 import { render, waitFor } from '@testing-library/react';
 import * as Avatar from '@radix-ui/react-avatar';
+import { HTMLAttributeReferrerPolicy, ReactElement } from 'react';
+import { renderToString } from 'react-dom/server';
 
 const ROOT_TEST_ID = 'avatar-root';
 const FALLBACK_TEXT = 'AB';
 const IMAGE_ALT_TEXT = 'Fake Avatar';
 const DELAY = 300;
+const cache = new Set<string>();
 
 describe('given an Avatar with fallback and no image', () => {
-  let rendered: RenderResult;
-
-  beforeEach(() => {
-    rendered = render(
-      <Avatar.Root data-testid={ROOT_TEST_ID}>
-        <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
-      </Avatar.Root>
-    );
-  });
+  const ui = (
+    <Avatar.Root data-testid={ROOT_TEST_ID}>
+      <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
+    </Avatar.Root>
+  );
 
   it('should have no accessibility violations', async () => {
+    const rendered = render(ui);
     expect(await axe(rendered.container)).toHaveNoViolations();
   });
+
+  it('should work with SSR', () => {
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+    container.innerHTML = renderToString(ui);
+    const rendered = render(ui, { hydrate: true, container });
+    const fallback = rendered.queryByText(FALLBACK_TEXT);
+    expect(fallback).toBeInTheDocument();
+  });
 });
 
-describe('given an Avatar with fallback and a working image', () => {
+describe('given an Avatar with fallback and an image', () => {
   let rendered: RenderResult;
   let image: HTMLElement | null = null;
-  const orignalGlobalImage = window.Image;
+  const originalGlobalImage = window.Image;
+  const ui = (src?: string) => (
+    <Avatar.Root data-testid={ROOT_TEST_ID}>
+      <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
+      <Avatar.Image src={src} alt={IMAGE_ALT_TEXT} />
+    </Avatar.Root>
+  );
 
   beforeAll(() => {
-    (window.Image as any) = class MockImage {
-      onload: () => void = () => {};
-      src: string = '';
-      constructor() {
-        setTimeout(() => {
-          this.onload();
-        }, DELAY);
-        return this;
-      }
-    };
+    (window.Image as any) = MockImage;
   });
 
   afterAll(() => {
-    window.Image = orignalGlobalImage;
+    window.Image = originalGlobalImage;
+    jest.restoreAllMocks();
   });
 
   beforeEach(() => {
-    rendered = render(
-      <Avatar.Root data-testid={ROOT_TEST_ID}>
-        <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
-        <Avatar.Image src="/test.jpg" alt={IMAGE_ALT_TEXT} />
-      </Avatar.Root>
-    );
+    cache.clear();
+    rendered = render(ui('/test.png'));
   });
 
   it('should render the fallback initially', () => {
@@ -74,6 +77,91 @@ describe('given an Avatar with fallback and a working image', () => {
     image = await rendered.findByAltText(IMAGE_ALT_TEXT);
     expect(image).toBeInTheDocument();
   });
+
+  it('does not leak event listeners', async () => {
+    rendered.unmount();
+    const addEventListenerSpy = jest.spyOn(window.Image.prototype, 'addEventListener');
+    const removeEventListenerSpy = jest.spyOn(window.Image.prototype, 'removeEventListener');
+    rendered = render(ui('/test.png'));
+    rendered.unmount();
+    expect(addEventListenerSpy.mock.calls.length).toEqual(removeEventListenerSpy.mock.calls.length);
+  });
+
+  it('can handle changing src', async () => {
+    image = await rendered.findByRole('img');
+    expect(image).toBeInTheDocument();
+    rendered.rerender(ui('/test2.png'));
+    image = rendered.queryByRole('img');
+    expect(image).not.toBeInTheDocument();
+    image = await rendered.findByRole('img');
+    expect(image).toBeInTheDocument();
+  });
+
+  it('should render the image immediately after it is cached', async () => {
+    image = await rendered.findByRole('img');
+    expect(image).toBeInTheDocument();
+
+    rendered.unmount();
+    rendered = render(ui('/test.png'));
+    image = rendered.queryByRole('img');
+    expect(image).toBeInTheDocument();
+  });
+
+  it('should not render image with no src', async () => {
+    rendered.rerender(ui());
+    image = rendered.queryByRole('img');
+    expect(image).not.toBeInTheDocument();
+    rendered.unmount();
+    rendered = render(ui());
+    image = rendered.queryByRole('img');
+    expect(image).not.toBeInTheDocument();
+  });
+
+  it('should not render image with empty string as src', async () => {
+    rendered.rerender(ui(''));
+    image = rendered.queryByRole('img');
+    expect(image).not.toBeInTheDocument();
+    rendered.unmount();
+    rendered = render(ui(''));
+    image = rendered.queryByRole('img');
+    expect(image).not.toBeInTheDocument();
+  });
+
+  it('should show fallback if image has no data', async () => {
+    rendered.unmount();
+    const spy = jest.spyOn(window.Image.prototype, 'naturalWidth', 'get');
+    spy.mockReturnValue(0);
+    rendered = render(ui('/test.png'));
+    const fallback = rendered.queryByText(FALLBACK_TEXT);
+    expect(fallback).toBeInTheDocument();
+    spy.mockRestore();
+  });
+
+  describe('SSR', () => {
+    function renderAndHydrate(ui: ReactElement) {
+      const container = document.createElement('div');
+      document.body.appendChild(container);
+      container.innerHTML = renderToString(ui);
+      return render(ui, { hydrate: true, container });
+    }
+
+    it('can render with working image', async () => {
+      const rendered = renderAndHydrate(ui('/test.png'));
+      let image = rendered.queryByRole('img');
+      expect(image).not.toBeInTheDocument();
+
+      image = await rendered.findByRole('img');
+      expect(image).toBeInTheDocument();
+    });
+
+    it('can render with no src', () => {
+      const rendered = renderAndHydrate(ui());
+      const image = rendered.queryByRole('img');
+      expect(image).not.toBeInTheDocument();
+      const fallback = rendered.queryByText(FALLBACK_TEXT);
+      expect(fallback).toBeInTheDocument();
+    });
+  });
 });
 
 describe('given an Avatar with fallback and delayed render', () => {
@@ -103,39 +191,39 @@ describe('given an Avatar with fallback and delayed render', () => {
 
 describe('given an Avatar with an image that only works when referrerPolicy=no-referrer', () => {
   let rendered: RenderResult;
-  const orignalGlobalImage = window.Image;
+  const originalGlobalImage = window.Image;
+  const ui = (src?: string, referrerPolicy?: HTMLAttributeReferrerPolicy) => (
+    <Avatar.Root data-testid={ROOT_TEST_ID}>
+      <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
+      <Avatar.Image src={src} alt={IMAGE_ALT_TEXT} referrerPolicy={referrerPolicy} />
+    </Avatar.Root>
+  );
 
   beforeAll(() => {
-    (window.Image as any) = class MockImage {
-      onload: () => void = () => {};
-      onerror: () => void = () => {};
-      src: string = '';
+    (window.Image as any) = class MockNoReferrerImage extends MockImage {
       referrerPolicy: string | undefined;
-      constructor() {
+
+      onSrcChange() {
         setTimeout(() => {
           if (this.referrerPolicy === 'no-referrer') {
-            this.onload();
+            this.dispatchEvent(new Event('load'));
           } else {
-            this.onerror();
+            this.dispatchEvent(new Event('error'));
           }
         }, DELAY);
-        return this;
       }
     };
   });
 
   afterAll(() => {
-    window.Image = orignalGlobalImage;
+    window.Image = originalGlobalImage;
+    jest.restoreAllMocks();
   });
 
   describe('referrerPolicy=no-referrer', () => {
     beforeEach(() => {
-      rendered = render(
-        <Avatar.Root data-testid={ROOT_TEST_ID}>
-          <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
-          <Avatar.Image src="/test.jpg" alt={IMAGE_ALT_TEXT} referrerPolicy="no-referrer" />
-        </Avatar.Root>
-      );
+      cache.clear();
+      rendered = render(ui('/test.png', 'no-referrer'));
     });
 
     it('should render the fallback initially', () => {
@@ -161,12 +249,8 @@ describe('given an Avatar with an image that only works when referrerPolicy=no-r
 
   describe('referrerPolicy=origin', () => {
     beforeEach(() => {
-      rendered = render(
-        <Avatar.Root data-testid={ROOT_TEST_ID}>
-          <Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
-          <Avatar.Image src="/test.jpg" alt={IMAGE_ALT_TEXT} referrerPolicy="origin" />
-        </Avatar.Root>
-      );
+      cache.clear();
+      rendered = render(ui('/test.png', 'origin'));
     });
 
     it('should render the fallback initially', () => {
@@ -187,3 +271,39 @@ describe('given an Avatar with an image that only works when referrerPolicy=no-r
     });
   });
 });
+
+class MockImage extends EventTarget {
+  _src: string = '';
+
+  constructor() {
+    super();
+    return this;
+  }
+
+  get src() {
+    return this._src;
+  }
+
+  set src(src: string) {
+    if (!src) {
+      return;
+    }
+    this._src = src;
+    this.onSrcChange();
+  }
+
+  get complete() {
+    return !this.src || cache.has(this.src);
+  }
+
+  get naturalWidth() {
+    return this.complete ? 300 : 0;
+  }
+
+  onSrcChange() {
+    setTimeout(() => {
+      this.dispatchEvent(new Event('load'));
+      cache.add(this.src);
+    }, DELAY);
+  }
+}