From b77fd7d9f8f2713042a32e2b86e1e5084db1020b Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@gmail.com>
Date: Wed, 20 Feb 2019 13:50:00 -0500
Subject: [PATCH] report(details-renderer): use new audit-details types (#7192)

---
 .../report/html/renderer/details-renderer.js  | 360 ++++++---------
 .../html/renderer/details-renderer-test.js    | 413 +++++++++++++++---
 types/audit-details.d.ts                      |  11 +-
 types/audit.d.ts                              |   1 -
 4 files changed, 501 insertions(+), 284 deletions(-)

diff --git a/lighthouse-core/report/html/renderer/details-renderer.js b/lighthouse-core/report/html/renderer/details-renderer.js
index 5e8bf852707f..51bf2ce72d7a 100644
--- a/lighthouse-core/report/html/renderer/details-renderer.js
+++ b/lighthouse-core/report/html/renderer/details-renderer.js
@@ -19,9 +19,7 @@
 /* globals self CriticalRequestChainRenderer SnippetRenderer Util URL */
 
 /** @typedef {import('./dom.js')} DOM */
-/** @typedef {LH.Audit.Details.Opportunity} OpportunityDetails */
 
-/** @type {Array<string>} */
 const URL_PREFIXES = ['http://', 'https://', 'data:'];
 
 class DetailsRenderer {
@@ -43,57 +41,34 @@ class DetailsRenderer {
   }
 
   /**
-   * @param {DetailsJSON|OpportunityDetails|LH.Audit.Details.SnippetValue} details
+   * @param {LH.Audit.Details} details
    * @return {Element|null}
    */
   render(details) {
     switch (details.type) {
-      case 'text':
-        return this._renderText(/** @type {StringDetailsJSON} */ (details));
-      case 'url':
-        return this._renderTextURL(/** @type {StringDetailsJSON} */ (details));
-      case 'bytes':
-        return this._renderBytes(/** @type {NumericUnitDetailsJSON} */ (details));
-      case 'ms':
-        // eslint-disable-next-line max-len
-        return this._renderMilliseconds(/** @type {NumericUnitDetailsJSON} */ (details));
-      case 'link':
-        // @ts-ignore - TODO(bckenny): Fix type hierarchy
-        return this._renderLink(/** @type {LinkDetailsJSON} */ (details));
-      case 'thumbnail':
-        return this._renderThumbnail(/** @type {ThumbnailDetails} */ (details));
       case 'filmstrip':
-        // @ts-ignore - TODO(bckenny): Fix type hierarchy
-        return this._renderFilmstrip(/** @type {FilmstripDetails} */ (details));
-      case 'table':
-        // @ts-ignore - TODO(bckenny): Fix type hierarchy
-        return this._renderTable(/** @type {TableDetailsJSON} */ (details));
+        return this._renderFilmstrip(details);
       case 'list':
-        return this._renderList(
-          // @ts-ignore - TODO(bckenny): Fix type hierarchy
-          /** @type {LH.Audit.Details.List} */ (details)
-        );
-      case 'code':
-        return this._renderCode(/** @type {DetailsJSON} */ (details));
-      case 'node':
-        return this.renderNode(/** @type {NodeDetailsJSON} */(details));
+        return this._renderList(details);
+      case 'table':
+        return this._renderTable(details);
       case 'criticalrequestchain':
-        return CriticalRequestChainRenderer.render(this._dom, this._templateContext,
-          // @ts-ignore - TODO(bckenny): Fix type hierarchy
-          /** @type {LH.Audit.Details.CriticalRequestChain} */ (details));
+        return CriticalRequestChainRenderer.render(this._dom, this._templateContext, details);
       case 'opportunity':
-        // @ts-ignore - TODO(bckenny): Fix type hierarchy
-        return this._renderOpportunityTable(details);
-      case 'numeric':
-        return this._renderNumeric(/** @type {StringDetailsJSON} */ (details));
+        return this._renderTable(details);
 
       // Internal-only details, not for rendering.
       case 'screenshot':
       case 'diagnostic':
         return null;
+      // Fallback for old LHRs, where no type meant don't render.
+      case undefined:
+        return null;
 
       default: {
-        throw new Error(`Unknown type: ${details.type}`);
+        // @ts-ignore tsc thinks this unreachable, but ts-ignore for error message just in case.
+        const detailsType = details.type;
+        throw new Error(`Unknown type: ${detailsType}`);
       }
     }
   }
@@ -158,7 +133,7 @@ class DetailsRenderer {
   }
 
   /**
-   * @param {LinkDetailsJSON} details
+   * @param {LH.Audit.Details.LinkValue} details
    * @return {Element}
    */
   _renderLink(details) {
@@ -202,7 +177,6 @@ class DetailsRenderer {
 
   /**
    * Create small thumbnail with scaled down image asset.
-   * If the supplied details doesn't have an image/* mimeType, then an empty span is returned.
    * @param {{value: string}} details
    * @return {Element}
    */
@@ -216,96 +190,126 @@ class DetailsRenderer {
   }
 
   /**
-   * @param {LH.Audit.Details.List} details
-   * @returns {Element}
-   */
-  _renderList(details) {
-    const listContainer = this._dom.createElement('div', 'lh-list');
-
-    details.items.forEach(item => {
-      const snippetEl = SnippetRenderer.render(this._dom, this._templateContext, item, this);
-      listContainer.appendChild(snippetEl);
-    });
-
-    return listContainer;
-  }
-
-  /**
-   * @param {TableDetailsJSON} details
-   * @return {Element}
+   * Render a details item value for embedding in a table. Renders the value
+   * based on the heading's valueType, unless the value itself has a `type`
+   * property to override it.
+   * @param {LH.Audit.Details.TableItem[string] | LH.Audit.Details.OpportunityItem[string]} value
+   * @param {LH.Audit.Details.OpportunityColumnHeading} heading
+   * @return {Element|null}
    */
-  _renderTable(details) {
-    if (!details.items.length) return this._dom.createElement('span');
-
-    const tableElem = this._dom.createElement('table', 'lh-table');
-    const theadElem = this._dom.createChildOf(tableElem, 'thead');
-    const theadTrElem = this._dom.createChildOf(theadElem, 'tr');
-
-    for (const heading of details.headings) {
-      const itemType = heading.itemType || 'text';
-      const classes = `lh-table-column--${itemType}`;
-      // @ts-ignore TODO(bckenny): this can never be null
-      this._dom.createChildOf(theadTrElem, 'th', classes).appendChild(this.render({
-        type: 'text',
-        value: heading.text || '',
-      }));
+  _renderTableValue(value, heading) {
+    if (typeof value === 'undefined' || value === null) {
+      return null;
     }
 
-    const tbodyElem = this._dom.createChildOf(tableElem, 'tbody');
-    for (const row of details.items) {
-      const rowElem = this._dom.createChildOf(tbodyElem, 'tr');
-      for (const heading of details.headings) {
-        const key = /** @type {keyof DetailsJSON} */ (heading.key);
-        // TODO(bckenny): type should be naturally inferred here.
-        const value = /** @type {number|string|DetailsJSON|undefined} */ (row[key]);
-
-        if (typeof value === 'undefined' || value === null) {
-          this._dom.createChildOf(rowElem, 'td', 'lh-table-column--empty');
-          continue;
+    // First deal with the possible object forms of value.
+    if (typeof value === 'object') {
+      // The value's type overrides the heading's for this column.
+      switch (value.type) {
+        case 'code': {
+          return this._renderCode(value);
+        }
+        case 'link': {
+          return this._renderLink(value);
         }
-        // handle nested types like code blocks in table rows.
-        // @ts-ignore - TODO(bckenny): narrow first
-        if (value.type) {
-          const valueAsDetails = /** @type {DetailsJSON} */ (value);
-          const classes = `lh-table-column--${valueAsDetails.type}`;
-          // @ts-ignore TODO(bckenny): this can never be null
-          this._dom.createChildOf(rowElem, 'td', classes).appendChild(this.render(valueAsDetails));
-          continue;
+        case 'node': {
+          return this.renderNode(value);
         }
+        case 'url': {
+          return this._renderTextURL(value);
+        }
+        default: {
+          throw new Error(`Unknown valueType: ${value.type}`);
+        }
+      }
+    }
 
-        // build new details item to render
-        const item = {
-          value: /** @type {number|string} */ (value),
-          type: heading.itemType,
-          displayUnit: heading.displayUnit,
+    // Next, deal with primitives.
+    switch (heading.valueType) {
+      case 'bytes': {
+        const numValue = Number(value);
+        return this._renderBytes({value: numValue, granularity: 1});
+      }
+      case 'code': {
+        const strValue = String(value);
+        return this._renderCode({value: strValue});
+      }
+      case 'ms': {
+        const msValue = {
+          value: Number(value),
           granularity: heading.granularity,
+          displayUnit: heading.displayUnit,
         };
-
-        /** @type {string|undefined} */
-        // @ts-ignore - TODO(bckenny): handle with refactoring above
-        const valueType = value.type;
-        const classes = `lh-table-column--${valueType || heading.itemType}`;
-        // @ts-ignore TODO(bckenny): this can never be null
-        this._dom.createChildOf(rowElem, 'td', classes).appendChild(this.render(item));
+        return this._renderMilliseconds(msValue);
+      }
+      case 'numeric': {
+        const strValue = String(value);
+        return this._renderNumeric({value: strValue});
+      }
+      case 'text': {
+        const strValue = String(value);
+        return this._renderText({value: strValue});
+      }
+      case 'thumbnail': {
+        const strValue = String(value);
+        return this._renderThumbnail({value: strValue});
+      }
+      case 'timespanMs': {
+        const numValue = Number(value);
+        return this._renderMilliseconds({value: numValue});
+      }
+      case 'url': {
+        const strValue = String(value);
+        if (URL_PREFIXES.some(prefix => strValue.startsWith(prefix))) {
+          return this._renderTextURL({value: strValue});
+        } else {
+          // Fall back to <pre> rendering if not actually a URL.
+          return this._renderCode({value: strValue});
+        }
+      }
+      default: {
+        throw new Error(`Unknown valueType: ${heading.valueType}`);
       }
     }
-    return tableElem;
   }
 
   /**
-   * TODO(bckenny): migrate remaining table rendering to this function, then rename
-   * back to _renderTable and replace the original.
-   * @param {OpportunityDetails} details
+   * Get the headings of a table-like details object, converted into the
+   * OpportunityColumnHeading type until we have all details use the same
+   * heading format.
+   * @param {LH.Audit.Details.Table|LH.Audit.Details.Opportunity} tableLike
+   * @return {Array<LH.Audit.Details.OpportunityColumnHeading>} header
+   */
+  _getCanonicalizedTableHeadings(tableLike) {
+    if (tableLike.type === 'opportunity') {
+      return tableLike.headings;
+    }
+
+    return tableLike.headings.map(heading => {
+      return {
+        key: heading.key,
+        label: heading.text,
+        valueType: heading.itemType,
+        displayUnit: heading.displayUnit,
+        granularity: heading.granularity,
+      };
+    });
+  }
+
+  /**
+   * @param {LH.Audit.Details.Table|LH.Audit.Details.Opportunity} details
    * @return {Element}
    */
-  _renderOpportunityTable(details) {
+  _renderTable(details) {
     if (!details.items.length) return this._dom.createElement('span');
 
     const tableElem = this._dom.createElement('table', 'lh-table');
     const theadElem = this._dom.createChildOf(tableElem, 'thead');
     const theadTrElem = this._dom.createChildOf(theadElem, 'tr');
 
-    for (const heading of details.headings) {
+    const headings = this._getCanonicalizedTableHeadings(details);
+
+    for (const heading of headings) {
       const valueType = heading.valueType || 'text';
       const classes = `lh-table-column--${valueType}`;
       const labelEl = this._dom.createElement('div', 'lh-text');
@@ -316,60 +320,38 @@ class DetailsRenderer {
     const tbodyElem = this._dom.createChildOf(tableElem, 'tbody');
     for (const row of details.items) {
       const rowElem = this._dom.createChildOf(tbodyElem, 'tr');
-      for (const heading of details.headings) {
-        const key = /** @type {keyof LH.Audit.Details.OpportunityItem} */ (heading.key);
-        const value = row[key];
-
-        if (typeof value === 'undefined' || value === null) {
+      for (const heading of headings) {
+        const value = row[heading.key];
+        const valueElement = this._renderTableValue(value, heading);
+
+        if (valueElement) {
+          const classes = `lh-table-column--${heading.valueType}`;
+          this._dom.createChildOf(rowElem, 'td', classes).appendChild(valueElement);
+        } else {
           this._dom.createChildOf(rowElem, 'td', 'lh-table-column--empty');
-          continue;
         }
-
-        const valueType = heading.valueType;
-        let itemElement;
-
-        // TODO(bckenny): as we add more table types, split out into _renderTableItem fn.
-        switch (valueType) {
-          case 'url': {
-            // Fall back to <pre> rendering if not actually a URL.
-            const strValue = /** @type {string} */ (value);
-            if (URL_PREFIXES.some(prefix => strValue.startsWith(prefix))) {
-              itemElement = this._renderTextURL({value: strValue});
-            } else {
-              const codeValue = /** @type {(number|string|undefined)} */ (value);
-              itemElement = this._renderCode({value: codeValue});
-            }
-            break;
-          }
-          case 'timespanMs': {
-            const numValue = /** @type {number} */ (value);
-            itemElement = this._renderMilliseconds({value: numValue});
-            break;
-          }
-          case 'bytes': {
-            const numValue = /** @type {number} */ (value);
-            itemElement = this._renderBytes({value: numValue, granularity: 1});
-            break;
-          }
-          case 'thumbnail': {
-            const strValue = /** @type {string} */ (value);
-            itemElement = this._renderThumbnail({value: strValue});
-            break;
-          }
-          default: {
-            throw new Error(`Unknown valueType: ${valueType}`);
-          }
-        }
-
-        const classes = `lh-table-column--${valueType}`;
-        this._dom.createChildOf(rowElem, 'td', classes).appendChild(itemElement);
       }
     }
     return tableElem;
   }
 
   /**
-   * @param {NodeDetailsJSON} item
+   * @param {LH.Audit.Details.List} details
+   * @returns {Element}
+   */
+  _renderList(details) {
+    const listContainer = this._dom.createElement('div', 'lh-list');
+
+    details.items.forEach(item => {
+      const snippetEl = SnippetRenderer.render(this._dom, this._templateContext, item, this);
+      listContainer.appendChild(snippetEl);
+    });
+
+    return listContainer;
+  }
+
+  /**
+   * @param {LH.Audit.Details.NodeValue} item
    * @return {Element}
    * @protected
    */
@@ -420,71 +402,3 @@ if (typeof module !== 'undefined' && module.exports) {
 } else {
   self.DetailsRenderer = DetailsRenderer;
 }
-
-// TODO, what's the diff between DetailsJSON and NumericUnitDetailsJSON?
-/**
- * @typedef {{
-      type: string,
-      value: (string|number|undefined),
-      granularity?: number,
-      displayUnit?: string
-  }} DetailsJSON
- */
-
-/**
- * @typedef {{
-      type: string,
-      value: string,
-      granularity?: number,
-      displayUnit?: string,
-  }} StringDetailsJSON
- */
-
-/**
- * @typedef {{
-      type: string,
-      value: number,
-      granularity?: number,
-      displayUnit?: string,
-  }} NumericUnitDetailsJSON
- */
-
-/**
- * @typedef {{
-      type: string,
-      path?: string,
-      selector?: string,
-      snippet?: string
-  }} NodeDetailsJSON
- */
-
-/**
- * @typedef {{
-      itemType: string,
-      key: string,
-      text?: string,
-      granularity?: number,
-      displayUnit?: string,
-  }} TableHeaderJSON
- */
-
-/** @typedef {{
-      type: string,
-      items: Array<DetailsJSON>,
-      headings: Array<TableHeaderJSON>
-  }} TableDetailsJSON
- */
-
-/** @typedef {{
-      type: string,
-      value: string,
-  }} ThumbnailDetails
- */
-
-/** @typedef {{
-      type: string,
-      text: string,
-      url: string
-  }} LinkDetailsJSON
- */
-
diff --git a/lighthouse-core/test/report/html/renderer/details-renderer-test.js b/lighthouse-core/test/report/html/renderer/details-renderer-test.js
index d348860c1dd7..21d00e62016d 100644
--- a/lighthouse-core/test/report/html/renderer/details-renderer-test.js
+++ b/lighthouse-core/test/report/html/renderer/details-renderer-test.js
@@ -13,6 +13,7 @@ const DOM = require('../../../../report/html/renderer/dom.js');
 const Util = require('../../../../report/html/renderer/util.js');
 const DetailsRenderer = require('../../../../report/html/renderer/details-renderer.js');
 const SnippetRenderer = require('../../../../report/html/renderer/snippet-renderer.js');
+const CrcDetailsRenderer = require('../../../../report/html/renderer/crc-details-renderer.js');
 
 const TEMPLATE_FILE = fs.readFileSync(__dirname +
     '/../../../../report/html/templates.html', 'utf8');
@@ -25,52 +26,22 @@ describe('DetailsRenderer', () => {
   beforeAll(() => {
     global.URL = URL;
     global.Util = Util;
+    global.CriticalRequestChainRenderer = CrcDetailsRenderer;
     global.SnippetRenderer = SnippetRenderer;
     const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window;
     const dom = new DOM(document);
     renderer = new DetailsRenderer(dom);
-    renderer.setTemplateContext(dom._document);
+    renderer.setTemplateContext(dom.document());
   });
 
   afterAll(() => {
     global.URL = undefined;
     global.Util = undefined;
+    global.CriticalRequestChainRenderer = undefined;
     global.SnippetRenderer = undefined;
   });
 
   describe('render', () => {
-    it('renders text', () => {
-      const el = renderer.render({type: 'text', value: 'My text content'});
-      assert.equal(el.textContent, 'My text content');
-      assert.ok(el.classList.contains('lh-text'), 'adds classes');
-    });
-
-    it('renders code', () => {
-      const el = renderer.render({
-        type: 'code',
-        value: 'code snippet',
-        lineNumber: 123,
-        source: 'deprecation',
-        url: 'https://example.com/feature',
-      });
-
-      assert.ok(el.localName === 'pre');
-      assert.ok(el.classList.contains('lh-code'));
-      assert.equal(el.textContent, 'code snippet');
-    });
-
-    it('renders thumbnails', () => {
-      const el = renderer.render({
-        type: 'thumbnail',
-        value: 'http://example.com/my-image.jpg',
-        mimeType: 'image/jpeg',
-      });
-
-      assert.ok(el.localName === 'img');
-      assert.ok(el.classList.contains('lh-thumbnail'));
-      assert.equal(el.src, 'http://example.com/my-image.jpg');
-    });
-
     it('renders filmstrips', () => {
       const el = renderer.render({
         type: 'filmstrip',
@@ -104,12 +75,12 @@ describe('DetailsRenderer', () => {
           {
             a: 'value A.1',
             b: 'value A.2',
-            c: {type: 'thumbnail', value: 'http://example.com/image.jpg'},
+            c: 'http://example.com/image.jpg',
           },
           {
             a: 'value B.1',
             b: 'value B.2',
-            c: {type: 'thumbnail', value: 'unknown'},
+            c: 'unknown',
           },
         ],
       });
@@ -123,6 +94,60 @@ describe('DetailsRenderer', () => {
           '--thumbnail not set');
     });
 
+    it('renders critical request chains', () => {
+      const details = {
+        type: 'criticalrequestchain',
+        longestChain: {
+          duration: 2,
+          length: 1,
+          transferSize: 221,
+        },
+        chains: {
+          F3B687683512E0F003DD41EB23E2091A: {
+            request: {
+              url: 'https://example.com',
+              startTime: 0,
+              endTime: 2,
+              responseReceivedTime: 1,
+              transferSize: 221,
+            },
+            children: {},
+          },
+        },
+      };
+
+      const crcEl = renderer.render(details);
+      assert.ok(crcEl.classList.contains('lh-crc-container'));
+      assert.strictEqual(crcEl.querySelectorAll('.crc-node').length, 1);
+    });
+
+    it('renders opportunity details as a table', () => {
+      const details = {
+        type: 'opportunity',
+        headings: [
+          {key: 'url', valueType: 'url', label: 'URL'},
+          {key: 'totalBytes', valueType: 'bytes', label: 'Size (KB)'},
+          {key: 'wastedBytes', valueType: 'bytes', label: 'Potential Savings (KB)'},
+        ],
+        items: [{
+          url: 'https://example.com',
+          totalBytes: 71654,
+          wastedBytes: 30470,
+          wastedPercent: 42,
+        }],
+        overallSavingsMs: 150,
+        overallSavingsBytes: 30470,
+      };
+
+      const oppEl = renderer.render(details);
+      assert.equal(oppEl.localName, 'table');
+      assert.ok(oppEl.querySelector('.lh-text__url').title === 'https://example.com', 'did not render recursive items');
+      assert.equal(oppEl.querySelectorAll('th').length, 3, 'did not render header items');
+      assert.equal(oppEl.querySelectorAll('td').length, 3, 'did not render table cells');
+      assert.equal(oppEl.querySelectorAll('.lh-table-column--url').length, 2, 'url column not set');
+      assert.equal(oppEl.querySelectorAll('.lh-table-column--bytes').length, 4, 'bytes not set');
+    });
+
     it('renders lists', () => {
       const snippet = {
         type: 'snippet',
@@ -144,47 +169,325 @@ describe('DetailsRenderer', () => {
       assert.ok(el.children[0].textContent.includes('Some snippet'), 'renders item content');
     });
 
-    it('renders links', () => {
+    it('does not render internal-only screenshot details', () => {
+      const details = {
+        type: 'screenshot',
+        timestamp: 185600000,
+        data: 'data:image/jpeg;base64,/9j/4AAQSkZJRYP/2Q==',
+      };
+
+      const screenshotEl = renderer.render(details);
+      assert.strictEqual(screenshotEl, null);
+    });
+
+    it('does not render internal-only diagnostic details', () => {
+      const details = {
+        type: 'diagnostic',
+        items: [{
+          failures: ['No manifest was fetched'],
+          isParseFailure: true,
+          parseFailureReason: 'No manifest was fetched',
+        }],
+      };
+
+      const diagnosticEl = renderer.render(details);
+      assert.strictEqual(diagnosticEl, null);
+    });
+
+    it('throws on unknown details type', () => {
+      // Disallowed by type system, but test that we get an error message out just in case.
+      const details = {
+        type: 'imaginary',
+        items: 5,
+      };
+
+      assert.throws(() => renderer.render(details), /^Error: Unknown type: imaginary$/);
+    });
+
+    it('supports old LHRs by not rendering when type is missing', () => {
+      // Disallowed by type system of current LH, but possible if trying to render an old LHR.
+      const details = {
+        timestamp: 15,
+      };
+
+      assert.strictEqual(renderer.render(details), null);
+    });
+  });
+
+  describe('Table rendering', () => {
+    it('renders text values', () => {
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'text', text: 'Heading'}],
+        items: [{content: 'My text content'}],
+      };
+
+      const el = renderer.render(details);
+      const textEls = el.querySelectorAll('.lh-text');
+      assert.strictEqual(textEls[0].textContent, 'Heading');
+      assert.strictEqual(textEls[1].textContent, 'My text content');
+    });
+
+    it('renders not much if items are empty', () => {
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'text', text: 'Heading'}],
+        items: [],
+      };
+
+      const el = renderer.render(details);
+      assert.strictEqual(el.outerHTML, '<span></span>');
+    });
+
+    it('renders an empty cell if item is missing a property', () => {
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'text', text: 'Heading'}],
+        items: [
+          {},
+          {content: undefined},
+          {content: 'a thing'},
+        ],
+      };
+
+      const el = renderer.render(details);
+      const itemEls = el.querySelectorAll('td');
+      assert.strictEqual(itemEls.length, 3);
+
+      // missing prop
+      assert.ok(itemEls[0].classList.contains('lh-table-column--empty'));
+      assert.strictEqual(itemEls[0].innerHTML, '');
+
+      // undefined prop
+      assert.ok(itemEls[1].classList.contains('lh-table-column--empty'));
+      assert.strictEqual(itemEls[1].innerHTML, '');
+
+      // defined prop
+      assert.ok(itemEls[2].classList.contains('lh-table-column--text'));
+      assert.strictEqual(itemEls[2].textContent, 'a thing');
+    });
+
+    it('renders code values from a string', () => {
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'code', text: 'Heading'}],
+        items: [{content: 'code snippet'}],
+      };
+
+      const el = renderer.render(details);
+      const codeEl = el.querySelector('.lh-code');
+      assert.ok(codeEl.localName === 'pre');
+      assert.equal(codeEl.textContent, 'code snippet');
+    });
+
+    it('renders code values from a code details object', () => {
+      const code = {
+        type: 'code',
+        value: 'code object',
+      };
+
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'code', text: 'Heading'}],
+        items: [{content: code}],
+      };
+
+      const el = renderer.render(details);
+      const codeEl = el.querySelector('.lh-code');
+      assert.ok(codeEl.localName === 'pre');
+      assert.equal(codeEl.textContent, 'code object');
+    });
+
+    it('renders thumbnail values', () => {
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'thumbnail', text: 'Heading'}],
+        items: [{content: 'http://example.com/my-image.jpg'}],
+      };
+
+      const el = renderer.render(details);
+      const thumbnailEl = el.querySelector('img');
+      assert.ok(thumbnailEl.classList.contains('lh-thumbnail'));
+      assert.strictEqual(thumbnailEl.src, 'http://example.com/my-image.jpg');
+      assert.strictEqual(thumbnailEl.title, 'http://example.com/my-image.jpg');
+      assert.strictEqual(thumbnailEl.alt, '');
+    });
+
+    it('renders link values', () => {
       const linkText = 'Example Site';
       const linkUrl = 'https://example.com/';
-      const el = renderer.render({
+      const link = {
         type: 'link',
         text: linkText,
         url: linkUrl,
-      });
+      };
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'link', text: 'Heading'}],
+        items: [{content: link}],
+      };
 
-      assert.equal(el.localName, 'a');
-      assert.equal(el.textContent, linkText);
-      assert.equal(el.href, linkUrl);
-      assert.equal(el.rel, 'noopener');
-      assert.equal(el.target, '_blank');
+      const el = renderer.render(details);
+      const anchorEl = el.querySelector('a');
+      assert.equal(anchorEl.textContent, linkText);
+      assert.equal(anchorEl.href, linkUrl);
+      assert.equal(anchorEl.rel, 'noopener');
+      assert.equal(anchorEl.target, '_blank');
     });
 
-    it('renders link as text if URL is not allowed', () => {
+    it('renders link value as text if URL is not allowed', () => {
       const linkText = 'Evil Link';
       const linkUrl = 'javascript:alert(5)';
-      const el = renderer.render({
+      const link = {
         type: 'link',
         text: linkText,
         url: linkUrl,
-      });
+      };
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'link', text: 'Heading'}],
+        items: [{content: link}],
+      };
 
-      assert.equal(el.localName, 'div');
-      assert.equal(el.textContent, linkText);
-      assert.ok(el.classList.contains('lh-text'), 'adds classes');
+      const el = renderer.render(details);
+      const linkEl = el.querySelector('td.lh-table-column--link > .lh-text');
+      assert.equal(linkEl.localName, 'div');
+      assert.equal(linkEl.textContent, linkText);
     });
 
-    it('renders text URLs', () => {
+    it('renders node values', () => {
+      const node = {
+        type: 'node',
+        path: '3,HTML,1,BODY,5,DIV,0,H2',
+        selector: 'h2',
+        snippet: '<h2>Do better web tester page</h2>',
+      };
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'node', text: 'Heading'}],
+        items: [{content: node}],
+      };
+
+      const el = renderer.render(details);
+      const nodeEl = el.querySelector('.lh-node');
+      assert.strictEqual(nodeEl.localName, 'span');
+      assert.equal(nodeEl.textContent, node.snippet);
+      assert.equal(nodeEl.title, node.selector);
+      assert.equal(nodeEl.getAttribute('data-path'), node.path);
+      assert.equal(nodeEl.getAttribute('data-selector'), node.selector);
+      assert.equal(nodeEl.getAttribute('data-snippet'), node.snippet);
+    });
+
+    it('renders text URL values from a string', () => {
       const urlText = 'https://example.com/';
       const displayUrlText = 'https://example.com';
-      const el = renderer.render({
+
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'url', text: 'Heading'}],
+        items: [{content: urlText}],
+      };
+
+      const el = renderer.render(details);
+      const urlEl = el.querySelector('td.lh-table-column--url > .lh-text__url');
+
+      assert.equal(urlEl.localName, 'div');
+      assert.equal(urlEl.title, urlText);
+      assert.ok(urlEl.firstChild.classList.contains('lh-text'));
+      assert.equal(urlEl.textContent, displayUrlText);
+    });
+
+    it('renders text URL values from a url details object', () => {
+      const urlText = 'https://example.com/';
+      const displayUrlText = 'https://example.com';
+      const url = {
         type: 'url',
         value: urlText,
-      });
+      };
 
-      assert.equal(el.localName, 'div');
-      assert.equal(el.textContent, displayUrlText);
-      assert.ok(el.classList.contains('lh-text__url'), 'adds classes');
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'url', text: 'Heading'}],
+        items: [{content: url}],
+        overallSavingsMs: 100,
+      };
+
+      const el = renderer.render(details);
+      const urlEl = el.querySelector('td.lh-table-column--url > .lh-text__url');
+
+      assert.equal(urlEl.localName, 'div');
+      assert.equal(urlEl.title, urlText);
+      assert.ok(urlEl.firstChild.classList.contains('lh-text'));
+      assert.equal(urlEl.textContent, displayUrlText);
+    });
+
+    it('renders text URL values as code if not an allowed URL', () => {
+      const urlText = 'invalid-url://example.com/';
+
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'url', text: 'Heading'}],
+        items: [{content: urlText}],
+      };
+
+      const el = renderer.render(details);
+      const codeItemEl = el.querySelector('td.lh-table-column--url');
+      assert.strictEqual(codeItemEl.innerHTML, '<pre class="lh-code">invalid-url://example.com/</pre>');
+    });
+
+    it('throws on unknown heading itemType', () => {
+      // Disallowed by type system, but test that we get an error message out just in case.
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'notRealValueType', text: 'Heading'}],
+        items: [{content: 'some string'}],
+      };
+
+      assert.throws(() => renderer.render(details), /^Error: Unknown valueType: notRealValueType$/);
+    });
+
+    it('throws on unknown item object type', () => {
+      // Disallowed by type system, but test that we get an error message out just in case.
+      const item = {
+        type: 'imaginaryItem',
+        items: 'alllll the items',
+      };
+
+      const details = {
+        type: 'table',
+        headings: [{key: 'content', itemType: 'url', text: 'Heading'}],
+        items: [{content: item}],
+      };
+
+      assert.throws(() => renderer.render(details), /^Error: Unknown valueType: imaginaryItem$/);
+    });
+
+    it('uses the item\'s type over the heading type', () => {
+      const details = {
+        type: 'table',
+        // itemType is overriden by code object
+        headings: [{key: 'content', itemType: 'url', text: 'Heading'}],
+        items: [
+          {content: {type: 'code', value: 'code object'}},
+          {content: 'https://example.com'},
+        ],
+      };
+
+      const el = renderer.render(details);
+      const itemElements = el.querySelectorAll('td.lh-table-column--url');
+
+      // First item's value uses its own type.
+      const codeEl = itemElements[0].firstChild;
+      assert.equal(codeEl.localName, 'pre');
+      assert.ok(codeEl.classList.contains('lh-code'));
+      assert.equal(codeEl.textContent, 'code object');
+
+      // Second item uses the heading's specified type for the column.
+      const urlEl = itemElements[1].firstChild;
+      assert.equal(urlEl.localName, 'div');
+      assert.ok(urlEl.classList.contains('lh-text__url'));
+      assert.equal(urlEl.title, 'https://example.com');
+      assert.equal(urlEl.textContent, 'https://example.com');
     });
   });
 });
