Skip to content

feat: add Format constants for convert media types, including JXL - #50

Open
Sreini wants to merge 1 commit into
masterfrom
feat/jxl-format
Open

feat: add Format constants for convert media types, including JXL#50
Sreini wants to merge 1 commit into
masterfrom
feat/jxl-format

Conversation

@Sreini

@Sreini Sreini commented Sep 3, 2026

Copy link
Copy Markdown

Adds a Format namespace of media-type constants so the types accepted by
convert are discoverable via autocomplete — including JPEG XL
(image/jxl), which the API already accepts but no client surfaced.

image/jxl is already in the API allowlist (apps/api/src/schemas/optimizations.ts
and apps/opt-api/src/constants/content-types.ts), so this is a client-side
discoverability change only. There is no API-side work outstanding.

Members

WEBP, PNG, JPEG, JPG, AVIF, JXL, ANY (*/*) — mirroring the
API allowlist exactly.

Why string constants and not an enum

The JSON body has to carry the media type verbatim. A language-level enum
serializes as its name or ordinal, not image/jxl. String constants also keep
this fully backward compatible: callers passing raw strings are unaffected.

Verification

⚠️ Not verified locally — no PHP runtime or working Docker daemon on this machine, so neither php -l nor PHPUnit could be run. The three new TinifyFormatTest cases and the syntax of lib/Tinify/Format.php rely on CI. Please confirm CI is green before merging.

Note composer.json requires PHP >=5.3.0, hence class constants rather than an enum (8.1+).

Part of a coordinated change across all six Tinify client libraries, so the
same Format API is available in each.

Adds a Format namespace of media-type string constants so the types
accepted by convert() are discoverable via autocomplete, including
JPEG XL (image/jxl), which the API already accepts.

String constants rather than a language-level enum: the JSON body must
carry the media type verbatim, and an enum would serialize as its name.
Existing callers passing raw strings are unaffected.
@Sreini

Sreini commented Sep 3, 2026

Copy link
Copy Markdown
Author

The phpstan failure here is pre-existing on master, not caused by this PR.

It reports 2 errors in lib/Tinify/Client.php:156-157 (curl_getinfo expects resource, resource given) — a file this PR does not touch.

Verified by pushing a throwaway branch at master's exact tip with zero changes: it produced the identical 2 errors. (Branch deleted again.)

Cause: composer.phpstan.json requires phpstan/phpstan: ^2.1 with no lockfile, so every CI run installs the newest 2.x. The last green run on master was 2026-04-01; a phpstan release since then tightened the curl_getinfo stub. This PR is simply the first run since.

Worth fixing separately — either pin phpstan, commit a composer.phpstan.lock, or add a baseline entry for those two lines.

@Sreini

Sreini commented Sep 3, 2026

Copy link
Copy Markdown
Author

Follow-up: the macOS unit-test failures are also pre-existing, not caused by this PR.

They are cancelled, not failed — killed inside shivammathur/setup-php, before actions/checkout ran. This PR's code was never checked out and no test ever executed in those jobs.

Cause: ci-cd.yaml sets timeout-minutes: 10, and setup-php on macOS-latest exceeds it for PHP 5.6 / 7.4 / 8.0 / 8.5. PHP 5.4 completes in ~82s and passes.

Verified against a throwaway branch at master's exact tip with zero changes (since deleted). Identical outcome, and both cancel at exactly the 10-minute mark:

Branch started cancelled elapsed
master baseline (8.5) 09:11:49 09:21:53 10m04s
this PR (8.5) 09:10:00 09:20:05 10m05s

Same for 5.6, 7.4 and 8.0. ubuntu-latest and windows-latest pass on every PHP version, on both branches.

So this PR has two independent pre-existing CI issues stacked on it — unpinned phpstan (see previous comment) and the macOS setup-php timeout. Both are worth fixing separately; neither reflects the change here. Raising timeout-minutes or caching the macOS PHP builds would address this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant