From 12d56af4c6e351985fa4495c931a680c2865aad7 Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Mon, 11 Feb 2019 08:55:10 +0000 Subject: [PATCH 01/20] Snippet renderer --- lighthouse-core/audits/audit.js | 66 ++++ lighthouse-core/lib/i18n/en-US.json | 8 + .../report/html/html-report-assets.js | 1 + .../report/html/renderer/details-renderer.js | 34 +- .../report/html/renderer/snippet-renderer.js | 343 ++++++++++++++++++ lighthouse-core/report/html/renderer/util.js | 49 +++ lighthouse-core/report/html/report-styles.css | 4 + lighthouse-core/report/html/templates.html | 139 +++++++ lighthouse-core/test/audits/audit-test.js | 96 +++++ .../html/renderer/snippet-renderer-test.js | 182 ++++++++++ lighthouse-core/test/results/sample_v2.json | 2 + package.json | 2 +- proto/lighthouse-result.proto | 6 + proto/sample_v2_round_trip.json | 2 + types/audit-details.d.ts | 26 ++ types/html-renderer.d.ts | 3 + 16 files changed, 960 insertions(+), 3 deletions(-) create mode 100644 lighthouse-core/report/html/renderer/snippet-renderer.js create mode 100644 lighthouse-core/test/report/html/renderer/snippet-renderer-test.js diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js index ce6188de9559..54ba36b37d76 100644 --- a/lighthouse-core/audits/audit.js +++ b/lighthouse-core/audits/audit.js @@ -125,6 +125,72 @@ class Audit { }; } + /** + * @param {LH.Audit.Details.List['items']} items + * @returns {LH.Audit.Details.List} + */ + static makeListDetails(items) { + return { + type: 'list', + items: items, + }; + } + + /** @typedef {{ + * content: string; + * title: string; + * lineMessages: LH.Audit.Details.Snippet['lineMessages']; + * generalMessages: LH.Audit.Details.Snippet['generalMessages']; + * node?: LH.Audit.Details.NodeValue; + * maxLineLength?: number; + * maxLinesAroundMessage?: number; + * }} SnippetInfo */ + /** + * @param {SnippetInfo} snippetInfo + * @return {LH.Audit.Details.Snippet} + */ + static makeSnippetDetails({ + content, + title, + lineMessages, + generalMessages, + node, + maxLineLength = 200, + maxLinesAroundMessage = 20, + }) { + const allLines = Audit._makeSnippetLinesArray(content, maxLineLength); + const lines = Util.filterRelevantLines(allLines, lineMessages, maxLinesAroundMessage); + return { + type: 'snippet', + lines, + title, + lineMessages, + generalMessages, + lineCount: allLines.length, + node, + }; + } + + /** + * @param {string} content + * @param {number} maxLineLength + * @returns {LH.Audit.Details.Snippet['lines']} + */ + static _makeSnippetLinesArray(content, maxLineLength) { + return content.split('\n').map((line, lineIndex) => { + const lineNumber = lineIndex + 1; + /** @type LH.Audit.Details.Snippet['lines'][0] */ + const lineDetail = { + content: line.slice(0, maxLineLength), + lineNumber, + }; + if (line.length > maxLineLength) { + lineDetail.truncated = true; + } + return lineDetail; + }); + } + /** * @param {LH.Audit.Details.Opportunity['headings']} headings * @param {LH.Audit.Details.Opportunity['items']} items diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index 2beef88eb553..10f654fbbb04 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1135,6 +1135,14 @@ "message": "Score scale:", "description": "Label preceding a pictorial explanation of the scoring scale: 0-50 is red (bad), 50-90 is orange (ok), 90-100 is green (good). These colors are used throughout the report to provide context for how good/bad a particular result is." }, + "lighthouse-core/report/html/renderer/util.js | snippetCollapseButtonLabel": { + "message": "Collapse snippet", + "description": "Label for button that only shows a few lines of the snippet when clicked" + }, + "lighthouse-core/report/html/renderer/util.js | snippetExpandButtonLabel": { + "message": "Expand snippet", + "description": "Label for button that shows all lines of the snippet when clicked" + }, "lighthouse-core/report/html/renderer/util.js | toplevelWarningsMessage": { "message": "There were issues affecting this run of Lighthouse:", "description": "Label shown preceding any important warnings that may have invalidated the entire report. For example, if the user has Chrome extensions installed, they may add enough performance overhead that Lighthouse's performance metrics are unreliable. If shown, this will be displayed at the top of the report UI." diff --git a/lighthouse-core/report/html/html-report-assets.js b/lighthouse-core/report/html/html-report-assets.js index 8ac7d203ea42..0a2b56c0a430 100644 --- a/lighthouse-core/report/html/html-report-assets.js +++ b/lighthouse-core/report/html/html-report-assets.js @@ -16,6 +16,7 @@ const REPORT_JAVASCRIPT = [ fs.readFileSync(require.resolve('details-element-polyfill'), 'utf8'), fs.readFileSync(__dirname + '/renderer/details-renderer.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/crc-details-renderer.js', 'utf8'), + fs.readFileSync(__dirname + '/renderer/snippet-renderer.js', 'utf8'), fs.readFileSync(__dirname + '/../../lib/file-namer.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/logger.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/report-ui-features.js', 'utf8'), diff --git a/lighthouse-core/report/html/renderer/details-renderer.js b/lighthouse-core/report/html/renderer/details-renderer.js index 91a8384b125f..6226ac11c12b 100644 --- a/lighthouse-core/report/html/renderer/details-renderer.js +++ b/lighthouse-core/report/html/renderer/details-renderer.js @@ -16,7 +16,7 @@ */ 'use strict'; -/* globals self CriticalRequestChainRenderer Util URL */ +/* globals self CriticalRequestChainRenderer SnippetRenderer Util URL */ /** @typedef {import('./dom.js')} DOM */ /** @typedef {LH.Audit.Details.Opportunity} OpportunityDetails */ @@ -43,7 +43,7 @@ class DetailsRenderer { } /** - * @param {DetailsJSON|OpportunityDetails} details + * @param {DetailsJSON|OpportunityDetails|LH.Audit.Details.Snippet} details * @return {Element|null} */ render(details) { @@ -68,6 +68,15 @@ class DetailsRenderer { case 'table': // @ts-ignore - TODO(bckenny): Fix type hierarchy return this._renderTable(/** @type {TableDetailsJSON} */ (details)); + case 'list': + return this._renderList( + // @ts-ignore + /** @type {LH.Audit.Details.List} */ (details) + ); + case 'snippet': + return SnippetRenderer.render(this._dom, this._templateContext, + // @ts-ignore + /** @type {LH.Audit.Details.Snippet} */ (details), this); case 'code': return this._renderCode(/** @type {DetailsJSON} */ (details)); case 'node': @@ -210,6 +219,21 @@ class DetailsRenderer { return element; } + /** + * @param {LH.Audit.Details.List} details + * @returns {Element} + */ + _renderList(details) { + const listContainer = this._dom.createElement('div', 'lh-list'); + + details.items.forEach(item => { + // @ts-ignore TODO(bckenny): this can never be null + listContainer.appendChild(this.render(item)); + }); + + return listContainer; + } + /** * @param {TableDetailsJSON} details * @return {Element} @@ -455,6 +479,12 @@ if (typeof module !== 'undefined' && module.exports) { }} TableDetailsJSON */ +/** @typedef {{ + type: string, + items: Array + }} ListDetailsJSON + */ + /** @typedef {{ type: string, value: string, diff --git a/lighthouse-core/report/html/renderer/snippet-renderer.js b/lighthouse-core/report/html/renderer/snippet-renderer.js new file mode 100644 index 000000000000..9003e1e3f258 --- /dev/null +++ b/lighthouse-core/report/html/renderer/snippet-renderer.js @@ -0,0 +1,343 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* globals self, Util */ + +/** @typedef {import('./details-renderer')} DetailsRenderer */ + +/** @enum {number} */ +const LineVisibility = { + ALWAYS: 0, + WHEN_COLLAPSED: 1, + WHEN_EXPANDED: 2, +}; + +/** @enum {number} */ +const LineContentType = { + /** A line of code */ + CODE_NORMAL: 0, + /** A line of code that's emphasized by setting the CSS background color */ + CODE_HIGHLIGHTED: 1, + /** Use when some lines are hidden, shows the "..." placeholder */ + PLACEHOLDER: 2, + /** A message about a line of code or the snippet in general */ + MESSAGE: 3, +}; + +/** @typedef {{ + content: string; + lineNumber: string | number; + contentType: LineContentType; + truncated?: boolean; + visibility?: LineVisibility; +}} LineDetails */ + +const classNameByContentType = { + [LineContentType.CODE_NORMAL]: ['lh-snippet__line--content'], + [LineContentType.CODE_HIGHLIGHTED]: [ + 'lh-snippet__line--content', + 'lh-snippet__line--highlighted', + ], + [LineContentType.PLACEHOLDER]: ['lh-snippet__line--placeholder'], + [LineContentType.MESSAGE]: ['lh-snippet__line--message'], +}; + +/** + * @param {LH.Audit.Details.Snippet['lines']} lines + * @param {number} lineNumber + * @return {{line?: LH.Audit.Details.Snippet['lines'][0], previousLine?: LH.Audit.Details.Snippet['lines'][0]}} + */ +function getLineAndPreviousLine(lines, lineNumber) { + return { + line: lines.find(l => l.lineNumber === lineNumber), + previousLine: lines.find(l => l.lineNumber === lineNumber - 1), + }; +} + +/** + * @param {LH.Audit.Details.Snippet["lineMessages"]} messages + * @param {number} lineNumber + */ +function getMessagesForLineNumber(messages, lineNumber) { + return messages.filter(h => h.lineNumber === lineNumber); +} + +class SnippetRenderer { + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.Snippet} details + * @param {DetailsRenderer} detailsRenderer + * @param {function} toggleExpandedFn + * @return {DocumentFragment} + */ + static renderHeader(dom, tmpl, details, detailsRenderer, toggleExpandedFn) { + const {lineCount, title} = details; + const showAll = lineCount <= 4; + + const header = dom.cloneTemplate('#tmpl-lh-snippet__header', tmpl); + dom.find('.lh-snippet__title', header).textContent = title; + + const { + snippetCollapseButtonLabel, + snippetExpandButtonLabel, + } = Util.UIStrings; + dom.find( + '.lh-snippet__btn-label-collapse', + header + ).textContent = snippetCollapseButtonLabel; + dom.find( + '.lh-snippet__btn-label-expand', + header + ).textContent = snippetExpandButtonLabel; + + const toggleExpandButton = dom.find('.lh-snippet__toggle-expand', header); + // If we're already showing all the lines of the snippet, we don't need an expand/collapse + // button and can remove it from the DOM. + // If we leave the button in though, wire up the click listener to toggle visibility! + if (showAll) { + toggleExpandButton.remove(); + } else { + toggleExpandButton.addEventListener('click', () => toggleExpandedFn()); + } + + if (details.node && dom.isDevTools()) { + const nodeContainer = dom.find('.lh-snippet__node', header); + nodeContainer.appendChild(detailsRenderer.renderNode(details.node)); + } + + return header; + } + + /** + * Renders a line of DOM content (code, message, or empty line) + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LineDetails} lineDetails + * @return {Element} + */ + static renderSnippetLine( + dom, + tmpl, + {content, lineNumber, truncated, contentType, visibility} + ) { + const clonedTemplate = dom.cloneTemplate('#tmpl-lh-snippet__line', tmpl); + const codeLine = dom.find('.lh-snippet__line', clonedTemplate); + const {classList} = codeLine; + + classNameByContentType[contentType].forEach(typeClass => + classList.add(typeClass) + ); + + if (visibility === LineVisibility.WHEN_COLLAPSED) { + classList.add('lh-snippet__show-if-collapsed'); + } else if (visibility === LineVisibility.WHEN_EXPANDED) { + classList.add('lh-snippet__show-if-expanded'); + } + + const lineContent = content + (truncated ? '…' : ''); + const lineContentEl = dom.find('.lh-snippet__line code', codeLine); + if (contentType === LineContentType.MESSAGE) { + lineContentEl.appendChild(dom.convertMarkdownLinkSnippets(lineContent)); + } else { + lineContentEl.textContent = lineContent; + } + + dom.find( + '.lh-snippet__line-number', + codeLine + ).textContent = lineNumber.toString(); + + return codeLine; + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {{message: string}} message + * @return {Element} + */ + static renderMessage(dom, tmpl, message) { + return SnippetRenderer.renderSnippetLine(dom, tmpl, { + lineNumber: ' ', + content: message.message, + contentType: LineContentType.MESSAGE, + }); + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LineVisibility} visibility + * @return {Element} + */ + static renderOmittedLinesPlaceholder(dom, tmpl, visibility) { + return SnippetRenderer.renderSnippetLine(dom, tmpl, { + lineNumber: '…', + content: '', + visibility, + contentType: LineContentType.PLACEHOLDER, + }); + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.Snippet} details + * @return {DocumentFragment} + */ + static renderSnippetContent(dom, tmpl, details) { + const template = dom.cloneTemplate('#tmpl-lh-snippet__content', tmpl); + const snippetEl = dom.find('.lh-snippet__snippet-inner', template); + + details.generalMessages.forEach(m => + snippetEl.append(SnippetRenderer.renderMessage(dom, tmpl, m)) + ); + + snippetEl.append(SnippetRenderer.renderSnippetLines(dom, tmpl, details)); + + return template; + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.Snippet} details + * @returns {DocumentFragment} + */ + static renderSnippetLines(dom, tmpl, details) { + const {lineMessages, generalMessages, lineCount, lines} = details; + const linesWhenCollapsed = Util.filterRelevantLines(lines, lineMessages, 2); + const hasOnlyGeneralMessages = + generalMessages.length > 0 && lineMessages.length === 0; + + const lineContainer = dom.createFragment(); + + // When a line is not shown in the collapsed state we try to see if we also need an + // omitted lines placeholder for the expanded state, rather than rendering two separate + // placeholders. + let hasPendingOmittedLinesPlaceholderForCollapsedState = false; + + for (let lineNumber = 1; lineNumber <= lineCount; lineNumber++) { + const {line, previousLine} = getLineAndPreviousLine(lines, lineNumber); + const { + line: lineWhenCollapsed, + previousLine: previousLineWhenCollapsed, + } = getLineAndPreviousLine(linesWhenCollapsed, lineNumber); + + const showLineWhenCollapsed = !!lineWhenCollapsed; + const showPreviousLineWhenCollapsed = !!previousLineWhenCollapsed; + + // If we went from showing lines in the collapsed state to not showing them + // we need to render a placeholder + if (showPreviousLineWhenCollapsed && !showLineWhenCollapsed) { + hasPendingOmittedLinesPlaceholderForCollapsedState = true; + } + // If we are back to lines being visible in the collapsed state then + // render the placeholder if it hasn't alerady been rendered + if ( + showLineWhenCollapsed && + hasPendingOmittedLinesPlaceholderForCollapsedState + ) { + lineContainer.append( + SnippetRenderer.renderOmittedLinesPlaceholder( + dom, + tmpl, + LineVisibility.WHEN_COLLAPSED + ) + ); + hasPendingOmittedLinesPlaceholderForCollapsedState = false; + } + + // Render omitted lines placeholder if we have not already rendered one for this gap + const isFirstOmittedLineWhenExpanded = !line && !!previousLine; + const isFirstLineOverallAndIsOmittedWhenExpanded = + !line && lineNumber === 1; + if ( + isFirstOmittedLineWhenExpanded || + isFirstLineOverallAndIsOmittedWhenExpanded + ) { + // In the collapsed state we don't show omitted lines placeholders around + // the edges of the snippet + const renderedAllLinesVisibleInCollapsedState = !linesWhenCollapsed.some( + l => l.lineNumber > lineNumber + ); + const onlyShowWhenExpanded = + renderedAllLinesVisibleInCollapsedState || lineNumber === 1; + lineContainer.append( + SnippetRenderer.renderOmittedLinesPlaceholder( + dom, + tmpl, + onlyShowWhenExpanded + ? LineVisibility.WHEN_EXPANDED + : LineVisibility.ALWAYS + ) + ); + hasPendingOmittedLinesPlaceholderForCollapsedState = false; + } + + if (!line) { + // Can't render the line if we don't know its content (instead we've rendered a placeholder) + continue; + } + + // Now render the line and any highlights + const messages = getMessagesForLineNumber(lineMessages, lineNumber); + const highlightLine = messages.length > 0 || hasOnlyGeneralMessages; + const codeLineDetails = Object.assign({}, line, { + contentType: highlightLine + ? LineContentType.CODE_HIGHLIGHTED + : LineContentType.CODE_NORMAL, + visibility: lineWhenCollapsed + ? LineVisibility.ALWAYS + : LineVisibility.WHEN_EXPANDED, + }); + lineContainer.append( + SnippetRenderer.renderSnippetLine(dom, tmpl, codeLineDetails) + ); + + messages.forEach(message => { + lineContainer.append( + SnippetRenderer.renderMessage(dom, tmpl, message) + ); + }); + } + + return lineContainer; + } + + /** + * @param {DOM} dom + * @param {ParentNode} templateContext + * @param {LH.Audit.Details.Snippet} details + * @param {DetailsRenderer} detailsRenderer + * @return {Element} + */ + static render(dom, templateContext, details, detailsRenderer) { + const tmpl = dom.cloneTemplate('#tmpl-lh-snippet', templateContext); + const snippetEl = dom.find('.lh-snippet', tmpl); + + const header = SnippetRenderer.renderHeader( + dom, + tmpl, + details, + detailsRenderer, + () => snippetEl.classList.toggle('lh-snippet--expanded') + ); + const content = SnippetRenderer.renderSnippetContent(dom, tmpl, details); + snippetEl.append(header, content); + + return snippetEl; + } +} + +// Allow Node require()'ing. +if (typeof module !== 'undefined' && module.exports) { + module.exports = SnippetRenderer; +} else { + self.SnippetRenderer = SnippetRenderer; +} diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js index 6a752d517f34..721aeb33adc5 100644 --- a/lighthouse-core/report/html/renderer/util.js +++ b/lighthouse-core/report/html/renderer/util.js @@ -449,6 +449,50 @@ class Util { // When testing, use a locale with more exciting numeric formatting if (Util.numberDateLocale === 'en-XA') Util.numberDateLocale = 'de'; } + + /** + * Returns only lines that are near a message, or the first few lines if there are + * no line messages. + * @param {LH.Audit.Details.Snippet['lines']} lines + * @param {LH.Audit.Details.Snippet['lineMessages']} lineMessages + * @param {number} surroundingLineCount Number of lines to include before and after + * the message. If this is e.g. 2 this function might return 5 lines. + */ + static filterRelevantLines(lines, lineMessages, surroundingLineCount) { + if (lineMessages.length === 0) { + // no lines with messages, just return the first bunch of lines + return lines.slice(0, surroundingLineCount * 2 + 1); + } + + const minGapSize = 3; + const lineNumbersToKeep = new Set(); + // Sort messages so we can check lineNumbersToKeep to see how big the gap to + // the previous line is. + lineMessages = lineMessages.sort((a, b) => (a.lineNumber || 0) - (b.lineNumber || 0)); + lineMessages.forEach(({lineNumber}) => { + if (!lineNumber) { + return; + } + let firstSurroundingLineNumber = lineNumber - surroundingLineCount; + let lastSurroundingLineNumber = lineNumber + surroundingLineCount; + while (firstSurroundingLineNumber < 1) { + // make sure we still show (surroundingLineCount * 2 + 1) lines in total + firstSurroundingLineNumber++; + lastSurroundingLineNumber++; + } + // If only a few lines would be omitted normally then we prefer to include + // extra lines to avoid the tiny gap + if (lineNumbersToKeep.has(firstSurroundingLineNumber - minGapSize - 1)) { + firstSurroundingLineNumber -= minGapSize; + } + for (let i = firstSurroundingLineNumber; i <= lastSurroundingLineNumber; i++) { + const surroundingLineNumber = i; + lineNumbersToKeep.add(surroundingLineNumber); + } + }); + + return lines.filter(line => lineNumbersToKeep.has(line.lineNumber)); + } } /** @@ -496,6 +540,11 @@ Util.UIStrings = { /** Label of value shown in the summary of critical request chains. Refers to the total amount of time (milliseconds) of the longest critical path chain/sequence of network requests. Example value: 2310 ms */ crcLongestDurationLabel: 'Maximum critical path latency:', + /** Label for button that shows all lines of the snippet when clicked */ + snippetExpandButtonLabel: 'Expand snippet', + /** Label for button that only shows a few lines of the snippet when clicked */ + snippetCollapseButtonLabel: 'Collapse snippet', + /** Explanation shown to users below performance results to inform them that the test was done with a 4G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation. */ lsPerformanceCategoryDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on an emulated mobile network. Values are estimated and may vary.', /** Title of the lab data section of the Performance category. Within this section are various speed metrics which quantify the pageload performance into values presented in seconds and milliseconds. "Lab" is an abbreviated form of "laboratory", and refers to the fact that the data is from a controlled test of a website, not measurements from real users visiting that site. */ diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css index 8ece38dc6be7..7915e7bd05cf 100644 --- a/lighthouse-core/report/html/report-styles.css +++ b/lighthouse-core/report/html/report-styles.css @@ -725,6 +725,10 @@ margin-top: var(--section-padding); } +.lh-list > div:not(:last-child) { + padding-bottom: 20px; +} + .lh-header-container { display: block; margin: 0 auto; diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html index 7b5362bec540..b68e62abd1e3 100644 --- a/lighthouse-core/report/html/templates.html +++ b/lighthouse-core/report/html/templates.html @@ -892,3 +892,142 @@ + + + + + diff --git a/lighthouse-core/test/audits/audit-test.js b/lighthouse-core/test/audits/audit-test.js index 0e4251a2ca0d..33e806bf849e 100644 --- a/lighthouse-core/test/audits/audit-test.js +++ b/lighthouse-core/test/audits/audit-test.js @@ -99,4 +99,100 @@ describe('Audit', () => { assert.equal(result.score, null); assert.equal(result.scoreDisplayMode, 'error'); }); + + describe('makeSnippetDetails', () => { + const maxLinesAroundMessage = 10; + + it('Transforms code to lines array', () => { + const details = Audit.makeSnippetDetails({ + content: 'a\nb\nc', + title: 'Title', + lineMessages: [], + generalMessages: [], + }); + + assert.equal(details.lines.length, 3); + assert.deepEqual(details.lines[1], { + lineNumber: 2, + content: 'b', + }); + }); + + it('Truncates long lines', () => { + const details = Audit.makeSnippetDetails({ + content: Array(1001).join('-'), + title: 'Title', + lineMessages: [], + generalMessages: [], + }); + + assert.equal(details.lines[0].truncated, true); + assert.ok(details.lines[0].content.length < 1000); + }); + + function makeLines(lineCount) { + return Array(lineCount + 1).join('-\n'); + } + + it('Limits the number of lines if there are no line-specific highlights', () => { + const details = Audit.makeSnippetDetails({ + content: makeLines(100), + title: 'Title', + lineMessages: [], + generalMessages: [{ + message: 'General', + }], + maxLinesAroundMessage, + }); + expect(details.lines.length).toBe(2 * maxLinesAroundMessage + 1); + }); + + it('Does not omit lines if fewer than 4 lines would be omitted', () => { + const details = Audit.makeSnippetDetails({ + content: makeLines(200), + title: 'Title', + lineMessages: [ + // without the special logic for small gaps lines 71-73 would be missing + { + lineNumber: 84, + message: 'Highlight 2', + }, { + lineNumber: 60, + message: 'Highlight 1', + }], + generalMessages: [], + maxLinesAroundMessage, + }); + + const normalExpectedLineNumber = 2 * (maxLinesAroundMessage * 2 + 1); + assert.equal(details.lines.length, normalExpectedLineNumber + 3); + }); + + it('Limits the number of lines around highlights', () => { + const content = makeLines(99) + 'A\n' + makeLines(99) + '\nB'; + const allLines = content.split('\n'); + const details = Audit.makeSnippetDetails({ + content, + title: 'Title', + lineMessages: [{ + lineNumber: allLines.findIndex(l => l === 'A') + 1, + message: 'a', + }, { + lineNumber: allLines.findIndex(l => l === 'B') + 1, + message: 'b', + }], + generalMessages: [], + maxLinesAroundMessage, + }); + + // 2 highlighted lines and their surounding lines, second highlight only has preceding lines + const lineCount = maxLinesAroundMessage * 3 + 2; + assert.equal(details.lines.length, lineCount); + const lastLine = details.lines.slice(-1)[0]; + assert.deepEqual(lastLine, { + lineNumber: 201, + content: 'B', + }); + }); + }); }); diff --git a/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js b/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js new file mode 100644 index 000000000000..08e2cdcbbff9 --- /dev/null +++ b/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js @@ -0,0 +1,182 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* eslint-env jest */ + +const assert = require('assert'); +const fs = require('fs'); +const jsdom = require('jsdom'); +const Util = require('../../../../report/html/renderer/util.js'); +const DOM = require('../../../../report/html/renderer/dom.js'); +const SnippetRenderer = require('../../../../report/html/renderer/snippet-renderer.js'); + +const TEMPLATE_FILE = fs.readFileSync( + __dirname + '/../../../../report/html/templates.html', + 'utf8' +); + +function makeDetails(lineMessages, generalMessages = [], lineRanges = [{from: 1, to: 6}]) { + const lines = []; + lineRanges.forEach(({from, to}) => { + for (let i = from; i <= to; i++) { + lines.push({ + lineNumber: i, + content: 'L' + i, + }); + } + }); + + + return { + type: 'snippet', + lines, + lineMessages, + generalMessages, + lineCount: 100, + }; +} + +describe('DetailsRenderer', () => { + let dom; + + beforeAll(() => { + global.Util = Util; + const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window; + dom = new DOM(document); + }); + + afterAll(() => { + global.Util = undefined; + }); + + function renderSnippet(lineMessages, generalMessages, lineRanges = undefined) { + const details = makeDetails(lineMessages, generalMessages, lineRanges); + const el = SnippetRenderer.render(dom, dom.document(), details, {}); + + return { + contentLines: el.querySelectorAll('.lh-snippet__line--content'), + collapsedContentLines: el.querySelectorAll( + '.lh-snippet__line--content.lh-snippet__show-if-expanded' + ), + uncollapsedContentLines: el.querySelectorAll( + '.lh-snippet__line--content:not(.lh-snippet__show-if-expanded)' + ), + messageLines: el.querySelectorAll('.lh-snippet__line--message'), + omittedLinesIndicatorsWhenExpanded: el.querySelectorAll( + '.lh-snippet__line--placeholder:not(.lh-snippet__show-if-collapsed)' + ), + omittedLinesIndicatorsWhenCollapsed: el.querySelectorAll( + '.lh-snippet__line--placeholder:not(.lh-snippet__show-if-expanded)' + ), + }; + } + + it('Renders snippet with message at the very top', () => { + const {contentLines, messageLines, collapsedContentLines} = renderSnippet([ + { + lineNumber: 1, + message: 'Error', + }, + ]); + + // 5 lines are visible, 1 is collapsed + assert.equal(collapsedContentLines.length, 1); + // All available lines are shown on expansion + assert.equal(contentLines.length, 6); + // 100 lines in total, so lines towards the end won't be shown + const lastLine = contentLines[contentLines.length - 1]; + assert.equal(lastLine.nextSibling.textContent.trim(), '…'); + + // Shows message for second line + assert.equal(messageLines[0].textContent.trim(), 'Error'); + assert.equal(messageLines[0].previousSibling.textContent.replace(/\s/g, ''), '1L1'); + }); + + it('Renders first few lines if there are no messages', () => { + const { + uncollapsedContentLines, + omittedLinesIndicatorsWhenExpanded, + omittedLinesIndicatorsWhenCollapsed, + } = renderSnippet([]); + const lastUncollapsedLine = uncollapsedContentLines[uncollapsedContentLines.length - 1]; + + // Shows first 5 visible lines + assert.equal(lastUncollapsedLine.textContent.replace(/\s/g, ''), '5L5'); + // "..." after the available lines, but only shows in expanded state + assert.equal(omittedLinesIndicatorsWhenExpanded.length, 1); + assert.equal(omittedLinesIndicatorsWhenCollapsed.length, 0); + }); + + it('Renders first few lines if there are no messages for specific lines', () => { + const {uncollapsedContentLines} = renderSnippet([ + { + lineNumber: null, + message: 'General error', + }, + ]); + const lastUncollapsedLine = uncollapsedContentLines[uncollapsedContentLines.length - 1]; + + // Shows first 5 visible lines + assert.equal(lastUncollapsedLine.textContent.replace(/\s/g, ''), '5L5'); + }); + + it('Renders snippet with multiple messages surrounded by other lines', () => { + const { + collapsedContentLines, + omittedLinesIndicatorsWhenCollapsed, + omittedLinesIndicatorsWhenExpanded, + } = renderSnippet( + [ + { + lineNumber: 40, + message: 'Error 1', + }, + { + lineNumber: 70, + message: 'Error 2', + }, + ], + [], + [ + { + from: 30, + to: 50, + }, + { + from: 60, + to: 80, + }, + ] + ); + + // first available line is collapsed + assert.equal(collapsedContentLines[0].textContent.replace(/\s/g, ''), '30L30'); + + // puts omitted lines placeholder between the two messages + assert.equal(omittedLinesIndicatorsWhenCollapsed.length, 1); + // puts omitted lines placeholder between the two messages and around the whole snippet + assert.equal(omittedLinesIndicatorsWhenExpanded.length, 3); + }); + + it('Can render both line-specific and non line-specific messages in one snippet', () => { + const {messageLines} = renderSnippet( + [ + { + lineNumber: 5, + message: 'Error on line', + }, + ], + [ + { + message: 'General error', + }, + ] + ); + + assert.equal(messageLines.length, 2); + }); +}); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 950cfedddf29..f9b75f85eef3 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -4836,6 +4836,8 @@ "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", "scorescaleLabel": "Score scale:", + "snippetCollapseButtonLabel": "Collapse snippet", + "snippetExpandButtonLabel": "Expand snippet", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.", "warningAuditsGroupTitle": "Passed audits but with warnings", diff --git a/package.json b/package.json index 23e6564f43bb..99831c2e16a7 100644 --- a/package.json +++ b/package.json @@ -187,7 +187,7 @@ }, { "path": "./dist/viewer/src/viewer.js", - "threshold": "65 Kb" + "threshold": "70 Kb" } ], "nyc": { diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index df0658e7e92a..fcc45bcb1ee6 100644 --- a/proto/lighthouse-result.proto +++ b/proto/lighthouse-result.proto @@ -334,6 +334,12 @@ message I18n { // The heading that is shown above a list of audits that have warnings string warning_audits_group_title = 17; + + // The label for the button to show all lines of a snippet + string snippet_expand_button_label = 18; + + // The label for the button to show only a few lines of a snippet + string snippet_collapse_button_label = 19; } // The message holding all formatted strings diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index f365ce24c794..0fc470ce8292 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -3731,6 +3731,8 @@ "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", "scorescaleLabel": "Score scale:", + "snippetCollapseButtonLabel": "Collapse snippet", + "snippetExpandButtonLabel": "Expand snippet", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.", "warningAuditsGroupTitle": "Passed audits but with warnings", diff --git a/types/audit-details.d.ts b/types/audit-details.d.ts index f2dd8a329ee0..f1eaae5c31c1 100644 --- a/types/audit-details.d.ts +++ b/types/audit-details.d.ts @@ -65,6 +65,11 @@ declare global { diagnostic?: Diagnostic; } + export interface List { + type: 'list'; + items: Snippet[] + } + /** * A details type that is not rendered in the final report; usually used * for including diagnostic information in the LHR. Can contain anything. @@ -164,6 +169,27 @@ declare global { type: 'url'; value: string; } + + export interface Snippet { + type: "snippet", + lines: { + content: string + /** Line number, starting from 1 */ + lineNumber: number; + truncated?: boolean + }[], + title: string, + lineMessages: { + /** Line number, starting from 1 */ + lineNumber: number, + message: string + }[]; + generalMessages: { + message: string + }[]; + lineCount: number, + node?: NodeValue, + } } } } diff --git a/types/html-renderer.d.ts b/types/html-renderer.d.ts index 6dcc65d110a8..453b24399eba 100644 --- a/types/html-renderer.d.ts +++ b/types/html-renderer.d.ts @@ -6,6 +6,7 @@ import _CategoryRenderer = require('../lighthouse-core/report/html/renderer/category-renderer.js'); import _CriticalRequestChainRenderer = require('../lighthouse-core/report/html/renderer/crc-details-renderer.js'); +import _SnippetRenderer = require('../lighthouse-core/report/html/renderer/snippet-renderer.js'); import _DetailsRenderer = require('../lighthouse-core/report/html/renderer/details-renderer.js'); import _DOM = require('../lighthouse-core/report/html/renderer/dom.js'); import _PerformanceCategoryRenderer = require('../lighthouse-core/report/html/renderer/performance-category-renderer.js'); @@ -19,6 +20,7 @@ import _FileNamer = require('../lighthouse-core/lib/file-namer.js'); declare global { var CategoryRenderer: typeof _CategoryRenderer; var CriticalRequestChainRenderer: typeof _CriticalRequestChainRenderer; + var SnippetRenderer: typeof _SnippetRenderer; var DetailsRenderer: typeof _DetailsRenderer; var DOM: typeof _DOM; var getFilenamePrefix: typeof _FileNamer.getFilenamePrefix; @@ -32,6 +34,7 @@ declare global { interface Window { CategoryRenderer: typeof _CategoryRenderer; CriticalRequestChainRenderer: typeof _CriticalRequestChainRenderer; + SnippetRenderer: typeof _SnippetRenderer; DetailsRenderer: typeof _DetailsRenderer; DOM: typeof _DOM; PerformanceCategoryRenderer: typeof _PerformanceCategoryRenderer; From 281791d020317faa48759ae8df017f85022d2c84 Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Sat, 2 Feb 2019 07:52:38 +0000 Subject: [PATCH 02/20] JSON LD Stuff --- build/build-bundle.js | 8 +- jslondtest.js | 33 + .../test/fixtures/seo/seo-failure-cases.html | 20 + .../test/fixtures/seo/seo-tester.html | 42 + .../test/smokehouse/seo/expectations.js | 11 + .../audits/seo/structured-data-automatic.js | 172 + lighthouse-core/config/default-config.js | 3 + .../gather/gatherers/seo/json-ld.js | 33 + package.json | 7 +- sd-validation/assets/jsonldcontext.json | 2284 ++++ sd-validation/assets/schema-tree.json | 9900 +++++++++++++++++ sd-validation/expand.js | 80 + sd-validation/helpers/walkObject.js | 31 + sd-validation/index.js | 141 + sd-validation/json.js | 53 + sd-validation/jsonld.js | 71 + sd-validation/package.json | 17 + sd-validation/schema.js | 149 + sd-validation/scripts/generate-schema-tree.js | 62 + sd-validation/test/json-validation-test.js | 62 + sd-validation/test/jsonld-validation-test.js | 67 + .../test/schema-org-validation-test.js | 86 + sd-validation/typings/jsonld.d.ts | 9 + sd-validation/typings/jsonlint-mod.d.ts | 3 + tsconfig.json | 1 + types/artifacts.d.ts | 2 + yarn.lock | 104 + 27 files changed, 13448 insertions(+), 3 deletions(-) create mode 100644 jslondtest.js create mode 100644 lighthouse-core/audits/seo/structured-data-automatic.js create mode 100644 lighthouse-core/gather/gatherers/seo/json-ld.js create mode 100644 sd-validation/assets/jsonldcontext.json create mode 100644 sd-validation/assets/schema-tree.json create mode 100644 sd-validation/expand.js create mode 100644 sd-validation/helpers/walkObject.js create mode 100644 sd-validation/index.js create mode 100644 sd-validation/json.js create mode 100644 sd-validation/jsonld.js create mode 100644 sd-validation/package.json create mode 100644 sd-validation/schema.js create mode 100644 sd-validation/scripts/generate-schema-tree.js create mode 100644 sd-validation/test/json-validation-test.js create mode 100644 sd-validation/test/jsonld-validation-test.js create mode 100644 sd-validation/test/schema-org-validation-test.js create mode 100644 sd-validation/typings/jsonld.d.ts create mode 100644 sd-validation/typings/jsonlint-mod.d.ts diff --git a/build/build-bundle.js b/build/build-bundle.js index 2426210f48f3..ca53521e2d37 100644 --- a/build/build-bundle.js +++ b/build/build-bundle.js @@ -64,7 +64,13 @@ async function browserifyFile(entryPath, distPath) { .ignore('raven') .ignore('mkdirp') .ignore('rimraf') - .ignore('pako/lib/zlib/inflate.js'); + .ignore('pako/lib/zlib/inflate.js') + .ignore('file') + .ignore('system'); + + // there is no way to add './doug-json-parse' to ignored packages via public API + // w/o browserify resolving the path into an absolute path + bundle._ignore.push('./doug-json-parse'); // Don't include the desktop protocol connection. bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js')); diff --git a/jslondtest.js b/jslondtest.js new file mode 100644 index 000000000000..e941b15edd97 --- /dev/null +++ b/jslondtest.js @@ -0,0 +1,33 @@ +'use strict'; + +const jsonld = require('jsonld'); + +jsonld.expand({ + '@context': { + 'image': { + '@id': '@error', + }, + }, +}); + +// const expanded = [ +// { +// '@type': [ +// 'http://schema.org/Cat', +// ], +// 'http://schema.org/items': [ +// { +// '@value': 'a', +// '@type': 'http://other.com/sth', +// }, +// { +// '@value': 'b', +// }, +// ], +// }, +// ]; + +// jsonld.compact(expanded, 'http://schema.org').then(res => { +// console.log(res); +// }) +// ; diff --git a/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html b/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html index b3713a90040d..a9a0ef2633a6 100644 --- a/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html +++ b/lighthouse-cli/test/fixtures/seo/seo-failure-cases.html @@ -20,6 +20,26 @@ + + + + + +

SEO

diff --git a/lighthouse-cli/test/fixtures/seo/seo-tester.html b/lighthouse-cli/test/fixtures/seo/seo-tester.html index 6f31dcae289e..01dd7bd1cb38 100644 --- a/lighthouse-cli/test/fixtures/seo/seo-tester.html +++ b/lighthouse-cli/test/fixtures/seo/seo-tester.html @@ -19,6 +19,48 @@ + + +