Move encoder options from constructor to getBuffer and writeToFile, re-add Node 18 & 20 tests

gomander · gomander · commit 24540f909df2 · 2025-01-05T11:30:21.000+01:00
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -203,7 +203,7 @@ jobs:
           if-no-files-found: error
 
   test-macOS-windows-binding:
-    name: Test on ${{ matrix.settings.target }}
+    name: Test on ${{ matrix.settings.target }} - Node ${{ matrix.node }}
     needs:
       - host-build
     strategy:
@@ -216,6 +216,7 @@ jobs:
             target: 'aarch64-apple-darwin'
           - host: windows-latest
             target: 'x86_64-pc-windows-msvc'
+        node: [18, 20, 22]
     runs-on: ${{ matrix.settings.host }}
     steps:
       - name: Checkout code
@@ -230,15 +231,15 @@ jobs:
         uses: actions/setup-node@v4
         if: startsWith(matrix.settings.target, 'x86_64')
         with:
-          node-version: 22
+          node-version: ${{ matrix.node }}
           cache: pnpm
           architecture: 'x64'
 
       - name: Setup node
         uses: actions/setup-node@v4
         if: startsWith(matrix.settings.target, 'aarch64')
         with:
-          node-version: 22
+          node-version: ${{ matrix.node }}
           cache: pnpm
           architecture: 'arm64'
 
@@ -255,11 +256,13 @@ jobs:
         run: pnpm test
 
   test-linux-x64-gnu-binding:
-    name: Test bindings on linux-x64-gnu
+    name: Test bindings on linux-x64-gnu - Node ${{ matrix.node }}
     needs:
       - cross-build
     strategy:
       fail-fast: false
+      matrix:
+        node: [18, 20, 22]
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code
@@ -273,7 +276,7 @@ jobs:
       - name: Setup node
         uses: actions/setup-node@v4
         with:
-          node-version: 22
+          node-version: ${{ matrix.node }}
           cache: pnpm
 
       - name: Install dependencies
diff --git a/README.md b/README.md
@@ -15,14 +15,9 @@ npm install napi-webp-animation
 |                  | node18 | node20 | node22 |
 | ---------------- | ------ | ------ | ------ |
 | Windows x64      | ✓      | ✓      | ✓      |
-| Windows arm64    | ✓      | ✓      | ✓      |
 | Linux x64 gnu    | ✓      | ✓      | ✓      |
-| Linux x64 musl   | ✓      | ✓      | ✓      |
-| Linux arm gnu    | ✓      | ✓      | ✓      |
-| Linux arm64 gnu  | ✓      | ✓      | ✓      |
-| Linux arm64 musl | ✓      | ✓      | ✓      |
-| Android arm64    | ✓      | ✓      | ✓      |
-| Android armv7    | ✓      | ✓      | ✓      |
+| macOS x64        | ✓      | ✓      | ✓      |
+| macOS aarch64    | ✓      | ✓      | ✓      |
 
 
 ## Developing
diff --git a/index.d.ts b/index.d.ts
@@ -11,14 +11,13 @@ export interface JsWebpEncoderOptions {
 }
 export type JsWebpEncoder = WebpEncoder
 export declare class WebpEncoder {
-  constructor(width: number, height: number, options?: JsWebpEncoderOptions)
+  constructor(width: number, height: number)
   setFrameRate(frameRate: number): void
   addFrame(frameData: Buffer, duration?: number): void
-  getBuffer(): Promise<Buffer>
-  getBufferSync(): Buffer
-  writeToFile(path: string): Promise<Buffer>
-  writeToFileSync(path: string): Buffer
+  getBuffer(options?: JsWebpEncoderOptions): Promise<Buffer>
+  getBufferSync(options?: JsWebpEncoderOptions): Buffer
+  writeToFile(path: string, options?: JsWebpEncoderOptions): Promise<Buffer>
+  writeToFileSync(path: string, options?: JsWebpEncoderOptions): Buffer
   clearFrames(): void
   setDimensions(width: number, height: number): void
-  setOptions(options: JsWebpEncoderOptions): void
 }
diff --git a/package.json b/package.json
@@ -52,7 +52,7 @@
     "build:debug": "napi build --platform",
     "create-npm-dir": "napi create-npm-dir -t .",
     "prepublishOnly": "napi prepublish -t npm",
