From fe20acc1ba807d0270f297a992417b63ee07ac25 Mon Sep 17 00:00:00 2001
From: Wil Wilsman <wil@wilwilsman.com>
Date: Mon, 22 Aug 2022 15:36:04 -0500
Subject: [PATCH] =?UTF-8?q?=F0=9F=92=A5=20Rename=20Ember=20specific=20scop?=
 =?UTF-8?q?e=20option?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 addon-test-support/@percy/ember/index.js | 24 +++++++++++++++---------
 tests/acceptance/index-test.js           | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/addon-test-support/@percy/ember/index.js b/addon-test-support/@percy/ember/index.js
index 073f339a..fe6a9fc7 100644
--- 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
index bc0718a0..5529b434 100644
--- 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">/));
+    });
+  });
 });