Update code for the review comments

* Make decode module return value `ImageData` * Fix global definition of ImageData * Use concrete Encoder types for encode functions * Use ArrayBufferView in FileLike instead of using a similar type * Throw error when the `encode` functions return null * Use generic types for WorkerPool * Fix `encode` function typing in `index.ts` * Remove ts-ignore for web-streams-polyfill and handle nulls for TransformStream * Fix rollup entry point (now we need to have `index.ts` instead of `index.js`)
2021-08-06 16:05:14 +03:00
parent de4eb9c8f7
commit fafcf97f0c
4 changed files with 82 additions and 56 deletions
--- a/libsquoosh/rollup.config.js
+++ b/libsquoosh/rollup.config.js
@@ -10,7 +10,7 @@ import { builtinModules } from 'module';

 /** @type {import('rollup').RollupOptions} */
 export default {
-  input: 'src/index.js',
+  input: 'src/index.ts',
  output: {
    dir: 'build',
    format: 'cjs',
--- a/libsquoosh/src/codecs.ts
+++ b/libsquoosh/src/codecs.ts
@@ -10,22 +10,11 @@ import { cpus } from 'os';
 };

 interface DecodeModule extends EmscriptenWasm.Module {
-  decode: (data: Uint8Array) => any;
-}
-
-interface EncodeModule extends EmscriptenWasm.Module {
-  encode: (
-    data: Uint8ClampedArray | ArrayBuffer,
-    width: number,
-    height: number,
-    opts: any,
-  ) => Uint8Array;
+  decode: (data: Uint8Array) => ImageData;
 }

 type DecodeModuleFactory = EmscriptenWasm.ModuleFactory<DecodeModule>;

-type EncodeModuleFactory = EmscriptenWasm.ModuleFactory<EncodeModule>;
-
 interface RotateModuleInstance {
  exports: {
    memory: WebAssembly.Memory;
@@ -50,7 +39,7 @@ interface ResizeInstantiateOptions {

 declare global {
  // Needed for being able to use ImageData as type in codec types
-  type ImageData = typeof import('./image_data.js');
+  type ImageData = import('./image_data.js').default;
  // Needed for being able to assign to `globalThis.ImageData`
  var ImageData: ImageData['constructor'];
 }
@@ -58,18 +47,21 @@ declare global {
 import type { QuantizerModule } from '../../codecs/imagequant/imagequant.js';

 // MozJPEG
+import type { MozJPEGModule as MozJPEGEncodeModule } from '../../codecs/mozjpeg/enc/mozjpeg_enc';
 import mozEnc from '../../codecs/mozjpeg/enc/mozjpeg_node_enc.js';
 import mozEncWasm from 'asset-url:../../codecs/mozjpeg/enc/mozjpeg_node_enc.wasm';
 import mozDec from '../../codecs/mozjpeg/dec/mozjpeg_node_dec.js';
 import mozDecWasm from 'asset-url:../../codecs/mozjpeg/dec/mozjpeg_node_dec.wasm';

 // WebP
+import type { WebPModule as WebPEncodeModule } from '../../codecs/webp/enc/webp_enc';
 import webpEnc from '../../codecs/webp/enc/webp_node_enc.js';
 import webpEncWasm from 'asset-url:../../codecs/webp/enc/webp_node_enc.wasm';
 import webpDec from '../../codecs/webp/dec/webp_node_dec.js';
 import webpDecWasm from 'asset-url:../../codecs/webp/dec/webp_node_dec.wasm';

 // AVIF
+import type { AVIFModule as AVIFEncodeModule } from '../../codecs/avif/enc/avif_enc';
 import avifEnc from '../../codecs/avif/enc/avif_node_enc.js';
 import avifEncWasm from 'asset-url:../../codecs/avif/enc/avif_node_enc.wasm';
 import avifEncMt from '../../codecs/avif/enc/avif_node_enc_mt.js';
@@ -79,12 +71,14 @@ import avifDec from '../../codecs/avif/dec/avif_node_dec.js';
 import avifDecWasm from 'asset-url:../../codecs/avif/dec/avif_node_dec.wasm';

 // JXL
+import type { JXLModule as JXLEncodeModule } from '../../codecs/jxl/enc/jxl_enc';
 import jxlEnc from '../../codecs/jxl/enc/jxl_node_enc.js';
 import jxlEncWasm from 'asset-url:../../codecs/jxl/enc/jxl_node_enc.wasm';
 import jxlDec from '../../codecs/jxl/dec/jxl_node_dec.js';
 import jxlDecWasm from 'asset-url:../../codecs/jxl/dec/jxl_node_dec.wasm';

 // WP2
+import type { WP2Module as WP2EncodeModule } from '../../codecs/wp2/enc/wp2_enc';
 import wp2Enc from '../../codecs/wp2/enc/wp2_node_enc.js';
 import wp2EncWasm from 'asset-url:../../codecs/wp2/enc/wp2_node_enc.wasm';
 import wp2Dec from '../../codecs/wp2/dec/wp2_node_dec.js';
@@ -284,7 +278,9 @@ export const codecs = {
    dec: () =>
      instantiateEmscriptenWasm(mozDec as DecodeModuleFactory, mozDecWasm),
    enc: () =>
-      instantiateEmscriptenWasm(mozEnc as EncodeModuleFactory, mozEncWasm),
+      instantiateEmscriptenWasm(mozEnc, mozEncWasm) as Promise<
+        MozJPEGEncodeModule
+      >,
    defaultEncoderOptions: {
      quality: 75,
      baseline: false,
@@ -316,7 +312,9 @@ export const codecs = {
    dec: () =>
      instantiateEmscriptenWasm(webpDec as DecodeModuleFactory, webpDecWasm),
    enc: () =>
-      instantiateEmscriptenWasm(webpEnc as EncodeModuleFactory, webpEncWasm),
+      instantiateEmscriptenWasm(webpEnc, webpEncWasm) as Promise<
+        WebPEncodeModule
+      >,
    defaultEncoderOptions: {
      quality: 75,
      target_size: 0,
@@ -361,15 +359,14 @@ export const codecs = {
    enc: async () => {
      if (await threads()) {
        return instantiateEmscriptenWasm(
-          avifEncMt as EncodeModuleFactory,
+          avifEncMt,
          avifEncMtWasm,
          avifEncMtWorker,
-        );
+        ) as Promise<AVIFEncodeModule>;
      }
-      return instantiateEmscriptenWasm(
-        avifEnc as EncodeModuleFactory,
-        avifEncWasm,
-      );
+      return instantiateEmscriptenWasm(avifEnc, avifEncWasm) as Promise<
+        AVIFEncodeModule
+      >;
    },
    defaultEncoderOptions: {
      cqLevel: 33,
@@ -396,7 +393,7 @@ export const codecs = {
    dec: () =>
      instantiateEmscriptenWasm(jxlDec as DecodeModuleFactory, jxlDecWasm),
    enc: () =>
-      instantiateEmscriptenWasm(jxlEnc as EncodeModuleFactory, jxlEncWasm),
+      instantiateEmscriptenWasm(jxlEnc, jxlEncWasm) as Promise<JXLEncodeModule>,
    defaultEncoderOptions: {
      speed: 4,
      quality: 75,
@@ -419,7 +416,7 @@ export const codecs = {
    dec: () =>
      instantiateEmscriptenWasm(wp2Dec as DecodeModuleFactory, wp2DecWasm),
    enc: () =>
-      instantiateEmscriptenWasm(wp2Enc as EncodeModuleFactory, wp2EncWasm),
+      instantiateEmscriptenWasm(wp2Enc, wp2EncWasm) as Promise<WP2EncodeModule>,
    defaultEncoderOptions: {
      quality: 75,
      alpha_quality: 75,
--- a/libsquoosh/src/index.ts
+++ b/libsquoosh/src/index.ts
@@ -10,7 +10,7 @@ import type ImageData from './image_data';
 export { ImagePool, encoders, preprocessors };
 type EncoderKey = keyof typeof encoders;
 type PreprocessorKey = keyof typeof preprocessors;
-type FileLike = Buffer | ArrayBuffer | string | { buffer: Buffer };
+type FileLike = Buffer | ArrayBuffer | string | ArrayBufferView;

 async function decodeFile({
  file,
@@ -24,8 +24,9 @@ async function decodeFile({
  } else if (file instanceof ArrayBuffer) {
    buffer = Buffer.from(file);
    file = 'Binary blob';
-  } else if (file instanceof Buffer) {
-    buffer = file;
+  } else if ((file as unknown) instanceof Buffer) {
+    // TODO: Check why we need type assertions here.
+    buffer = (file as unknown) as Buffer;
    file = 'Binary blob';
  } else if (typeof file === 'string') {
    buffer = await fsp.readFile(file);
@@ -99,9 +100,16 @@ async function encodeImage({
        }),
      );
    const decode = (binary: Uint8Array) => decoder.decode(binary);
+    const nonNullEncode = (bitmap: ImageData, quality: number): Uint8Array => {
+      const result = encode(bitmap, quality);
+      if (!result) {
+        throw new Error('There was an error while encoding');
+      }
+      return result;
+    };
    const { binary: optimizedBinary, quality } = await autoOptimize(
      bitmapIn,
-      encode,
+      nonNullEncode,
      decode,
      {
        min: encoders[encName].autoOptimize.min,
@@ -116,12 +124,18 @@ async function encodeImage({
      [optionToOptimize]: Math.round(quality * 10000) / 10000,
    };
  } else {
-    binary = encoder.encode(
+    const result = encoder.encode(
      bitmapIn.data.buffer,
      bitmapIn.width,
      bitmapIn.height,
      encConfig,
    );
+
+    if (!result) {
+      throw new Error('There was an error while encoding');
+    }
+
+    binary = result;
  }
  return {
    optionsUsed,
@@ -155,11 +169,11 @@ function handleJob(params: JobMessage) {
 */
 class Image {
  public file: FileLike;
-  public workerPool: WorkerPool;
+  public workerPool: WorkerPool<JobMessage, any>;
  public decoded: Promise<{ bitmap: ImageData }>;
  public encodedWith: { [key: string]: any };

-  constructor(workerPool: WorkerPool, file: FileLike) {
+  constructor(workerPool: WorkerPool<JobMessage, any>, file: FileLike) {
    this.file = file;
    this.workerPool = workerPool;
    this.decoded = workerPool.dispatchJob({ operation: 'decode', file });
@@ -201,6 +215,8 @@ class Image {
    encodeOptions: {
      optimizerButteraugliTarget?: number;
      maxOptimizerRounds?: number;
+    } & {
+      [key in EncoderKey]?: any; // any is okay for now
    } = {},
  ): Promise<void> {
    const { bitmap } = await this.decoded;
@@ -233,7 +249,7 @@ class Image {
 * A pool where images can be ingested and squooshed.
 */
 class ImagePool {
-  public workerPool: WorkerPool;
+  public workerPool: WorkerPool<JobMessage, any>;

  /**
   * Create a new pool.
--- a/libsquoosh/src/worker_pool.ts
+++ b/libsquoosh/src/worker_pool.ts
@@ -1,7 +1,5 @@
 import { Worker, parentPort } from 'worker_threads';
-// @ts-ignore
 import { TransformStream } from 'web-streams-polyfill';
-import type { JobMessage } from './index';

 function uuid() {
  return Array.from({ length: 16 }, () =>
@@ -9,28 +7,16 @@ function uuid() {
  ).join('');
 }

-function jobPromise(worker: Worker, msg: JobMessage) {
-  return new Promise((resolve, reject) => {
-    const id = uuid();
-    worker.postMessage({ msg, id });
-    worker.on('message', function f({ error, result, id: rid }) {
-      if (rid !== id) {
-        return;
-      }
-      if (error) {
-        reject(error);
-        return;
-      }
-      worker.off('message', f);
-      resolve(result);
-    });
-  });
+interface Job<I> {
+  msg: I;
+  resolve: Function;
+  reject: Function;
 }

-export default class WorkerPool {
+export default class WorkerPool<I, O> {
  public numWorkers: number;
-  public jobQueue: TransformStream;
-  public workerQueue: TransformStream;
+  public jobQueue: TransformStream<Job<I>, Job<I>>;
+  public workerQueue: TransformStream<Worker, Worker>;
  public done: Promise<void>;

  constructor(numWorkers: number, workerFile: string) {
@@ -55,9 +41,14 @@ export default class WorkerPool {
        await this._terminateAll();
        return;
      }
+
+      if (!value) {
+        throw new Error('Reader did not return any value');
+      }
+
      const { msg, resolve, reject } = value;
      const worker = await this._nextWorker();
-      jobPromise(worker, msg)
+      this.jobPromise(worker, msg)
        .then((result) => resolve(result))
        .catch((reason) => reject(reason))
        .finally(() => {
@@ -73,6 +64,10 @@ export default class WorkerPool {
    const reader = this.workerQueue.readable.getReader();
    const { value } = await reader.read();
    reader.releaseLock();
+    if (!value) {
+      throw new Error('No worker left');
+    }
+
    return value;
  }

@@ -89,7 +84,7 @@ export default class WorkerPool {
    await this.done;
  }

-  dispatchJob(msg: JobMessage): Promise<any> {
+  dispatchJob(msg: I): Promise<O> {
    return new Promise((resolve, reject) => {
      const writer = this.jobQueue.writable.getWriter();
      writer.write({ msg, resolve, reject });
@@ -97,7 +92,25 @@ export default class WorkerPool {
    });
  }

-  static useThisThreadAsWorker(cb: (msg: JobMessage) => any) {
+  private jobPromise(worker: Worker, msg: I) {
+    return new Promise((resolve, reject) => {
+      const id = uuid();
+      worker.postMessage({ msg, id });
+      worker.on('message', function f({ error, result, id: rid }) {
+        if (rid !== id) {
+          return;
+        }
+        if (error) {
+          reject(error);
+          return;
+        }
+        worker.off('message', f);
+        resolve(result);
+      });
+    });
+  }
+
+  static useThisThreadAsWorker<I, O>(cb: (msg: I) => O) {
    parentPort!.on('message', async (data) => {
      const { msg, id } = data;
      try {