Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new_audit: json-ld audit #8328

Closed
wants to merge 47 commits into from
Closed
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
47e7427
Structured-data-automatic-audit
mattzeunert Apr 16, 2019
14e5d76
Name tweak
mattzeunert Apr 16, 2019
19e5511
File extension for require
mattzeunert Apr 16, 2019
e407bba
Tweak description
patrickhulce Apr 16, 2019
5be7b74
PR feedback
mattzeunert Apr 16, 2019
7da5fa5
Merge branch 'master' into structured-data-automatic-2
mattzeunert Apr 16, 2019
a1470cf
Fix sample json
mattzeunert Apr 16, 2019
39377d0
Merge branch 'structured-data-automatic-2' of https://github.com/Goog…
mattzeunert Apr 16, 2019
f7cb682
Move type parsing into sd validation module
mattzeunert Apr 16, 2019
b7cdf44
Update sample json
mattzeunert Apr 16, 2019
c9c8672
Fix browserify build
mattzeunert Apr 17, 2019
e0563f9
Fix validTypes test
mattzeunert Apr 17, 2019
b19564c
Merge branch 'master' into structured-data-automatic-2
mattzeunert Apr 17, 2019
3d4633f
Remove rawValue
mattzeunert Apr 17, 2019
ea65f67
Not async anymore
mattzeunert Apr 21, 2019
416d60b
More i18n
mattzeunert Apr 21, 2019
cacabd2
Revert "More i18n"
mattzeunert Apr 21, 2019
0ff4070
Type instead of ||
mattzeunert Apr 21, 2019
1c15871
Explain why no snippet title i18n
mattzeunert Apr 22, 2019
82d2785
Types for JsonLD
mattzeunert Apr 26, 2019
46ad970
Tweak displaystring
mattzeunert Apr 26, 2019
2d42a4c
Merge branch 'master' into structured-data-automatic-2
mattzeunert Apr 26, 2019
534f10f
Rename structured-data-automatic to json-ld
mattzeunert Apr 26, 2019
1243e70
Basic test case for when entity name is an object
mattzeunert Apr 27, 2019
6e9a8d8
Merge branch 'master' into structured-data-automatic-2
mattzeunert May 2, 2019
41c8e28
Merge branch 'master' into structured-data-automatic-2
mattzeunert May 14, 2019
ad58799
Add JSON-LD failure to sample json
mattzeunert May 14, 2019
96cf12d
Revert "Add JSON-LD failure to sample json"
mattzeunert May 15, 2019
4d8198a
Try undo merge
mattzeunert May 15, 2019
fc3b034
Merge branch 'master' into structured-data-automatic-2
mattzeunert May 15, 2019
f65b807
Add JSON-LD failure to sample json
mattzeunert May 14, 2019
991bf3c
Description tweaks
mattzeunert May 17, 2019
87e2897
Merge branch 'master' into structured-data-automatic-2
mattzeunert May 18, 2019
aac8717
Merge branch 'master' into structured-data-automatic-2
paulirish Aug 8, 2019
2ac1ace
sample json
paulirish Aug 8, 2019
8d63bf2
Addresses comments for JSON-LD audit
Aug 13, 2019
055065a
Fixes snippet theme for dark mode
Aug 13, 2019
7a3ad59
Adjusts strings for JSON-LD in sample json
Sep 16, 2019
b578c23
Merge branch 'master' into structured-data-automatic-2
AVGP Sep 16, 2019
53a932d
Fixes SEO smoke test expectations
Sep 16, 2019
72fa128
Merge branch 'master' into structured-data-automatic-2
paulirish Jan 28, 2020
d84fb24
update smoke expectations
devtools-bot Jan 28, 2020
9e74c1f
add expectation for failing jsonld details
devtools-bot Jan 28, 2020
e51903b
eslint
paulirish Jan 28, 2020
0bad7c4
Merge branch 'master' into structured-data-automatic-2
paulirish Apr 14, 2020
88a234d
color fixups with yuin's design
paulirish Apr 14, 2020
8849076
bump bundlesize numbers
paulirish Apr 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion build/build-bundle.js
Expand Up @@ -64,7 +64,14 @@ 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') // required by jsonlint-mod
.ignore('system'); // required by jsonlint-mod

// 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
AVGP marked this conversation as resolved.
Show resolved Hide resolved
// @ts-ignore
bundle._ignore.push('./doug-json-parse');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like a recipe for struggles on next browserify update 😬

maybe it's time to investigate moving to something better maintained/more flexible post 5.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still a bit concerned about this but I think whatever we end up doing will be hacky in some way 🤷‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a test that the bundle output does not contain some code in douge-json-parse? so we remember to fix this when it break.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a test that the bundle output does not contain some code in douge-json-parse? so we remember to fix this when it break.