diff --git a/types/audit-details.d.ts b/types/audit-details.d.ts
index 50c2e9565ffb..7bc1530d73a4 100644
--- a/types/audit-details.d.ts
+++ b/types/audit-details.d.ts
@@ -10,6 +10,7 @@ declare global {
       Details.CriticalRequestChain |
       Details.Diagnostic |
       Details.Filmstrip |
+      Details.List |
       Details.Opportunity |
       Details.Screenshot |
       Details.Table;
@@ -39,6 +40,11 @@ declare global {
         }[];
       }
 
+      export interface List {
+        type: 'list';
+        items: SnippetValue[]
+      }
+
       export interface Opportunity {
         type: 'opportunity';
         overallSavingsMs: number;
@@ -65,11 +71,6 @@ declare global {
         diagnostic?: Diagnostic;
       }
 
-      export interface List {
-        type: 'list';
-        items: SnippetValue[]
-      }
-
       /**
        * A details type that is not rendered in the final report; usually used
        * for including diagnostic information in the LHR. Can contain anything.
diff --git a/types/audit.d.ts b/types/audit.d.ts
index 41cceacad7fb..0afbc416f0ce 100644
--- a/types/audit.d.ts
+++ b/types/audit.d.ts
@@ -100,7 +100,6 @@ declare global {
       id: string;
       /** A more detailed description that describes why the audit is important and links to Lighthouse documentation on the audit; markdown links supported. */
       description: string;
-      // TODO(bckenny): define details
       details?: Audit.Details;
     }