Fix image url error when missing crop or hotspot (#923)

* [image-url] Upgrade to latest jest * [image-url] Add failing test and use snapshots instead * [image-url] Fix error caused by missing crop or hotspot
sanity-io · Aug 20, 2018 · 0f66b31 · 0f66b31
1 parent 85cb8ae
commit 0f66b31
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 41 deletions.
diff --git a/packages/@sanity/image-url/package.json b/packages/@sanity/image-url/package.json
@@ -25,7 +25,7 @@
     "babel-cli": "^6.26.0",
     "browserify": "^14.3.0",
     "envify": "^4.0.0",
-    "jest": "^22.4.3",
+    "jest": "^23.5.0",
     "rimraf": "^2.6.2",
     "uglify-js": "^3.1.10",
     "uglifyify": "^3.0.4"

diff --git a/packages/@sanity/image-url/src/parseSource.js b/packages/@sanity/image-url/src/parseSource.js
@@ -43,12 +43,14 @@ export default function parseSource(source) {
     return null
   }
 
-  if (source && (source.crop || source.hotspot)) {
+  if (source.crop) {
     image.crop = source.crop
+  }
+  if (source.hotspot) {
     image.hotspot = source.hotspot
   }
 
-  return applyDefaultHotspot(image)
+  return applyDefaults(image)
 }
 
 function isUrl(url) {
@@ -61,7 +63,7 @@ function urlToId(url) {
 }
 
 // Mock crop and hotspot if image lacks it
-function applyDefaultHotspot(image) {
+function applyDefaults(image) {
   if (image.crop && image.hotspot) {
     return image
   }

diff --git a/packages/@sanity/image-url/test/__snapshots__/builder.test.js.snap b/packages/@sanity/image-url/test/__snapshots__/builder.test.js.snap
@@ -0,0 +1,29 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`builder all hotspot/crop-compatible params 1`] = `"rect=200,300,1600,2400&flip=hv&fm=png&dl=a.png&blur=50&sharp=7&invert=true&or=90&min-h=150&max-h=300&min-w=100&max-w=200&q=50&fit=crop"`;
+
+exports[`builder all params 1`] = `"rect=10,20,30,40&fp-x=10&fp-x=20&flip=hv&fm=png&dl=a.png&blur=50&invert=true&or=90&min-h=150&max-h=300&min-w=100&max-w=200&q=50&fit=crop&crop=center"`;
+
+exports[`builder can be told to ignore hotspot 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80"`;
+
+exports[`builder can specify options with url params 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?rect=200,300,1600,2400&w=320&h=240"`;
+
+exports[`builder constrains aspect ratio 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?rect=200,300,1600,1280&w=100&h=80"`;
+
+exports[`builder does crop image with no crop/hotspot specified if aspect ratio is forced 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/vK7bXJPEjVpL_C950gH1N73Zv14r7pYsbUdXl-4288x2848.jpg?rect=720,0,2848,2848&w=80&h=80"`;
+
+exports[`builder does not crop image with no crop/hotspot specified 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/vK7bXJPEjVpL_C950gH1N73Zv14r7pYsbUdXl-4288x2848.jpg?w=80"`;
+
+exports[`builder flip horizontal 1`] = `"flip=h"`;
+
+exports[`builder flip vertical 1`] = `"flip=v"`;
+
+exports[`builder handles crop but no hotspot 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?rect=200,300,1600,2400"`;
+
+exports[`builder handles hotspot but no crop 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg"`;
+
+exports[`builder skips hotspot/crop if crop mode specified 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80&crop=center"`;
+
+exports[`builder skips hotspot/crop if focal point specified 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?fp-x=10&fp-x=20&w=100&h=80"`;
+
+exports[`builder toString() aliases url() 1`] = `"https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80"`;
diff --git a/packages/@sanity/image-url/test/builder.test.js b/packages/@sanity/image-url/test/builder.test.js
@@ -1,5 +1,5 @@
 import sanityImage from '../src/builder'
-import {imageWithNoCropSpecified, noHotspotImage, croppedImage} from './fixtures'
+import {croppedImage, imageWithNoCropSpecified, noHotspotImage} from './fixtures'
 
 const urlFor = sanityImage()
   .projectId('zp7mbokg')
@@ -10,14 +10,48 @@ function stripPath(url) {
 }
 
 const cases = [
+  {
+    name: 'handles hotspot but no crop',
+    url: urlFor
+      .image({
+        _type: 'image',
+        asset: {
+          _ref: 'image-Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000-jpg',
+          _type: 'reference'
+        },
+        hotspot: {
+          height: 0.3,
+          width: 0.3,
+          x: 0.3,
+          y: 0.3
+        }
+      })
+      .url()
+  },
+  {
+    name: 'handles crop but no hotspot',
+    url: urlFor
+      .image({
+        _type: 'image',
+        asset: {
+          _ref: 'image-Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000-jpg',
+          _type: 'reference'
+        },
+        crop: {
+          bottom: 0.1,
+          left: 0.1,
+          right: 0.1,
+          top: 0.1
+        }
+      })
+      .url()
+  },
   {
     name: 'constrains aspect ratio',
     url: urlFor
       .image(croppedImage())
       .size(100, 80)
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?rect=200,300,1600,1280&w=100&h=80'
+      .url()
   },
 
   {
@@ -26,9 +60,7 @@ const cases = [
       .image(croppedImage())
       .ignoreImageParams()
       .size(100, 80)
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80'
+      .url()
   },
 
   {
@@ -37,9 +69,7 @@ const cases = [
       .image(croppedImage())
       .ignoreImageParams()
       .size(100, 80)
-      .toString(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80'
+      .toString()
   },
 
   {
@@ -48,9 +78,7 @@ const cases = [
       .image(croppedImage())
       .size(100, 80)
       .crop('center')
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?w=100&h=80&crop=center'
+      .url()
   },
 
   {
@@ -59,19 +87,15 @@ const cases = [
       .image(croppedImage())
       .size(100, 80)
       .focalPoint(10, 20)
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?fp-x=10&fp-x=20&w=100&h=80'
+      .url()
   },
 
   {
     name: 'does not crop image with no crop/hotspot specified',
     url: urlFor
       .image(imageWithNoCropSpecified())
       .width(80)
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/vK7bXJPEjVpL_C950gH1N73Zv14r7pYsbUdXl-4288x2848.jpg?w=80'
+      .url()
   },
 
   {
@@ -80,19 +104,15 @@ const cases = [
       .image(imageWithNoCropSpecified())
       .width(80)
       .height(80)
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/vK7bXJPEjVpL_C950gH1N73Zv14r7pYsbUdXl-4288x2848.jpg?rect=720,0,2848,2848&w=80&h=80'
+      .url()
   },
 
   {
     name: 'can specify options with url params',
     url: urlFor
       .image(croppedImage())
       .withOptions({w: 320, h: 240})
-      .url(),
-    expect:
-      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?rect=200,300,1600,2400&w=320&h=240'
+      .url()
   },
 
   {
@@ -102,8 +122,7 @@ const cases = [
         .image(noHotspotImage())
         .flipHorizontal()
         .url()
-    ),
-    expect: 'flip=h'
+    )
   },
 
   {
@@ -113,8 +132,7 @@ const cases = [
         .image(noHotspotImage())
         .flipVertical()
         .url()
-    ),
-    expect: 'flip=v'
+    )
   },
 
   {
@@ -137,10 +155,8 @@ const cases = [
         .flipVertical()
         .fit('crop')
         .url()
-    ),
+    )
     // eslint-disable-next-line max-len
-    expect:
-      'rect=200,300,1600,2400&flip=hv&fm=png&dl=a.png&blur=50&sharp=7&invert=true&or=90&min-h=150&max-h=300&min-w=100&max-w=200&q=50&fit=crop'
   },
 
   {
@@ -165,17 +181,15 @@ const cases = [
         .fit('crop')
         .crop('center')
         .url()
-    ),
+    )
     // eslint-disable-next-line max-len
-    expect:
-      'rect=10,20,30,40&fp-x=10&fp-x=20&flip=hv&fm=png&dl=a.png&blur=50&invert=true&or=90&min-h=150&max-h=300&min-w=100&max-w=200&q=50&fit=crop&crop=center'
   }
 ]
 
 describe('builder', () => {
   cases.forEach(testCase => {
     test(testCase.name, () => {
-      expect(testCase.url).toBe(testCase.expect)
+      expect(testCase.url).toMatchSnapshot()
     })
   })
 

diff --git a/packages/@sanity/image-url/test/urlForHotspotImage.test.js b/packages/@sanity/image-url/test/urlForHotspotImage.test.js
@@ -1,5 +1,10 @@
 import urlForHotspotImage from '../src/urlForImage'
-import {uncroppedImage, croppedImage, noHotspotImage, materializedAssetWithCrop} from './fixtures'
+import {
+  uncroppedImage,
+  croppedImage,
+  noHotspotImage,
+  materializedAssetWithCrop
+} from './fixtures'
 
 describe('urlForHotspotImage', () => {
   test('does not crop when no crop is required', () => {
@@ -128,6 +133,19 @@ describe('urlForHotspotImage', () => {
     )
   })
 
+  test('gracefully handles a non-crop image', () => {
+    expect(
+      urlForHotspotImage({
+        source: noHotspotImage(),
+        projectId: 'zp7mbokg',
+        dataset: 'production',
+        height: 100
+      })
+    ).toBe(
+      'https://cdn.sanity.io/images/zp7mbokg/production/Tb9Ew8CXIwaY6R1kjMvI0uRR-2000x3000.jpg?h=100'
+    )
+  })
+
   test('gracefully handles materialized asset', () => {
     expect(
       urlForHotspotImage({