feat: Add Klipy as a GIF provider alongside Tenor#1932
Conversation
ref https://linear.app/ghost/project/replace-tenor-with-klipy-for-gif-cards-c48db5d2a3d5/overview - Google is shutting down the Tenor API on 2026-06-30, so the GIF card needs a replacement provider - makes the GIF client provider-aware: host and API key now come from card config via getGifProviderConfig(), resolving Klipy when configured and falling back to Tenor - Klipy exposes a Tenor-compatible search API, so the success path is unchanged; extractErrorMessage() handles Klipy's different error shape - fixes media_filter to request tinygif,gif explicitly - updates the selector placeholder and error copy to be provider-neutral - adds unit tests and e2e coverage for the Klipy path
ref https://linear.app/ghost/project/replace-tenor-with-klipy-for-gif-cards-c48db5d2a3d5/overview - the editor GIF card was named after Tenor throughout, but it now supports multiple providers (Tenor and Klipy) - renamed the files, components, commands and DOM attribute from Tenor* to Gif*: tenor.js -> gif.js, TenorPlugin -> GifPlugin, TenorSelector -> GifSelector, useTenor -> useGif, the OPEN/INSERT_FROM_TENOR commands -> _GIF, data-tenor-index -> data-gif-index - pure rename: Tenor-the-provider references (config.tenor, provider: 'tenor', the Tenor-specific tests) are unchanged
ref https://linear.app/ghost/project/replace-tenor-with-klipy-for-gif-cards-c48db5d2a3d5/overview - loadNextPage passed the searchTerm useRef object instead of searchTerm.current, so page 2+ of a GIF search sent q=[object Object] and returned junk results - pre-existing bug carried over from the original tenor.js, surfaced during PR review - first-page search and trending pagination were unaffected
ref https://linear.app/ghost/project/replace-tenor-with-klipy-for-gif-cards-c48db5d2a3d5/overview - extracted invalid-key detection into an exported isInvalidKeyError() function, consistent with the already-extracted extractErrorMessage(), so the classification is unit-testable - added tests for invalid-key detection across both providers' verified wording, a non-array Klipy error message, and provider fallback when the Klipy config has no key - no behaviour change; the classification logic is identical
Injects VITE_KLIPY_API_KEY into the demo build from the new KLIPY_API_KEY repo secret so the deployed editor at koenig.ghost.org uses Klipy as the GIF provider, with Tenor as the fallback.
WalkthroughThis PR extends the Koenig editor's GIF card to support a new Klipy GIF provider alongside the existing Tenor provider. The refactoring consolidates provider-specific logic into reusable utilities, updates the GIF service layer to dynamically select between Klipy and Tenor based on configuration, renames and extends the selector component to be provider-agnostic, updates plugin commands and menu integration to reference the new GIF abstractions, and adds comprehensive test coverage for both providers. The component hierarchy remains unchanged; only the underlying provider selection and API integration have been generalized. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 83-84: Update the environment variable descriptions for
VITE_KLIPY_API_KEY and VITE_TENOR_API_KEY to use the acronym “GIF” (uppercase)
instead of “Gif”; locate the lines containing the text for `VITE_KLIPY_API_KEY`
and `VITE_TENOR_API_KEY` in the CLAUDE.md content and replace “Gif card
functionality” with “GIF card functionality” (and similarly update the
parenthetical notes such as “fallback when Klipy key is not set”) to maintain
consistent uppercase acronym usage.
In `@packages/koenig-lexical/README.md`:
- Around line 36-40: The fenced code block containing the environment variables
(the block with VITE_KLIPY_API_KEY and VITE_TENOR_API_KEY) is missing a language
tag which triggers markdownlint MD040; update that fenced block to include a
language tag (for example "bash") right after the opening backticks so the block
becomes ```bash ... ``` to satisfy the linter.
In `@packages/koenig-lexical/src/nodes/ImageNode.jsx`:
- Line 61: Update the visibility predicate for the GIF menu item so it checks
actual provider API keys instead of just provider objects: replace the isHidden
function used in ImageNode.jsx (isHidden: ({config}) => ...) to return true when
neither config.klipy.apiKey nor config.tenor.googleApiKey are present (i.e.,
hide when both keys are missing) so the GIF menu is only shown when a usable
provider key exists.
In `@packages/koenig-lexical/src/utils/services/gif.js`:
- Around line 214-216: In checkStatus, don't access response.headers.map
(Headers is a Headers object) — instead read the content type via
response.headers.get('content-type') (guarding for null/undefined and
normalizing case) and use that value for checks; e.g., set const contentType =
(response.headers.get('content-type') || '').toLowerCase() and replace uses of
response.headers.map['content-type'] with contentType, then call
.startsWith('application/json') or compare to 'text/xml' as before so error
parsing (response.json(), response.text(), etc.) works reliably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93f1dcf7-4249-4621-9ddb-2e16ca0fc6df
📒 Files selected for processing (22)
.github/workflows/pages.ymlCLAUDE.mdpackages/koenig-lexical/README.mdpackages/koenig-lexical/demo/DemoApp.jsxpackages/koenig-lexical/demo/HtmlOutputDemo.jsxpackages/koenig-lexical/demo/RestrictedContentDemo.jsxpackages/koenig-lexical/demo/utils/gifConfig.jspackages/koenig-lexical/demo/utils/tenorConfig.jspackages/koenig-lexical/src/components/ui/GifPlugin.jsxpackages/koenig-lexical/src/components/ui/GifSelector.jsxpackages/koenig-lexical/src/components/ui/GifSelector.stories.jsxpackages/koenig-lexical/src/components/ui/file-selectors/Gif/Error.jsxpackages/koenig-lexical/src/components/ui/file-selectors/Gif/Gif.jsxpackages/koenig-lexical/src/components/ui/file-selectors/Gif/Loader.jsxpackages/koenig-lexical/src/components/ui/file-selectors/Tenor/Error.jsxpackages/koenig-lexical/src/nodes/ImageNode.jsxpackages/koenig-lexical/src/plugins/AllDefaultPlugins.jsxpackages/koenig-lexical/src/plugins/KoenigSelectorPlugin.jsxpackages/koenig-lexical/src/utils/buildCardMenu.jspackages/koenig-lexical/src/utils/services/gif.jspackages/koenig-lexical/test/e2e/cards/image-card.test.jspackages/koenig-lexical/test/unit/utils/gif-provider.test.js
💤 Files with no reviewable changes (2)
- packages/koenig-lexical/src/components/ui/file-selectors/Tenor/Error.jsx
- packages/koenig-lexical/demo/utils/tenorConfig.js
| priority: 17, | ||
| queryParams: ['src'], | ||
| isHidden: ({config}) => !config?.tenor, | ||
| isHidden: ({config}) => !config?.tenor && !config?.klipy, |
There was a problem hiding this comment.
Hide GIF menu item when no valid provider key is configured.
Line 61 gates visibility by provider object presence, but provider resolution requires config.klipy.apiKey or config.tenor.googleApiKey. This can expose a GIF menu entry that opens with no usable provider config.
Suggested fix
- isHidden: ({config}) => !config?.tenor && !config?.klipy,
+ isHidden: ({config}) => !(config?.klipy?.apiKey || config?.tenor?.googleApiKey),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| isHidden: ({config}) => !config?.tenor && !config?.klipy, | |
| isHidden: ({config}) => !(config?.klipy?.apiKey || config?.tenor?.googleApiKey), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/koenig-lexical/src/nodes/ImageNode.jsx` at line 61, Update the
visibility predicate for the GIF menu item so it checks actual provider API keys
instead of just provider objects: replace the isHidden function used in
ImageNode.jsx (isHidden: ({config}) => ...) to return true when neither
config.klipy.apiKey nor config.tenor.googleApiKey are present (i.e., hide when
both keys are missing) so the GIF menu is only shown when a usable provider key
exists.
There was a problem hiding this comment.
This is a pre-existing pattern — the previous line isHidden: ({config}) => !config?.tenor already gated on object presence, not key presence, and this PR extends the same pattern to klipy.
In the real Ghost ↔ Koenig contract, the consumer nullifies the provider object when no key is set:
ghost/admin/app/components/koenig-lexical-editor.js:klipy: this.config.klipy?.apiKey ? this.config.klipy : nullapps/admin-x-settings/.../member-email-editor.tsx:const klipyConfig = config.klipy?.apiKey ? config.klipy : null;
(Tenor follows the same pattern, long-standing.) So in practice the object-presence check is equivalent to the key-presence check. The defensive-by-apiKey variant is a reasonable tightening for callers that don't follow the contract, but it's a behaviour change beyond the scope of this migration.
There was a problem hiding this comment.
@renatoworks, that's a clear and well-reasoned explanation — the contract-level nullification makes the two checks equivalent in practice, and my suggestion would have introduced an unnecessary scope creep in this migration PR. I'll withdraw it.
✏️ Learnings added
Learnt from: renatoworks
Repo: TryGhost/Koenig PR: 1932
File: packages/koenig-lexical/src/nodes/ImageNode.jsx:61-61
Timestamp: 2026-05-25T18:46:42.200Z
Learning: In the Ghost ↔ Koenig integration, GIF provider config objects (`config.tenor`, `config.klipy`) are nullified by consumers when no API key is set (e.g., `klipy: this.config.klipy?.apiKey ? this.config.klipy : null`). Therefore, gating on object presence (e.g., `!config?.tenor && !config?.klipy`) is equivalent to gating on key presence. This is a long-standing pattern and checking for specific key fields (e.g., `config?.klipy?.apiKey`) would be an unnecessary tightening that goes beyond the established contract. See `packages/koenig-lexical/src/nodes/ImageNode.jsx` `isHidden` predicate for the GIF menu item.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| if (response.headers.map['content-type'].startsWith('application/json')) { | ||
| responseText = await response.json().then(json => json.error.message || json.error); | ||
| responseText = await response.json().then(json => extractErrorMessage(json)); | ||
| } else if (response.headers.map['content-type'] === 'text/xml') { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify content-type header access style in GIF service.
# Expected after fix:
# 1) No matches for `headers.map[...]`
# 2) At least one match for `headers.get('content-type')`
rg -n "response\.headers\.map\['content-type'\]" packages/koenig-lexical/src/utils/services/gif.js
rg -n "response\.headers\.get\('content-type'\)" packages/koenig-lexical/src/utils/services/gif.jsRepository: TryGhost/Koenig
Length of output: 224
Fix Fetch Headers access in checkStatus
In packages/koenig-lexical/src/utils/services/gif.js, checkStatus reads response.headers.map['content-type'] (lines 214-216). Fetch response.headers is a Headers object and doesn’t expose .map; this can throw during non-2xx handling and prevent provider error parsing/classification.
Proposed fix
- if (response.headers.map['content-type'].startsWith('application/json')) {
+ const contentType = response.headers.get('content-type') || '';
+ if (contentType.startsWith('application/json')) {
responseText = await response.json().then(json => extractErrorMessage(json));
- } else if (response.headers.map['content-type'] === 'text/xml') {
+ } else if (contentType.includes('text/xml')) {
responseText = await response.text();
+ } else {
+ responseText = response.statusText || 'Request failed';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response.headers.map['content-type'].startsWith('application/json')) { | |
| responseText = await response.json().then(json => json.error.message || json.error); | |
| responseText = await response.json().then(json => extractErrorMessage(json)); | |
| } else if (response.headers.map['content-type'] === 'text/xml') { | |
| const contentType = response.headers.get('content-type') || ''; | |
| if (contentType.startsWith('application/json')) { | |
| responseText = await response.json().then(json => extractErrorMessage(json)); | |
| } else if (contentType.includes('text/xml')) { | |
| responseText = await response.text(); | |
| } else { | |
| responseText = response.statusText || 'Request failed'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/koenig-lexical/src/utils/services/gif.js` around lines 214 - 216, In
checkStatus, don't access response.headers.map (Headers is a Headers object) —
instead read the content type via response.headers.get('content-type') (guarding
for null/undefined and normalizing case) and use that value for checks; e.g.,
set const contentType = (response.headers.get('content-type') ||
'').toLowerCase() and replace uses of response.headers.map['content-type'] with
contentType, then call .startsWith('application/json') or compare to 'text/xml'
as before so error parsing (response.json(), response.text(), etc.) works
reliably.
There was a problem hiding this comment.
Acknowledged. This is pre-existing tech debt called out in the PR description's "Technical debt" section, carried over verbatim from the old tenor.js. It works today because Ghost loads the whatwg-fetch polyfill (whose Headers implementation exposes .map), and has been working in production this way for years.
On a native Response the access would throw, but the catch in makeRequest would still surface an error to the user (just a generic "trouble reaching the service" rather than the specific API error). Not in scope for this 1:1 migration; flagging as something to revisit if Ghost's polyfill strategy changes.
There was a problem hiding this comment.
@renatoworks, understood — the context around the whatwg-fetch polyfill clarifies why this has been stable in production. The catch in makeRequest does provide a safe fallback either way.
Would you like me to open a follow-up GitHub issue to track this so it doesn't get lost when Ghost's polyfill strategy is eventually revisited?
✏️ Learnings added
Learnt from: renatoworks
Repo: TryGhost/Koenig PR: 1932
File: packages/koenig-lexical/src/utils/services/gif.js:214-216
Timestamp: 2026-05-25T18:49:00.766Z
Learning: In `packages/koenig-lexical/src/utils/services/gif.js`, the `checkStatus` function accesses `response.headers.map['content-type']` rather than `response.headers.get('content-type')`. This is pre-existing tech debt carried over verbatim from the old `tenor.js`. It works in the Ghost environment because Ghost loads the whatwg-fetch polyfill, whose `Headers` implementation exposes `.map`. On a native `Response` (Fetch API) this would throw, but the `catch` in `makeRequest` still surfaces a generic error to the user. This was intentionally left as-is in the Tenor→Klipy migration PR `#1932` and flagged for future cleanup if Ghost's polyfill strategy changes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
ref https://linear.app/ghost/project/replace-tenor-with-klipy-for-gif-cards-c48db5d2a3d5/overview Ghost-side config plumbing for Klipy as a GIF provider, ahead of the Tenor API shutdown on 2026-06-30. Non-breaking: without a Klipy key configured, the editor's GIF card behaves identically to today (Tenor as before). Klipy is used once the `koenig-lexical` dependency is bumped to a version that supports it (a separate small follow-up, Renovate's auto-PR or a tiny manual bump). * exposes a `klipy` config block (apiKey, contentFilter) through defaults, public-config and the config serializer, mirroring `tenor` * forwards `klipy` into the editor card config in both the Ember post editor and the admin-x welcome-email editor * adds the optional `klipy` field to the admin-x-framework `Config` type and the test fixture * removes the orphaned ghost/admin/app/services/tenor.js (pre-Lexical GIF client, zero injection sites) * regenerates the config API e2e snapshot to include the new `klipy` block Paired with TryGhost/Koenig#1932. Google has confirmed that media.tenor.com will continue serving existing asset URLs to third-party requests after the API shutdown, so existing embedded GIFs in published content remain safe; no Ghost(Pro) CDN re-host is required.
2 participants
Describe the problem you are solving
This PR adds Klipy as a GIF provider in the editor's GIF card, alongside the existing Tenor provider. Google is shutting down the Tenor API on 2026-06-30, so a replacement is needed.
Klipy exposes a Tenor-compatible API: same
/v2/searchand/v2/featuredendpoints and the samemedia_formats/results/nextresponse shape, so one client serves either provider. The error response shape differs (Tenor returns{error:{message}}, Klipy returns{result:false, errors:{message:[...]}}), and that is handled in the error parser.No breaking changes. Provider resolution prefers Klipy when a Klipy key is configured, otherwise it falls back to Tenor exactly as today. Deployments without a Klipy key behave identically to before. Tenor stays in place; removing it entirely is a separate later phase after the API shutdown.
This PR is paired with the Ghost-side config plumbing (TryGhost/Ghost#28109), but each PR is independently safe to merge: the two are backward-compatible in every direction. The Ghost catalog pin bump that lets Ghost actually consume this version is a separate, small dependency PR (Renovate's auto-PR, or a tiny manual PR), not coupled to either of these.
Changelog for devs
getGifProviderConfig(cardConfig)(Klipy preferred when configured, Tenor as fallback,nullwhen neither is set)extractErrorMessageandisInvalidKeyErrorpure functions so error parsing and invalid-key classification are unit-testable across both providers' verified wordingmedia_filterfrom'minimal'to'tinygif,gif'data-tenor-indexDOM attribute toGif*/data-gif-indexso the card is provider-agnosticconfig.tenor,provider: 'tenor', Tenor-specific tests, the "Search Tenor for GIFs" placeholder) intentionallyAPI_URL; the base URL and API key now come from card configloadNextPagewas passing thesearchTermuseRefobject instead ofsearchTerm.current, so page 2+ of a search sentq=[object Object]and returned junk results. Pre-existing intenor.js, found during the rename.VITE_KLIPY_API_KEYin the READMEVITE_KLIPY_API_KEYinto the demo deployment workflow (.github/workflows/pages.yml) from a newKLIPY_API_KEYrepo secret so the editor demo at koenig.ghost.org runs against KlipyCLAUDE.mdChangelog for customers
Notes
getGifProviderConfig, the Tenor-specific tests, the "Search Tenor for GIFs" placeholder, and theconfig.tenorgating. Until then, Tenor stays as the fallback so anyone without a Klipy key keeps working.tenor.js, silently broken in production for anyone scrolling past the first page of GIF search results. It is included here as a small drive-by because the file was being renamed and touched anyway; worth highlighting since it would otherwise be invisible inside the bigger feature commit when this PR is squash-merged.{"result":false,"errors":{"message":["The provided API key is invalid: ..."]}}); the regex inisInvalidKeyErrormatches that wording, so a misconfigured Klipy key surfaces the correctINVALID_API_KEYUI rather than a generic "connection" error.pnpm.overrideslink:setup. All four scenarios were verified manually in the local Ghost editor: Klipy provider, Tenor provider, Tenor fallback when no Klipy key is set, and no GIF card when neither is configured.VITE_KLIPY_API_KEYinpackages/koenig-lexical/.env.localand runningyarn dev.Technical debt
checkStatusreadsresponse.headers.map['content-type']rather than the standardresponse.headers.get('content-type'). This relies on whatwg-fetch polyfill internals and would throw on a nativeResponse. Pre-existing, not introduced by this PR, kept for a 1:1 migration; worth addressing separately.PR Scope (mark all that apply)
Checklist before requesting a review (mark all that apply)
Merge instructions