From d8608c8562f6369383c74eb912ac4c16971c9527 Mon Sep 17 00:00:00 2001 From: Thorsten Harders Date: Wed, 19 Aug 2020 16:39:22 +0200 Subject: [PATCH 1/3] Preload checks for blocking extensions Preload check for runtime allows hosted version and javascript module version --- packages/linter/src/index.ts | 6 ++- .../src/rules/BlockingExtensionsPreloaded.ts | 27 ++++++++++++ .../linter/src/rules/RuntimeIsPreloaded.ts | 11 +++-- packages/linter/tests/local.test.ts | 41 +++++++++++++++++++ .../BlockingExtensionsPreloaded-1/source.html | 35 ++++++++++++++++ .../BlockingExtensionsPreloaded-2/source.html | 28 +++++++++++++ .../BlockingExtensionsPreloaded-3/source.html | 32 +++++++++++++++ .../local/RuntimeIsPreloaded-3/source.html | 29 +++++++++++++ 8 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 packages/linter/src/rules/BlockingExtensionsPreloaded.ts create mode 100644 packages/linter/tests/local/BlockingExtensionsPreloaded-1/source.html create mode 100644 packages/linter/tests/local/BlockingExtensionsPreloaded-2/source.html create mode 100644 packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html create mode 100644 packages/linter/tests/local/RuntimeIsPreloaded-3/source.html diff --git a/packages/linter/src/index.ts b/packages/linter/src/index.ts index c2aac1c1f..4099ac454 100644 --- a/packages/linter/src/index.ts +++ b/packages/linter/src/index.ts @@ -22,10 +22,11 @@ import { VideosHaveAltText } from "./rules/VideosHaveAltText"; import { VideosAreSubtitled } from "./rules/VideosAreSubtitled"; import { IsValid } from "./rules/IsValid"; import { TitleMeetsLengthCriteria } from "./rules/TitleMeetsLengthCriteria"; -import { RuleConstructor } from "./rule"; -import { isArray } from "util"; import { IsTransformedAmp } from "./rules/IsTransformedAmp"; import { ModuleRuntimeUsed } from "./rules/ModuleRuntimeUsed"; +import { BlockingExtensionsPreloaded } from "./rules/BlockingExtensionsPreloaded"; +import { RuleConstructor } from "./rule"; +import { isArray } from "util"; export enum LintMode { Amp = "amp", @@ -128,6 +129,7 @@ function testsForMode(type: LintMode) { (tests.get(LintMode.PageExperience) || []).concat([ IsValid, RuntimeIsPreloaded, + BlockingExtensionsPreloaded, IsTransformedAmp, ModuleRuntimeUsed, ]) diff --git a/packages/linter/src/rules/BlockingExtensionsPreloaded.ts b/packages/linter/src/rules/BlockingExtensionsPreloaded.ts new file mode 100644 index 000000000..d561f9398 --- /dev/null +++ b/packages/linter/src/rules/BlockingExtensionsPreloaded.ts @@ -0,0 +1,27 @@ +import { Context } from "../index"; +import { Rule } from "../rule"; + +const blockingExtension = ["amp-experiment", "amp-dynamic-css-classes"]; + +export class BlockingExtensionsPreloaded extends Rule { + run({ $ }: Context) { + const results = []; + blockingExtension.forEach((extension) => { + const scriptPart = `/v0/${extension}-`; + if ($(`script[src*='${scriptPart}']`).length > 0) { + if ($(`link[rel$='preload'][href*='${scriptPart}']`).length == 0) { + results.push(this.warn(`Preload for ${extension} is missing`)); + } + } + }); + return Promise.all(results); + } + meta() { + return { + url: + "https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/optimize_amp/#optimize-the-amp-runtime-loading", + title: "Render-blocking extensions are preloaded", + info: "", + }; + } +} diff --git a/packages/linter/src/rules/RuntimeIsPreloaded.ts b/packages/linter/src/rules/RuntimeIsPreloaded.ts index 6efbcded2..cbdf2cc13 100644 --- a/packages/linter/src/rules/RuntimeIsPreloaded.ts +++ b/packages/linter/src/rules/RuntimeIsPreloaded.ts @@ -3,14 +3,13 @@ import { Rule } from "../rule"; export class RuntimeIsPreloaded extends Rule { run({ $ }: Context) { - const attr = [ - "href='https://cdn.ampproject.org/v0.js'", - "rel='preload'", - "as='script'", - ] + const jsAttr = ["href$='/v0.js'", "rel='preload'", "as='script'"] .map((s) => `[${s}]`) .join(""); - const isPreloaded = $(`link${attr}`).length > 0; + const mjsAttr = ["href$='/v0.mjs'", "rel='modulepreload'"] + .map((s) => `[${s}]`) + .join(""); + const isPreloaded = $(`link${jsAttr}, link${mjsAttr}`).length > 0; return isPreloaded ? this.pass() : this.warn( diff --git a/packages/linter/tests/local.test.ts b/packages/linter/tests/local.test.ts index 875e0f2b5..637b5b09e 100644 --- a/packages/linter/tests/local.test.ts +++ b/packages/linter/tests/local.test.ts @@ -12,6 +12,7 @@ import { BookendExists } from "../src/rules/BookendExists"; import { TitleMeetsLengthCriteria } from "../src/rules/TitleMeetsLengthCriteria"; import { IsTransformedAmp } from "../src/rules/IsTransformedAmp"; import { ModuleRuntimeUsed } from "../src/rules/ModuleRuntimeUsed"; +import { BlockingExtensionsPreloaded } from '../src/rules/BlockingExtensionsPreloaded'; describe(AmpImgAmpPixelPreferred.name, () => { it(`${AmpImgAmpPixelPreferred.name} - `, async () => { @@ -60,6 +61,37 @@ describe(AmpImgAmpPixelPreferred.name, () => { }); }); + +describe(BlockingExtensionsPreloaded.name, () => { + it(`${BlockingExtensionsPreloaded.name} - preload for js and mjs present`, async () => { + return assertPass( + runLocalTest( + BlockingExtensionsPreloaded, + `${__dirname}/local/BlockingExtensionsPreloaded-1/source.html` + ) + ); + }); + + it(`${BlockingExtensionsPreloaded.name} - No blocking extensions present`, async () => { + return assertPass( + runLocalTest( + BlockingExtensionsPreloaded, + `${__dirname}/local/BlockingExtensionsPreloaded-2/source.html` + ) + ); + }); + + it(`${BlockingExtensionsPreloaded.name} - preload for js and mjs is missing`, async () => { + const results = await runLocalTest( + BlockingExtensionsPreloaded, + `${__dirname}/local/BlockingExtensionsPreloaded-3/source.html` + ); + expect(results).toHaveLength(2); + await assertWarn(results[0]); + await assertWarn(results[1]); + }); +}); + describe(IsTransformedAmp.name, () => { it(`${IsTransformedAmp.name} - Transformed AMP detected`, async () => { return assertPass( @@ -138,6 +170,15 @@ describe(RuntimeIsPreloaded.name, () => { ) ); }); + + it(`${RuntimeIsPreloaded.name} - is present`, async () => { + return assertPass( + runLocalTest( + RuntimeIsPreloaded, + `${__dirname}/local/RuntimeIsPreloaded-3/source.html` + ) + ); + }); }); describe(SchemaMetadataIsNews.name, () => { diff --git a/packages/linter/tests/local/BlockingExtensionsPreloaded-1/source.html b/packages/linter/tests/local/BlockingExtensionsPreloaded-1/source.html new file mode 100644 index 000000000..a5e563d30 --- /dev/null +++ b/packages/linter/tests/local/BlockingExtensionsPreloaded-1/source.html @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + Hello, AMPs + + + +

Hello, AMP!

+ + diff --git a/packages/linter/tests/local/BlockingExtensionsPreloaded-2/source.html b/packages/linter/tests/local/BlockingExtensionsPreloaded-2/source.html new file mode 100644 index 000000000..716f17e5f --- /dev/null +++ b/packages/linter/tests/local/BlockingExtensionsPreloaded-2/source.html @@ -0,0 +1,28 @@ + + + + + + + Hello, AMPs + + + + + + + +

Hello, AMP!

+ + \ No newline at end of file diff --git a/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html b/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html new file mode 100644 index 000000000..c906d19d6 --- /dev/null +++ b/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + Hello, AMPs + + + +

Hello, AMP!

+ + diff --git a/packages/linter/tests/local/RuntimeIsPreloaded-3/source.html b/packages/linter/tests/local/RuntimeIsPreloaded-3/source.html new file mode 100644 index 000000000..13aa48b9c --- /dev/null +++ b/packages/linter/tests/local/RuntimeIsPreloaded-3/source.html @@ -0,0 +1,29 @@ + + + + + + + + Hello, AMPs + + + + + + + +

Hello, AMP!

+ + From 2990b9ed863696e0573f033e1a740b338cc2a813 Mon Sep 17 00:00:00 2001 From: Thorsten Harders Date: Wed, 19 Aug 2020 17:55:31 +0200 Subject: [PATCH 2/3] Add amp-story to BlockingExtensionsPreloaded check --- packages/linter/src/rules/BlockingExtensionsPreloaded.ts | 6 +++++- packages/linter/tests/local.test.ts | 3 ++- .../tests/local/BlockingExtensionsPreloaded-3/source.html | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/linter/src/rules/BlockingExtensionsPreloaded.ts b/packages/linter/src/rules/BlockingExtensionsPreloaded.ts index d561f9398..7098d4a3a 100644 --- a/packages/linter/src/rules/BlockingExtensionsPreloaded.ts +++ b/packages/linter/src/rules/BlockingExtensionsPreloaded.ts @@ -1,7 +1,11 @@ import { Context } from "../index"; import { Rule } from "../rule"; -const blockingExtension = ["amp-experiment", "amp-dynamic-css-classes"]; +const blockingExtension = [ + "amp-dynamic-css-classes", + "amp-experiment", + "amp-story", +]; export class BlockingExtensionsPreloaded extends Rule { run({ $ }: Context) { diff --git a/packages/linter/tests/local.test.ts b/packages/linter/tests/local.test.ts index 637b5b09e..9baecf9b4 100644 --- a/packages/linter/tests/local.test.ts +++ b/packages/linter/tests/local.test.ts @@ -86,9 +86,10 @@ describe(BlockingExtensionsPreloaded.name, () => { BlockingExtensionsPreloaded, `${__dirname}/local/BlockingExtensionsPreloaded-3/source.html` ); - expect(results).toHaveLength(2); + expect(results).toHaveLength(3); await assertWarn(results[0]); await assertWarn(results[1]); + await assertWarn(results[2]); }); }); diff --git a/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html b/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html index c906d19d6..4e10023fd 100644 --- a/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html +++ b/packages/linter/tests/local/BlockingExtensionsPreloaded-3/source.html @@ -11,6 +11,8 @@ + +