Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-vta-frame-size-and-zoom-zero.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@imgproxy/imgproxy-js-core": patch
---

Fix `video_thumbnail_animation` generating URLs containing the literal string `undefined` when `frame_width` or `frame_height` was omitted. Both arguments are required by imgproxy and by the option's type, so they are now validated like `step`, `delay` and `frames`, and a missing value raises an error instead of producing a broken URL.

Fix `zoom` silently ignoring a `zoom` value of `0`. The option was dropped before validation, so no error was raised. imgproxy requires zoom factors to be greater than `0`, so `0` is now rejected for `zoom`, `zoom_x` and `zoom_y`. Note that this changes the error message for non-positive values from "can't be less than 0" to "can't be less or equal than 0".
9 changes: 2 additions & 7 deletions src/options/videoThumbnailAnimation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@ function build(options: VideoThumbnailAnimationOptionsPartial) {
guardIsNotNum(vta.delay, "video_thumbnail_animation.delay");
guardIsNotNum(vta.frames, "video_thumbnail_animation.frames");

if (vta.frame_width !== undefined) {
guardIsNotNum(vta.frame_width, "video_thumbnail_animation.frame_width");
}

if (vta.frame_height !== undefined) {
guardIsNotNum(vta.frame_height, "video_thumbnail_animation.frame_height");
}
guardIsNotNum(vta.frame_width, "video_thumbnail_animation.frame_width");
guardIsNotNum(vta.frame_height, "video_thumbnail_animation.frame_height");

const parts = [];

Expand Down
7 changes: 4 additions & 3 deletions src/options/zoom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ import { guardIsUndef, guardIsNotNum } from "../utils";

const validateValue = (value: number, optName: string): void => {
guardIsUndef(value, optName);
guardIsNotNum(value, optName, { addParam: { min: 0 } });
guardIsNotNum(value, optName, { addParam: { min: 0, minEqual: true } });
};

const getOpt = (options: ZoomOptionsPartial): Zoom | undefined =>
options.zoom || options.z;
options.zoom ?? options.z;

const test = (options: ZoomOptionsPartial): boolean => Boolean(getOpt(options));
const test = (options: ZoomOptionsPartial): boolean =>
getOpt(options) !== undefined;

const build = (options: ZoomOptionsPartial): string => {
const zoomOpts = getOpt(options);
Expand Down
16 changes: 16 additions & 0 deletions tests/optionsBasic/videoThumbnailAnimation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,20 @@ describe("Check `video_thumbnail_animation` type declarations", () => {
},
});
});

describe("build (required frame size)", () => {
it("should throw an error if frame_width is undefined", () => {
expect(() =>
// @ts-expect-error: Let's ignore an error (check for users with vanilla js).
build({ vta: { step: 10, delay: 100, frames: 5, frame_height: 240 } })
).toThrow("video_thumbnail_animation.frame_width is not a number");
});

it("should throw an error if frame_height is undefined", () => {
expect(() =>
// @ts-expect-error: Let's ignore an error (check for users with vanilla js).
build({ vta: { step: 10, delay: 100, frames: 5, frame_width: 320 } })
).toThrow("video_thumbnail_animation.frame_height is not a number");
});
});
});
20 changes: 18 additions & 2 deletions tests/optionsBasic/zoom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ describe("zoom", () => {
expect(test({ zoom: 1 })).toEqual(true);
});

it("should return true if zoom option is 0", () => {
expect(test({ zoom: 0 })).toEqual(true);
});

it("should return false if zoom option is undefined", () => {
expect(test({})).toEqual(false);
});
Expand All @@ -30,9 +34,21 @@ describe("zoom", () => {
expect(() => build({ zoom: "1" })).toThrow("zoom option is not a number");
});

it("should throw an error if zoom is 0", () => {
expect(() => build({ zoom: 0 })).toThrow(
"zoom option value can't be less or equal than 0"
);
});

it("should throw an error if zoom_x is 0", () => {
expect(() => build({ zoom: { zoom_x: 0, zoom_y: 0.5 } })).toThrow(
"zoom.zoom_x value can't be less or equal than 0"
);
});

it("should throw an error if zoom is less than 0", () => {
expect(() => build({ z: -1 })).toThrow(
"zoom option value can't be less than 0"
"zoom option value can't be less or equal than 0"
);
});

Expand All @@ -45,7 +61,7 @@ describe("zoom", () => {

it("should throw an error if zoom_x is less than 0", () => {
expect(() => build({ zoom: { zoom_x: -1, zoom_y: 0.5 } })).toThrow(
"zoom.zoom_x value can't be less than 0"
"zoom.zoom_x value can't be less or equal than 0"
);
});

Expand Down
Loading