Sort the classes that appear in attribution selectors to reduce cardi…

…nality (#518) * Sort class names in getSelector() * Remove debug comment * Review feedback * Review feedback 2 * Partially reverting last change * Simplify code and add test case --------- Co-authored-by: Philip Walton <[email protected]>
GoogleChrome · Sep 27, 2024 · ffa8ed5 · ffa8ed5
1 parent be41ea5
commit ffa8ed5
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 15 deletions.
diff --git a/src/lib/getSelector.ts b/src/lib/getSelector.ts
@@ -21,24 +21,24 @@ const getName = (node: Node) => {
     : name.toUpperCase().replace(/^#/, '');
 };
 
-export const getSelector = (node: Node | null | undefined, maxLen?: number) => {
+const MAX_LEN = 100;
+
+export const getSelector = (node: Node | null | undefined) => {
   let sel = '';
 
   try {
-    while (node && node.nodeType !== 9) {
+    while (node?.nodeType !== 9) {
       const el: Element = node as Element;
       const part = el.id
         ? '#' + el.id
-        : getName(el) +
-          (el.classList &&
-          el.classList.value &&
-          el.classList.value.trim() &&
-          el.classList.value.trim().length
-            ? '.' + el.classList.value.trim().replace(/\s+/g, '.')
-            : '');
-      if (sel.length + part.length > (maxLen || 100) - 1) return sel || part;
+        : [getName(el), ...Array.from(el.classList).sort()].join('.');
+      if (sel.length + part.length > MAX_LEN - 1) {
+        return sel || part;
+      }
       sel = sel ? part + '>' + sel : part;
-      if (el.id) break;
+      if (el.id) {
+        break;
+      }
       node = el.parentNode;
     }
   } catch (err) {

diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js
@@ -465,7 +465,7 @@ describe('onLCP()', async function () {
       assertStandardReportsAreCorrect([lcp]);
 
       assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500'));
-      assert.equal(lcp.attribution.element, 'html>body>main>p>img');
+      assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');
       assert.equal(
         lcp.attribution.timeToFirstByte +
           lcp.attribution.resourceLoadDelay +
@@ -515,7 +515,7 @@ describe('onLCP()', async function () {
       assertStandardReportsAreCorrect([lcp]);
 
       assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500'));
-      assert.equal(lcp.attribution.element, 'html>body>main>p>img');
+      assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');
 
       // Specifically check that resourceLoadDelay falls back to `startTime`.
       assert.equal(
@@ -565,7 +565,7 @@ describe('onLCP()', async function () {
 
       assert(lcp.attribution.url.endsWith('/test/img/square.png?delay=500'));
       assert.equal(lcp.navigationType, 'prerender');
-      assert.equal(lcp.attribution.element, 'html>body>main>p>img');
+      assert.equal(lcp.attribution.element, 'html>body>main>p>img.bar.foo');
 
       // Assert each individual LCP sub-part accounts for `activationStart`
       assert.equal(

diff --git a/test/views/lcp.njk b/test/views/lcp.njk
@@ -20,7 +20,7 @@
     {% if not imgDelay %}
       {% set imgDelay = 500 %}
     {% endif %}
-    <img elementtiming="main-image" {% if imgHidden %}hidden{% endif %} src="/test/img/square.png?delay={{ imgDelay }}">
+    <img class="foo bar" elementtiming="main-image" {% if imgHidden %}hidden{% endif %} src="/test/img/square.png?delay={{ imgDelay }}">
   </p>
   <p>Text below the image</p>