-    "test": "node --experimental-strip-types test/main.test.mts",
+    "test": "node test/main.test.mjs",
     "version": "napi version"
   },
   "devDependencies": {
diff --git a/src/encoder.rs b/src/encoder.rs
@@ -19,17 +19,15 @@ impl Clone for WebpEncoderFrame {
 pub struct WebpEncoder {
     width: u32,
     height: u32,
-    options: EncoderOptions,
     frame_rate: u16,
     frames: Vec<WebpEncoderFrame>,
 }
 
 impl WebpEncoder {
-    pub fn new(width: u32, height: u32, options: Option<EncoderOptions>) -> Self {
+    pub fn new(width: u32, height: u32) -> Self {
         WebpEncoder {
             width,
             height,
-            options: options.unwrap_or_else(EncoderOptions::default),
             frame_rate: 30,
             frames: Vec::new(),
         }
@@ -46,10 +44,10 @@ impl WebpEncoder {
         });
     }
 
-    pub fn get_buffer(&self) -> Result<Vec<u8>> {
-        let mut encoder =
-            Encoder::new_with_options((self.width, self.height), self.options.to_owned())
-                .map_err(EncoderError::EncoderError)?;
+    pub fn get_buffer(&self, options: Option<EncoderOptions>) -> Result<Vec<u8>> {
+        let options = options.unwrap_or_else(|| EncoderOptions::default());
+        let mut encoder = Encoder::new_with_options((self.width, self.height), options)
+            .map_err(EncoderError::EncoderError)?;
         let mut timestamp: i32 = 0;
         for frame in &self.frames {
             encoder
@@ -63,8 +61,8 @@ impl WebpEncoder {
             .to_vec())
     }
 
-    pub fn write_to_file(&self, path: String) -> Result<Vec<u8>> {
-        let buffer = self.get_buffer()?;
+    pub fn write_to_file(&self, path: String, options: Option<EncoderOptions>) -> Result<Vec<u8>> {
+        let buffer = self.get_buffer(options)?;
         std::fs::write(
             match path.ends_with(".webp") {
                 true => path,
@@ -85,15 +83,10 @@ impl WebpEncoder {
         self.height = height;
     }
 
-    pub fn set_options(&mut self, options: EncoderOptions) {
-        self.options = options;
-    }
-
     pub fn clone(&self) -> Self {
         WebpEncoder {
             width: self.width,
             height: self.height,
-            options: self.options.clone(),
             frame_rate: self.frame_rate,
             frames: self.frames.clone(),
         }
@@ -110,6 +103,7 @@ pub struct JsWebpEncoderOptions {
 
 pub struct AsyncGetBuffer {
     encoder: WebpEncoder,
+    options: Option<EncoderOptions>,
 }
 
 #[napi]
@@ -118,7 +112,7 @@ impl Task for AsyncGetBuffer {
     type JsValue = Buffer;
 
     fn compute(&mut self) -> Result<Self::Output> {
-        self.encoder.get_buffer()
+        self.encoder.get_buffer(self.options.clone())
     }
 
     fn resolve(&mut self, _: Env, output: Self::Output) -> Result<Self::JsValue> {
@@ -129,6 +123,7 @@ impl Task for AsyncGetBuffer {
 pub struct AsyncWriteToFile {
     encoder: WebpEncoder,
     path: String,
+    options: Option<EncoderOptions>,
 }
 
 #[napi]
@@ -137,7 +132,8 @@ impl Task for AsyncWriteToFile {
     type JsValue = Buffer;
 
     fn compute(&mut self) -> Result<Self::Output> {
-        self.encoder.write_to_file(self.path.clone())
+        self.encoder
+            .write_to_file(self.path.clone(), self.options.clone())
     }
 
     fn resolve(&mut self, _: Env, output: Self::Output) -> Result<Self::JsValue> {
@@ -153,28 +149,9 @@ pub struct JsWebpEncoder {
 #[napi]
 impl JsWebpEncoder {
     #[napi(constructor)]
-    pub fn new(
-        width: u32,
-        height: u32,
-        #[napi(ts_arg_type = "JsWebpEncoderOptions")] options: Option<JsWebpEncoderOptions>,
-    ) -> Self {
-        let options = options.map(|options| EncoderOptions {
-            anim_params: AnimParams {
-                loop_count: options.loop_count.unwrap_or(0),
-            },
-            encoding_config: Some(EncodingConfig {
-                encoding_type: if options.lossless.unwrap_or(true) {
-                    webp_animation::EncodingType::Lossless
-                } else {
-                    webp_animation::EncodingType::Lossy(LossyEncodingConfig::default())
-                },
-                quality: options.quality.unwrap_or(1) as f32,
-                method: options.method.unwrap_or(4) as usize,
-            }),
-            ..Default::default()
-        });
+    pub fn new(width: u32, height: u32) -> Self {
         JsWebpEncoder {
-            encoder: WebpEncoder::new(width, height, options),
+            encoder: WebpEncoder::new(width, height),
         }
     }
 
@@ -193,28 +170,50 @@ impl JsWebpEncoder {
     }
 
     #[napi]
-    pub fn get_buffer(&self) -> AsyncTask<AsyncGetBuffer> {
+    pub fn get_buffer(
+        &self,
+        #[napi(ts_arg_type = "JsWebpEncoderOptions")] options: Option<JsWebpEncoderOptions>,
+    ) -> AsyncTask<AsyncGetBuffer> {
         AsyncTask::new(AsyncGetBuffer {
             encoder: self.encoder.clone(),
+            options: options.map(map_js_webp_encoder_options),
         })
     }
 
     #[napi]
-    pub fn get_buffer_sync(&self) -> Result<Buffer> {
-        Ok(self.encoder.get_buffer()?.into())
+    pub fn get_buffer_sync(
+        &self,
+        #[napi(ts_arg_type = "JsWebpEncoderOptions")] options: Option<JsWebpEncoderOptions>,
+    ) -> Result<Buffer> {
+        Ok(self
+            .encoder
+            .get_buffer(options.map(map_js_webp_encoder_options))?
+            .into())
     }
 
     #[napi]
-    pub fn write_to_file(&self, path: String) -> AsyncTask<AsyncWriteToFile> {
+    pub fn write_to_file(
+        &self,
+        path: String,
+        #[napi(ts_arg_type = "JsWebpEncoderOptions")] options: Option<JsWebpEncoderOptions>,
+    ) -> AsyncTask<AsyncWriteToFile> {
         AsyncTask::new(AsyncWriteToFile {
             encoder: self.encoder.clone(),
             path,
+            options: options.map(map_js_webp_encoder_options),
         })
     }
 
     #[napi]
-    pub fn write_to_file_sync(&self, path: String) -> Result<Buffer> {
-        Ok(self.encoder.write_to_file(path)?.into())
+    pub fn write_to_file_sync(
+        &self,
+        path: String,
+        #[napi(ts_arg_type = "JsWebpEncoderOptions")] options: Option<JsWebpEncoderOptions>,
+    ) -> Result<Buffer> {
+        Ok(self
+            .encoder
+            .write_to_file(path, options.map(map_js_webp_encoder_options))?
+            .into())
     }
 
     #[napi]
@@ -226,24 +225,23 @@ impl JsWebpEncoder {
     pub fn set_dimensions(&mut self, width: u32, height: u32) {
         self.encoder.set_dimensions(width, height);
     }
+}
 
-    #[napi]
-    pub fn set_options(&mut self, options: JsWebpEncoderOptions) {
-        self.encoder.set_options(EncoderOptions {
-            anim_params: AnimParams {
-                loop_count: options.loop_count.unwrap_or(0),
+fn map_js_webp_encoder_options(options: JsWebpEncoderOptions) -> EncoderOptions {
+    EncoderOptions {
+        anim_params: AnimParams {
+            loop_count: options.loop_count.unwrap_or(0),
+        },
+        encoding_config: Some(EncodingConfig {
+            encoding_type: if options.lossless.unwrap_or(true) {
+                webp_animation::EncodingType::Lossless
+            } else {
+                webp_animation::EncodingType::Lossy(LossyEncodingConfig::default())
             },
-            encoding_config: Some(EncodingConfig {
-                encoding_type: if options.lossless.unwrap_or(true) {
-                    webp_animation::EncodingType::Lossless
-                } else {
-                    webp_animation::EncodingType::Lossy(LossyEncodingConfig::default())
-                },
-                quality: options.quality.unwrap_or(1) as f32,
-                method: options.method.unwrap_or(4) as usize,
-            }),
-            ..Default::default()
-        });
+            quality: options.quality.unwrap_or(1) as f32,
+            method: options.method.unwrap_or(4) as usize,
+        }),
+        ..Default::default()
     }
 }
 
diff --git a/test/main.test.mjs b/test/main.test.mjs
@@ -1,14 +1,20 @@
-import fs from 'node:fs/promises'
+// @ts-check
+
 import assert from 'node:assert'
+import fs from 'node:fs/promises'
+import { dirname } from 'node:path'
 import test from 'node:test'
+import { fileURLToPath } from 'node:url'
 import { WebpEncoder } from '../index.js'
 import { Canvas, loadImage } from '@napi-rs/canvas'
 
-async function getFrames(): Promise<Buffer[]> {
-  const frames: Buffer[] = []
+/** @returns {Promise<Buffer[]>} */
+async function getFrames() {
+  /** @type {Buffer[]} */
+  const frames = []
   const ctx = new Canvas(480, 270).getContext('2d')
   for (let i = 1; i <= 17; i++) {
-    const path = `${import.meta.dirname}/fixtures/frames/${i}.png`
+    const path = `${dirname(fileURLToPath(import.meta.url))}/fixtures/frames/${i}.png`
     const file = await fs.readFile(path)
     const img = await loadImage(file)
     ctx.drawImage(img, 0, 0)
@@ -20,13 +26,16 @@ async function getFrames(): Promise<Buffer[]> {
 
 test('encodes webp', async () => {
   const frames = await getFrames()
-  const encoder = new WebpEncoder(480, 270, { lossless: false, quality: 75 })
+  const encoder = new WebpEncoder(480, 270)
 
   encoder.setFrameRate(24)
 
   for (const frame of frames) {
     encoder.addFrame(frame)
   }
-  const data = encoder.writeToFileSync('test/output/test.webp')
+  const data = encoder.writeToFileSync(
+    'test/output/test.webp',
+    { lossless: false, quality: 75 }
+  )
   assert.strictEqual(data.length, 178436)
 })