the build throws if we forget this, so it will definitely fail the tests :)

The issue is that jsonlint-mod's bundled js has a reference to this file, which isn't included in the module. Browserify throws when it tries to resolve it.


// Don't include the desktop protocol connection.
bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js'));
Expand Down
8 changes: 8 additions & 0 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Expand Up @@ -378,6 +378,9 @@ Object {
Object {
"path": "seo/canonical",
},
Object {
"path": "seo/json-ld",
},
Object {
"path": "seo/manual/structured-data",
},
Expand Down Expand Up @@ -1001,6 +1004,11 @@ Object {
"id": "canonical",
"weight": 1,
},
Object {
"group": "seo-content",
"id": "json-ld",
"weight": 1,
},
Object {
"group": "seo-mobile",
"id": "font-size",
Expand Down
9 changes: 9 additions & 0 deletions lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html
Expand Up @@ -108,6 +108,15 @@
<!-- Force FCP to be ~5s out, necessary to make sure our render-blocking audits still work -->
<!-- even when other forces were responsible for delaying the render. -->
<script src="./fcp-delayer.js?delay=5000"></script>

<!-- FAIL(json-ld): buttons too close together -->
AVGP marked this conversation as resolved.
Show resolved Hide resolved
<script type="application/ld+json">
{
"@context": "http://schema.org",
"@type": "Event",
"unknownProp": "Oh no"
}
AVGP marked this conversation as resolved.
Show resolved Hide resolved
</script>
</head>
<body>

Expand Down
8 changes: 8 additions & 0 deletions lighthouse-cli/test/fixtures/seo/seo-failure-cases.html
Expand Up @@ -20,6 +20,14 @@
<link rel="alternate" href="http://example.com/" hreflang=" x-default" />
<!-- FAIL(canonical): multiple canonical URLs provided (another one is in the HTTP header) -->
<link rel="canonical" href="https://example.com/other" />
<!-- FAIL(json-ld): invalid @type -->
<script type="application/ld+json">
{
"@context": "http://schema.org",
"@type": "CatConvention",
AVGP marked this conversation as resolved.
Show resolved Hide resolved
"name": "Cat Global"
}
</script>
</head>
<body>
<h1>SEO</h1>
Expand Down
8 changes: 8 additions & 0 deletions lighthouse-cli/test/fixtures/seo/seo-tester.html
Expand Up @@ -19,6 +19,14 @@
<link rel="alternate" href="http://example.com/" hreflang="x-default" />
<!-- PASS(canonical): valid canonical URL -->
<link rel="canonical" href="http://localhost:10200/seo/" />
<!-- PASS(json-ld): valid JSON-LD -->
<script type="application/ld+json">
{
"@context": "http://schema.org",
"@type": "Event",
"name": "Cat Global"
}
</script>

<style>
.small {
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Expand Up @@ -80,6 +80,9 @@ module.exports = [
score: null,
scoreDisplayMode: 'notApplicable',
},
'json-ld': {
score: 1,
},
},
}},
{
Expand Down Expand Up @@ -141,6 +144,9 @@ module.exports = [
score: 0,
explanation: 'Multiple conflicting URLs (https://example.com/other, https://example.com/)',
},
'json-ld': {
score: 0,
},
},
},
},
Expand Down
172 changes: 172 additions & 0 deletions lighthouse-core/audits/seo/json-ld.js
@@ -0,0 +1,172 @@
/**
* @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';

const Audit = require('../audit.js');
const validateJsonLD = require('../../lib/sd-validation/sd-validation.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on whether JSON-LD structured data snippets are valid. This descriptive title is shown when no invalid JSON-LD snippets were found. */
title: 'JSON-LD structured data syntax is valid',
/** Title of a Lighthouse audit that provides detail on whether JSON-LD structured data snippets are valid. This descriptive title is shown when JSON-LD snippets with invalid content were found. */
failureTitle: 'JSON-LD structured data syntax is invalid',
/** Description of a Lighthouse audit that tells the user whether JSON-LD snippets on the page are invalid. This is displayed after a user expands the section to see more. No character length limits. */
/* eslint-disable-next-line max-len */
description: 'Structured data contains rich metadata about a web page. The data is used in search results and social sharing. Invalid metadata will affect how the page appears in these contexts. This audit currently validates a subset of JSON-LD rules. See also the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) to validate other types of structured data.',
AVGP marked this conversation as resolved.
Show resolved Hide resolved
/** Explanatory message stating how many JSON-LD structured data snippets are invalid */
displayValue: `{invalidSnippetCount, plural,
=1 {# invalid snippet}
other {# invalid snippets}
}`,
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class StructuredDataAutomatic extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'json-ld',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ScriptElements'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts) {
const jsonLDElements = artifacts.ScriptElements.filter(
script => script.type === 'application/ld+json' && !!script.content);

