feat(directus): add Sharp transforms & key modifier.#2207
feat(directus): add Sharp transforms & key modifier.#2207
key modifier.#2207Conversation
Intentionally omitting Resizing operations at the moment due to coverage in Nuxt/Image and abstraction available to Directus. TODO: Fix duplication of transform object.
Updated Directus playground/e2e_snapshots to use the Directus Sandbox.
…orms Fixes nuxt#2194 * Implements the ability to use a `key` to use a preconfigured Directus Transform set. * Adds Types for Sharp Transforms [intentionally omitted resizing since the basic functionality is abstracted in the nuxt/image module] * Fixes issue of incorrect typing for transforms to allow for `[string, ...any[]]` (was incorrectly typed as `string[]`)
Whitespace was causing tests to fail;
* Sort import for `providers.test.ts` (NOTE: PR nuxt#2193 greatly improves test and readability) * Add E2E snapshot for `key` modifier. * Fix missing test in `providers.test.ts` * Add `key` test for `providers.test.ts`
Updated Docs to include: * `key` :modifier * Explain Sharp Transforms and provide examples.
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2207 +/- ##
=======================================
Coverage 32.52% 32.52%
=======================================
Files 7 7
Lines 372 372
Branches 131 131
=======================================
Hits 121 121
Misses 194 194
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I don't understand why the tests specifically care about the order, nor did I do extensive research on why I needed to put the requests in a different order than the sources for a successful test. If I'm hiding a problem here, I'd like to know about it but as far as I can tell the order has no meaning outside of the test.
Remove unnecessary comments
kheiner
left a comment
There was a problem hiding this comment.
Looking forward to the feedback.
📝 WalkthroughWalkthroughThis pull request implements typed Sharp operation support and keyed transform functionality for the Directus image provider. The core provider implementation adds type-safe Sharp operation definitions, supports mutually-exclusive keyed transforms via Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/runtime/providers/directus.ts (1)
102-114: Transforms deduplication approach works but has a minor inefficiency.The JSON stringify/parse approach for deduplication is functional but performs redundant parsing. Consider using a
Mapor filtering duplicates in a single pass for better performance, though this is a minor concern given typical transform array sizes.♻️ Alternative deduplication approach
const operationsGenerator = createOperationsGenerator({ valueMap: { transforms(value: SharpOperation[]) { - return value.length > 0 - ? JSON.stringify( - Array.from(new Set(value.map(v => JSON.stringify(v)))) - .map(v => JSON.parse(v)), - ) - : undefined + if (value.length === 0) return undefined + const seen = new Set<string>() + const unique = value.filter((v) => { + const key = JSON.stringify(v) + if (seen.has(key)) return false + seen.add(key) + return true + }) + return JSON.stringify(unique) }, }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/directus.ts` around lines 102 - 114, The transforms valueMap currently deduplicates by JSON.stringify/parse which works but does extra parsing; update the transforms function inside createOperationsGenerator (the operationsGenerator/valueMap/transforms handler for SharpOperation[]) to deduplicate in a single pass (e.g., use a Map or Set of serialized entries to filter duplicates while building the resulting array) and then JSON.stringify that deduplicated array (return undefined for empty), keeping the same return shape and types as before.docs/content/3.providers/directus.md (1)
41-41: Minor: Use hyphen for compound adjective.For grammatical correctness, "Directus specific" should be "Directus-specific" when used as a compound adjective.
📝 Proposed fix
-The `modifiers` object is used for Directus specific features. All modifiers are optional. +The `modifiers` object is used for Directus-specific features. All modifiers are optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/3.providers/directus.md` at line 41, Update the sentence describing the "modifiers" object to use a hyphenated compound adjective: change "Directus specific features" to "Directus-specific features" so the phrase correctly reads "The `modifiers` object is used for Directus-specific features. All modifiers are optional." Ensure the `modifiers` symbol remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/content/3.providers/directus.md`:
- Line 41: Update the sentence describing the "modifiers" object to use a
hyphenated compound adjective: change "Directus specific features" to
"Directus-specific features" so the phrase correctly reads "The `modifiers`
object is used for Directus-specific features. All modifiers are optional."
Ensure the `modifiers` symbol remains unchanged.
In `@src/runtime/providers/directus.ts`:
- Around line 102-114: The transforms valueMap currently deduplicates by
JSON.stringify/parse which works but does extra parsing; update the transforms
function inside createOperationsGenerator (the
operationsGenerator/valueMap/transforms handler for SharpOperation[]) to
deduplicate in a single pass (e.g., use a Map or Set of serialized entries to
filter duplicates while building the resulting array) and then JSON.stringify
that deduplicated array (return undefined for empty), keeping the same return
shape and types as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c8abb98-16cf-45a3-8906-78b5a8da1ec5
📒 Files selected for processing (7)
docs/content/3.providers/directus.mdplayground/app/providers.tsplayground/nuxt.config.tssrc/runtime/providers/directus.tstest/e2e/__snapshots__/directus.json5test/nuxt/providers.test.tstest/providers.ts
key modifier.
🔗 Linked issue
fixes #2194
📚 Description
keystring, stripping all other transforms supported by Nuxt/Imagekeyif present is not paired with other modifiers.keys in the public space for better explanation to developers.string[]which is not valid. Transforms should fallback to a tuple of type[string, ...any[]][]to prevent errors with using transforms not typed explicitly. (Future proofing for new transforms added to Sharp & supported by Directus.)keymodifier.