feat(image): various improvements (#3264)

Author: diegomura

.changeset/quick-camels-grin.md

@@ -0,0 +1,5 @@
+---
+"@react-pdf/image": patch
+---
+
+feat(image): various improvements

packages/image/src/cache.ts

@@ -1,21 +1,23 @@
 const createCache = <T>({ limit = 100 } = {}) => {
-  let cache: Record<string, T> = {};
-  let keys: string[] = [];
+  let cache = new Map<string, T>();
 
   return {
-    get: (key: string | null): T | null => (key ? cache[key] : null),
+    get: (key: string | null): T | undefined | null =>
+      key ? cache.get(key) ?? undefined : null,
     set: (key: string, value: T) => {
-      keys.push(key);
-      if (keys.length > limit) {
-        delete cache[keys.shift()!];
+      cache.delete(key);
+
+      if (cache.size >= limit) {
+        const firstKey = cache.keys().next().value as string;
+        cache.delete(firstKey);
       }
-      cache[key] = value;
+
+      cache.set(key, value);
     },
     reset: () => {
-      cache = {};
-      keys = [];
+      cache = new Map();
     },
-    length: () => keys.length,
+    length: () => cache.size,
   };
 };
 

packages/image/src/png.ts

@@ -18,11 +18,18 @@ class PNG implements Image {
   }
 
   static isValid(data: Buffer): boolean {
-    try {
-      return !!new PNG(data);
-    } catch {
-      return false;
-    }
+    return (
+      data &&
+      Buffer.isBuffer(data) &&
+      data[0] === 137 &&
+      data[1] === 80 &&
+      data[2] === 78 &&
+      data[3] === 71 &&
+      data[4] === 13 &&
+      data[5] === 10 &&
+      data[6] === 26 &&
+      data[7] === 10
+    );
   }
 }
 

packages/image/src/resolve.ts

@@ -27,9 +27,8 @@ const isDataImageSrc = (src: ImageSrc): src is DataImageSrc => {
   return 'data' in src;
 };
 
-const isBase64Src = (imageSrc: ImageSrc): imageSrc is Base64ImageSrc =>
-  'uri' in imageSrc &&
-  /^data:image\/[a-zA-Z]*;base64,[^"]*/g.test(imageSrc.uri);
+const isDataUri = (imageSrc: ImageSrc): imageSrc is Base64ImageSrc =>
+  'uri' in imageSrc && imageSrc.uri.startsWith('data:');
 
 const getAbsoluteLocalPath = (src: string) => {
   if (BROWSER) {
@@ -58,14 +57,14 @@ const fetchLocalFile = (src: LocalImageSrc): Promise<Buffer> =>
   new Promise((resolve, reject) => {
     try {
       if (BROWSER) {
-        reject(new Error('Cannot fetch local file in this environemnt'));
+        reject(new Error('Cannot fetch local file in this environment'));
         return;
       }
 
       const absolutePath = getAbsoluteLocalPath(src.uri);
 
       if (!absolutePath) {
-        reject(new Error(`Cannot fetch non-local path: ${src}`));
+        reject(new Error(`Cannot fetch non-local path: ${src.uri}`));
         return;
       }
 
@@ -97,13 +96,13 @@ const isValidFormat = (format: string): format is ImageFormat => {
   return lower === 'jpg' || lower === 'jpeg' || lower === 'png';
 };
 
-const guessFormat = (buffer: Buffer) => {
+const getImageFormat = (buffer: Buffer) => {
   let format;
 
   if (JPEG.isValid(buffer)) {
-    format = 'jpg';
+    format = 'jpg' as const;
   } else if (PNG.isValid(buffer)) {
-    format = 'png';
+    format = 'png' as const;
   }
 
   return format;
@@ -144,7 +143,7 @@ const resolveImageFromData = async (src: DataImageSrc) => {
 };
 
 const resolveBufferImage = async (buffer: Buffer) => {
-  const format = guessFormat(buffer);
+  const format = getImageFormat(buffer);
 
   if (format) {
     return getImage(buffer, format);
@@ -176,31 +175,6 @@ const resolveBlobImage = async (blob: Blob) => {
   return getImage(Buffer.from(buffer), format);
 };
 
-const getImageFormat = (body: Buffer) => {
-  const isPng =
-    body[0] === 137 &&
-    body[1] === 80 &&
-    body[2] === 78 &&
-    body[3] === 71 &&
-    body[4] === 13 &&
-    body[5] === 10 &&
-    body[6] === 26 &&
-    body[7] === 10;
-
-  const isJpg = body[0] === 255 && body[1] === 216 && body[2] === 255;
-
-  let extension = '';
-  if (isPng) {
-    extension = 'png';
-  } else if (isJpg) {
-    extension = 'jpg';
-  } else {
-    throw new Error('Not valid image extension');
-  }
-
-  return extension;
-};
-
 const resolveImageFromUrl = async (src: LocalImageSrc | RemoteImageSrc) => {
   const data =
     !BROWSER && getAbsoluteLocalPath(src.uri)
@@ -209,13 +183,17 @@ const resolveImageFromUrl = async (src: LocalImageSrc | RemoteImageSrc) => {
 
   const format = getImageFormat(data);
 
+  if (!format) {
+    throw new Error('Not valid image extension');
+  }
+
   return getImage(data, format);
 };
 
 const getCacheKey = (src: ImageSrc): string | null => {
   if (isBlob(src) || isBuffer(src)) return null;
 
-  if (isDataImageSrc(src)) return src.data.toString();
+  if (isDataImageSrc(src)) return src.data?.toString('base64') ?? null;
 
   return src.uri;
 };
@@ -231,18 +209,14 @@ const resolveImage = (src: ImageSrc, { cache = true } = {}) => {
     image = resolveBufferImage(src);
   } else if (cache && IMAGE_CACHE.get(cacheKey)) {
     return IMAGE_CACHE.get(cacheKey);
-  } else if (isBase64Src(src)) {
+  } else if (isDataUri(src)) {
     image = resolveBase64Image(src);
   } else if (isDataImageSrc(src)) {
     image = resolveImageFromData(src);
   } else {
     image = resolveImageFromUrl(src);
   }
 
-  if (!image) {
-    throw new Error('Cannot resolve image');
-  }
-
   if (cache && cacheKey) {
     IMAGE_CACHE.set(cacheKey, image);
   }

packages/image/tests/cache.test.ts

@@ -52,4 +52,48 @@ describe('Background', () => {
     expect(cache.get('4')).toBe('4');
     expect(cache.get('5')).toBe('5');
   });
+
+  test('should return null when getting with null key', () => {
+    const cache = createCache();
+    cache.set('somekey', 'somevalue');
+    expect(cache.get(null)).toBeNull();
+  });
+
+  test('should use default limit of 100', () => {
+    const cache = createCache();
+
+    for (let i = 0; i < 105; i++) {
+      cache.set(`key${i}`, `value${i}`);
+    }
+
+    expect(cache.length()).toBe(100);
+    expect(cache.get('key0')).toBe(undefined);
+    expect(cache.get('key4')).toBe(undefined);
+    expect(cache.get('key5')).toBe('value5');
+    expect(cache.get('key104')).toBe('value104');
+  });
+
+  test('should overwrite existing key without increasing length', () => {
+    const cache = createCache({ limit: 3 });
+
+    cache.set('key1', 'value1');
+    cache.set('key2', 'value2');
+    cache.set('key1', 'updatedValue1');
+
+    expect(cache.get('key1')).toBe('updatedValue1');
+    expect(cache.length()).toBe(2);
+  });
+
+  test('should handle limit of 1', () => {
+    const cache = createCache({ limit: 1 });
+
+    cache.set('key1', 'value1');
+    expect(cache.get('key1')).toBe('value1');
+    expect(cache.length()).toBe(1);
+
+    cache.set('key2', 'value2');
+    expect(cache.get('key1')).toBe(undefined);
+    expect(cache.get('key2')).toBe('value2');
+    expect(cache.length()).toBe(1);
+  });
 });

packages/image/tests/resolve.test.ts

@@ -101,7 +101,6 @@ describe('image resolveImage', () => {
     expect(image?.height).toBeGreaterThan(0);
   });
 
-
   test('Should render a local image from relative path', async () => {
     const image = await resolveImage({
       uri: 'packages/layout/tests/assets/test.jpg',
@@ -112,7 +111,6 @@ describe('image resolveImage', () => {
     expect(image?.height).toBeGreaterThan(0);
   });
 
-
   test('Should render a local image from src object', async () => {
     const image = await resolveImage({
       uri: './packages/layout/tests/assets/test.jpg',
@@ -232,4 +230,81 @@ describe('image resolveImage', () => {
     expect(image?.width).toBeGreaterThan(0);
     expect(image?.height).toBeGreaterThan(0);
   });
+
+  test('Should throw error for unsupported base64 format', async () => {
+    await expect(
+      resolveImage({
+        uri: 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==',
+      }),
+    ).rejects.toThrow('Base64 image invalid format: gif');
+  });
+
+  test('Should throw error for invalid base64 URI', async () => {
+    await expect(
+      resolveImage({ uri: 'data:image/pngbase64,invalid' } as any),
+    ).rejects.toThrow('Invalid base64 image');
+  });
+
+  test('Should throw error for invalid blob type', async () => {
+    const blob = new Blob([localJPGImage], { type: 'text/plain' });
+    await expect(resolveImage(blob)).rejects.toThrow(
+      'Invalid blob type: text/plain',
+    );
+  });
+
+  test('Should throw error for unsupported blob image type', async () => {
+    const blob = new Blob([localJPGImage], { type: 'image/gif' });
+    await expect(resolveImage(blob)).rejects.toThrow(
+      'Invalid blob type: image/gif',
+    );
+  });
+
+  test('Should throw error for invalid data source', async () => {
+    await expect(
+      resolveImage({ data: undefined as any, format: 'jpg' }),
+    ).rejects.toThrow('Invalid data given for local file');
+  });
+
+  test('Should throw error for non-existent local file', async () => {
+    await expect(
+      resolveImage({ uri: '/nonexistent/path/image.jpg' }),
+    ).rejects.toThrow();
+  });
+
+  test('Should return null for invalid buffer', async () => {
+    const invalidBuffer = Buffer.from('not an image');
+    const image = await resolveImage(invalidBuffer);
+    expect(image).toBeNull();
+  });
+
+  test('Should throw on network fetch failure', async () => {
+    fetchMock.once(() => {
+      throw new Error('Network error');
+    });
+
+    await expect(resolveImage({ uri: jpgImageUrl })).rejects.toThrow(
+      'Network error',
+    );
+  });
+
+  test('Should throw for unsupported remote image format', async () => {
+    // GIF89a magic bytes - neither JPEG nor PNG
+    const gifBuffer = Buffer.from([0x47, 0x49, 0x46, 0x38, 0x39, 0x61]);
+    fetchMock.once(gifBuffer);
+
+    await expect(
+      resolveImage({ uri: 'https://example.com/image.gif' }),
+    ).rejects.toThrow('Not valid image extension');
+  });
+
+  test('Should cache DataImageSrc images using base64 key', async () => {
+    const image1 = await resolveImage({ data: localJPGImage, format: 'jpg' });
+    const image2 = await resolveImage({ data: localJPGImage, format: 'jpg' });
+
+    expect(image1).toBe(image2);
+  });
+
+  test('Should return null when getting cache with null key', () => {
+    expect(IMAGE_CACHE.get(null)).toBeNull();
+  });
 });