if (jsonLDElements.length === 0) {
return {
notApplicable: true,
score: 1,
};
}

const validatedSnippets = await Promise.all(jsonLDElements.map(async (element) => {
AVGP marked this conversation as resolved.
Show resolved Hide resolved
// We don't want to show empty lines around the snippet
const content = /** @type string */ (element.content).trim();
AVGP marked this conversation as resolved.
Show resolved Hide resolved

return {
devtoolsNodePath: element.devtoolsNodePath,
content,
errors: await validateJsonLD(content),
};
}));
// Show invalid snippets at the top
validatedSnippets.sort((a, b) => {
return b.errors.length - a.errors.length;
});

const renderedSnippets = validatedSnippets.map(
snippetWithErrors => renderValidatedSnippet(snippetWithErrors)
AVGP marked this conversation as resolved.
Show resolved Hide resolved
);
const details = Audit.makeListDetails(renderedSnippets);

const invalidSnippets = validatedSnippets.filter(vs => vs.errors.length > 0);
const displayValue = str_(UIStrings.displayValue, {
invalidSnippetCount: invalidSnippets.length,
});

return {
score: invalidSnippets.length === 0 ? 1 : 0,
details,
displayValue,
};
}
}

/**
* @param {{content: string, devtoolsNodePath: string, errors: LH.StructuredData.ValidationError[]}} validatedSnippet
*/
function renderValidatedSnippet(validatedSnippet) {
const {content, devtoolsNodePath, errors} = validatedSnippet;

/** @type {LH.StructuredData.JsonLDDocument} */
let parsedContent = {};
let topLevelType = '';
let topLevelName = '';
try {
parsedContent = JSON.parse(content);
} catch (err) {}
if (parsedContent['@type']) {
topLevelType = parsedContent['@type'];
}
if (typeof parsedContent.name === 'string') {
topLevelName = parsedContent.name.toString();
}

let title = '';
if (topLevelName && topLevelType) {
title = `${topLevelType}: ${topLevelName}`;
} else if (topLevelType) {
title = `@type ${topLevelType}`;
} else {
title = 'Invalid JSON-LD element';
}
// No 18n here, because it's tricky to do because of the if statement above. The
// entity type and error messages are in English anyway.
title += ` (${errors.length} Error${errors.length !== 1 ? 's' : ''})`;

/** @type LH.Audit.Details.NodeValue */
const node = {
type: 'node',
path: devtoolsNodePath,
snippet: `<script type="application/ld+json">`,
};

const {lineMessages, generalMessages} = getErrorMessages(errors);

return Audit.makeSnippetDetails({
content: parsedContent ? JSON.stringify(parsedContent, null, 2) : content,
title,
lineMessages,
generalMessages,
node,
});
}

/**
* @param {Array<LH.StructuredData.ValidationError>} errors
*/
function getErrorMessages(errors) {
/** @type {LH.Audit.Details.SnippetValue['lineMessages']} */
const lineMessages = [];
/** @type {LH.Audit.Details.SnippetValue['generalMessages']} */
const generalMessages = [];
errors.forEach(({
message, lineNumber, validTypes, validator,
}) => {
if (validTypes && validator === 'schema-org') {
const typeStrings = validTypes.map(type => {
return `[${type.name}](${type.uri})`;
});
message = `Invalid ${typeStrings.join('/')}: ${message}`;
}

if (lineNumber) {
lineMessages.push({lineNumber, message});
} else {
generalMessages.push({
message,
});
}
});

return {lineMessages, generalMessages};
}

module.exports = StructuredDataAutomatic;
module.exports.UIStrings = UIStrings;
2 changes: 1 addition & 1 deletion lighthouse-core/audits/seo/manual/structured-data.js
Expand Up @@ -10,7 +10,7 @@ const i18n = require('../../../lib/i18n/i18n.js');

const UIStrings = {
/** Description of a Lighthouse audit that provides detail on the structured data in a page. "Structured data" is a standardized data format on a page that helps a search engine categorize and understand its contents. This description is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
description: 'Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more](https://developers.google.com/search/docs/guides/mark-up-content).',
description: 'Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) to validate structured data. [Learn more](https://developers.google.com/search/docs/guides/mark-up-content).',
/** Title of a Lighthouse audit that prompts users to manually check their page for valid structured data. "Structured data" is a standardized data format on a page that helps a search engine categorize and understand its contents. */
title: 'Structured data is valid',
};
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/config/default-config.js
Expand Up @@ -282,6 +282,7 @@ const defaultConfig = {
'seo/hreflang',
'seo/plugins',
'seo/canonical',
'seo/json-ld',
'seo/manual/structured-data',
],

