💥 Rename Ember specific scope option (#631)

This SDK compensates for Ember's testing UI by utilizing a `scope` option to dump the contents of that scoped element into the document body. We semi-recently implemented a new type of `scope` option directly in our API that allows scoping snapshots to specific elements in the DOM. With this feature, the two scope options conflict, and this SDK cannot use the new element scoping feature while the Ember testing UI scope feature exists. Rather than rename the element scoping option for this SDK only, to keep all SDKs consistent we've decided to rename the existing Ember SDK scope option to `emberTestingScope`. Unfortunately, that makes this a breaking change for anybody relying on the old `scope` option behavior. When upgrading, if not renamed, the new `scope` option will take effect and any cropping for that scoped element will be done by our infrastructure and might have unexpected results.
percy · Aug 23, 2022 · 3e01725 · 3e01725
1 parent 03fe3e3
commit 3e01725
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 9 deletions.
diff --git a/addon-test-support/@percy/ember/index.js b/addon-test-support/@percy/ember/index.js
@@ -26,11 +26,10 @@ function generateName(assertOrTestOrName) {
   }
 }
 
-// Helper to scope a DOM snapshot to the ember-testing container
-function scopeDOM(dom, { scope, domTransformation }) {
-  if (domTransformation) domTransformation(dom);
-  // we only want to capture the ember application, not the testing UI
-  let $scoped = dom.querySelector(scope || '#ember-testing');
+// Helper to scope a DOM snapshot to the ember-testing container to capture the
+// ember application without the testing UI
+function scopeDOM(scope, dom) {
+  let $scoped = dom.querySelector(scope);
   let $body = dom.querySelector('body');
   if (!$scoped) return;
 
@@ -45,11 +44,16 @@ function scopeDOM(dom, { scope, domTransformation }) {
     $body.setAttribute(name, value);
   }
 
-  // remove ember-testing styles by removing the id
-  dom.querySelector('#ember-testing').removeAttribute('id');
+  // remove #ember-testing styles by removing the id
+  dom.querySelector('#ember-testing')?.removeAttribute('id');
 }
 
-export default async function percySnapshot(name, options = {}) {
+export default async function percySnapshot(name, {
+  // separate SDK specific options from snapshot options
+  emberTestingScope = '#ember-testing',
+  domTransformation,
+  ...options
+} = {}) {
   // Check if Percy is enabled
   if (!(await utils.isPercyEnabled())) return;
   let log = utils.logger('ember');
@@ -65,7 +69,9 @@ export default async function percySnapshot(name, options = {}) {
     // Serialize and capture the DOM
     let domSnapshot = window.PercyDOM.serialize({
       enableJavaScript: options.enableJavaScript,
-      domTransformation: dom => scopeDOM(dom, options)
+      domTransformation: dom => scopeDOM(emberTestingScope, (
+        domTransformation ? domTransformation(dom) : dom
+      ))
     });
 
     // Post the DOM to the snapshot endpoint with snapshot options and other info

diff --git a/tests/acceptance/index-test.js b/tests/acceptance/index-test.js
@@ -87,4 +87,26 @@ module('percySnapshot', hooks => {
       'Could not take DOM snapshot "Snapshot 1"'
     ]);
   });
+
+  module('with an alternate ember-testing scope', hooks => {
+    let $scope;
+
+    hooks.beforeEach(() => {
+      $scope = document.querySelector('#ember-testing');
+      $scope.id = 'testing-container';
+    });
+
+    hooks.afterEach(() => {
+      $scope.id = 'ember-testing';
+    });
+
+    test('uses the alternate scope', async assert => {
+      await percySnapshot('Snapshot 1', {
+        emberTestingScope: '#testing-container'
+      });
+
+      assert.matches((await helpers.get('requests'))[1].body.domSnapshot, (
+        /<body id="testing-container" class="ember-application">/));
+    });
+  });
 });