Expand Down Expand Up @@ -495,6 +496,7 @@ const defaultConfig = {
{id: 'image-alt', weight: 1, group: 'seo-content'},
{id: 'hreflang', weight: 1, group: 'seo-content'},
{id: 'canonical', weight: 1, group: 'seo-content'},
{id: 'json-ld', weight: 1, group: 'seo-content'},
{id: 'font-size', weight: 1, group: 'seo-mobile'},
{id: 'plugins', weight: 1, group: 'seo-content'},
{id: 'tap-targets', weight: 1, group: 'seo-mobile'},
Expand Down
18 changes: 17 additions & 1 deletion lighthouse-core/lib/i18n/en-US.json
Expand Up @@ -891,6 +891,22 @@
"message": "Page isn’t blocked from indexing",
"description": "Title of a Lighthouse audit that provides detail on if search-engine crawlers are blocked from indexing the page. This title is shown when the page is not blocked from indexing and can be crawled."
},
"lighthouse-core/audits/seo/json-ld.js | description": {
"message": "Structured data contains rich metadata about a web page. The data is used in search results and social sharing. Invalid metadata will affect how the page appears in these contexts. This audit currently validates a subset of JSON-LD rules. See also the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) to validate other types of structured data.",
"description": "Description of a Lighthouse audit that tells the user whether JSON-LD snippets on the page are invalid. This is displayed after a user expands the section to see more. No character length limits."
},
"lighthouse-core/audits/seo/json-ld.js | displayValue": {
"message": "{invalidSnippetCount, plural,\n =1 {# invalid snippet}\n other {# invalid snippets}\n }",
"description": "Explanatory message stating how many JSON-LD structured data snippets are invalid"
},
"lighthouse-core/audits/seo/json-ld.js | failureTitle": {
"message": "JSON-LD structured data syntax is invalid",
"description": "Title of a Lighthouse audit that provides detail on whether JSON-LD structured data snippets are valid. This descriptive title is shown when JSON-LD snippets with invalid content were found."
},
"lighthouse-core/audits/seo/json-ld.js | title": {
"message": "JSON-LD structured data syntax is valid",
"description": "Title of a Lighthouse audit that provides detail on whether JSON-LD structured data snippets are valid. This descriptive title is shown when no invalid JSON-LD snippets were found."
},
"lighthouse-core/audits/seo/link-text.js | description": {
"message": "Descriptive link text helps search engines understand your content. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/descriptive-link-text).",
"description": "Description of a Lighthouse audit that tells the user *why* they need to have descriptive text on the links in their page. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
Expand All @@ -908,7 +924,7 @@
"description": "Title of a Lighthouse audit that tests if each link on a page contains a sufficient description of what a user will find when they click it. Generic, non-descriptive text like \"click here\" doesn't give an indication of what the link leads to. This descriptive title is shown when all links on the page have sufficient textual descriptions."
},
"lighthouse-core/audits/seo/manual/structured-data.js | description": {
"message": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more](https://developers.google.com/search/docs/guides/mark-up-content).",
"message": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) to validate structured data. [Learn more](https://developers.google.com/search/docs/guides/mark-up-content).",
"description": "Description of a Lighthouse audit that provides detail on the structured data in a page. \"Structured data\" is a standardized data format on a page that helps a search engine categorize and understand its contents. This description is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
},
"lighthouse-core/audits/seo/manual/structured-data.js | title": {
Expand Down
17 changes: 12 additions & 5 deletions lighthouse-core/lib/sd-validation/schema-validator.js
Expand Up @@ -96,11 +96,18 @@ function validateObjectKeys(typeOrTypes, keys) {
// remove Schema.org input/output constraints http://schema.org/docs/actions.html#part-4
.map(key => key.replace(/-(input|output)$/, ''))
.filter(key => !allKnownProps.has(key))
.map(key => ({
message: `Unexpected property "${key}"`,
key,
validTypes: types,
}));
.map(key => {
return ({
message: `Unexpected property "${key}"`,
key,
validTypes: types.map(typeUri => {
const typeNameMatch = typeUri.match(/[\w]+$/);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const name = typeNameMatch ? typeNameMatch[0] : typeUri;
const uri = typeUri.replace('http://', 'https://');
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
return {name, uri};
}),
});
});
}

/**
Expand Down