diff --git a/packages/optimizer/lib/AmpConstants.js b/packages/optimizer/lib/AmpConstants.js index 90cfdb65d..e5b0d6d93 100644 --- a/packages/optimizer/lib/AmpConstants.js +++ b/packages/optimizer/lib/AmpConstants.js @@ -19,5 +19,6 @@ module.exports = { AMP_TAGS: ['amp', '⚡', '⚡4ads', 'amp4ads', '⚡4email', 'amp4email'], AMP_CACHE_HOST: 'https://cdn.ampproject.org', AMP_FORMATS: ['AMP', 'AMP4EMAIL', 'AMP4ADS'], + AMP_RUNTIME_CSS_PATH: '/v0.css', appendRuntimeVersion: (prefix, version) => prefix + '/rtv/' + version, }; diff --git a/packages/optimizer/lib/DomTransformer.js b/packages/optimizer/lib/DomTransformer.js index 43123b1ad..eeca3af07 100644 --- a/packages/optimizer/lib/DomTransformer.js +++ b/packages/optimizer/lib/DomTransformer.js @@ -18,8 +18,8 @@ const treeParser = require('./TreeParser'); const log = require('./log'); const {oneBehindFetch} = require('@ampproject/toolbox-core'); -const validatorRules = require('@ampproject/toolbox-validator-rules'); const RuntimeVersion = require('@ampproject/toolbox-runtime-version/lib/RuntimeVersion'); +const fetchRuntimeParameters = require('./fetchRuntimeParameters'); /** * AMP Optimizer Configuration only applying AMP validity perserving transformations. @@ -125,15 +125,14 @@ class DomTransformer { /** * Transforms a DOM tree. * @param {Tree} tree - a DOM tree. - * @param {Object} params - a dictionary containing transformer specific parameters. + * @param {Object} customParams - a dictionary containing transformer specific parameters. */ - async transformTree(tree, params) { - params = params || {}; - params.validatorRules = params.validatorRules || (await validatorRules.fetch()); - log.verbose(params.verbose || false); + async transformTree(tree, customParams = {}) { + const runtimeParameters = await fetchRuntimeParameters(this.config, customParams); + log.verbose(runtimeParameters.verbose); const sequence = async (promise, transformer) => { await promise; - return transformer.transform(tree, params); + return transformer.transform(tree, runtimeParameters); }; return this.transformers_.reduce(sequence, Promise.resolve()); } @@ -145,13 +144,13 @@ class DomTransformer { * @param {Array.} config.transformations - a list of transformers to be applied. */ setConfig(config) { - config = Object.assign({}, DEFAULT_CONFIG, config); - if (!config.runtimeVersion) { + this.config = Object.assign({}, DEFAULT_CONFIG, config); + if (!this.config.runtimeVersion) { // Re-use custom fetch implementation for runtime version provider - config.runtimeVersion = new RuntimeVersion(config.fetch); + this.config.runtimeVersion = new RuntimeVersion(this.config.fetch); } - log.verbose(config.verbose); - this.initTransformers_(config); + log.verbose(this.config.verbose); + this.initTransformers_(this.config); } /** diff --git a/packages/optimizer/lib/RuntimeHostHelper.js b/packages/optimizer/lib/RuntimeHostHelper.js index 62c4318b6..2cb62a14e 100644 --- a/packages/optimizer/lib/RuntimeHostHelper.js +++ b/packages/optimizer/lib/RuntimeHostHelper.js @@ -17,13 +17,13 @@ const {AMP_CACHE_HOST, appendRuntimeVersion} = require('./AmpConstants.js'); -function calculateHost({ampUrlPrefix = AMP_CACHE_HOST, ampRuntimeVersion, lts}) { +function calculateHost({ampUrlPrefix = AMP_CACHE_HOST, ampRuntimeVersion, lts, rtv = false}) { if (lts && ampRuntimeVersion) { throw new Error('lts flag is not compatible with runtime version parameter'); } ampUrlPrefix = ampUrlPrefix.replace(/\/$/, ''); - if (ampRuntimeVersion) { + if (ampRuntimeVersion && rtv) { ampUrlPrefix = appendRuntimeVersion(ampUrlPrefix, ampRuntimeVersion); } else if (lts) { ampUrlPrefix += '/lts'; diff --git a/packages/optimizer/lib/fetchRuntimeParameters.js b/packages/optimizer/lib/fetchRuntimeParameters.js new file mode 100644 index 000000000..24fae63a1 --- /dev/null +++ b/packages/optimizer/lib/fetchRuntimeParameters.js @@ -0,0 +1,91 @@ +/** + * Copyright 2020 The AMP HTML Authors. 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. + */ +'mode strict'; + +const validatorRules = require('@ampproject/toolbox-validator-rules'); +const {AMP_CACHE_HOST, AMP_RUNTIME_CSS_PATH, appendRuntimeVersion} = require('./AmpConstants.js'); + +/** + * Creates the runtime parameters used by the transformers. Runtime parameters might + * change at runtime with new AMP releases. + * + * @param {Object} config - the AMP Optimizer config + * @param {Object} customRuntimeParameters - user defined runtime parameters + */ +async function fetchRuntimeParameters(config, customRuntimeParameters) { + const runtimeParameters = Object.assign({}, customRuntimeParameters); + // Configure the log level + runtimeParameters.verbose = customRuntimeParameters.verbose || config.verbose || false; + // Copy lts and rtv runtime flag from custom parameters or the static config. Both are disabled by default. + runtimeParameters.lts = customRuntimeParameters.lts || config.lts || false; + runtimeParameters.rtv = customRuntimeParameters.rtv || config.rtv || false; + // Fetch the validator rules + runtimeParameters.validatorRules = config.validatorRules || (await validatorRules.fetch()); + let {ampUrlPrefix, ampRuntimeVersion, ampRuntimeStyles, lts} = runtimeParameters; + // Use existing runtime version or fetch lts or latest + runtimeParameters.ampRuntimeVersion = + ampRuntimeVersion || (await config.runtimeVersion.currentVersion({ampUrlPrefix, lts})); + // Fetch runtime styles based on the runtime version + runtimeParameters.ampRuntimeStyles = + ampRuntimeStyles || + (await fetchAmpRuntimeStyles_(config, ampUrlPrefix, runtimeParameters.ampRuntimeVersion)); + return runtimeParameters; +} + +/** + * @private + */ +async function fetchAmpRuntimeStyles_(config, ampUrlPrefix, ampRuntimeVersion) { + if (ampUrlPrefix && !_isAbsoluteUrl(ampUrlPrefix)) { + config.log.warn( + `AMP runtime styles cannot be fetched from relative ampUrlPrefix, please use the 'ampRuntimeStyles' parameter to provide the correct runtime style.` + ); + // Gracefully fallback to latest runtime version + ampUrlPrefix = AMP_CACHE_HOST; + ampRuntimeVersion = ampRuntimeVersion || (await config.runtimeVersion.currentVersion()); + } + // Construct the AMP runtime CSS download URL, the default is: https://cdn.ampproject.org/rtv/${ampRuntimeVersion}/v0.css + const runtimeCssUrl = + appendRuntimeVersion(ampUrlPrefix || AMP_CACHE_HOST, ampRuntimeVersion) + AMP_RUNTIME_CSS_PATH; + // Fetch runtime styles + const response = await config.fetch(runtimeCssUrl); + if (!response.ok) { + config.log.error( + `Could not fetch ${runtimeCssUrl}, failed with status ` + `${response.status}.` + ); + if (ampUrlPrefix || ampRuntimeVersion) { + // Try to download latest from cdn.ampproject.org instead + return fetchAmpRuntimeStyles_(AMP_CACHE_HOST, await config.runtimeVersion.currentVersion()); + } else { + return ''; + } + } + return response.text(); +} + +/** + * @private + */ +function _isAbsoluteUrl(url) { + try { + new URL(url); + return true; + } catch (ex) {} + + return false; +} + +module.exports = fetchRuntimeParameters; diff --git a/packages/optimizer/lib/transformers/AmpBoilerplateTransformer.js b/packages/optimizer/lib/transformers/AmpBoilerplateTransformer.js index 7413146c5..b61c5399e 100644 --- a/packages/optimizer/lib/transformers/AmpBoilerplateTransformer.js +++ b/packages/optimizer/lib/transformers/AmpBoilerplateTransformer.js @@ -22,9 +22,7 @@ const { hasAttribute, firstChildByTag, } = require('../NodeUtils'); -const {AMP_CACHE_HOST, appendRuntimeVersion} = require('../AmpConstants.js'); - -const V0_CSS_PATH = '/v0.css'; +const {AMP_CACHE_HOST, V0_CSS_PATH, appendRuntimeVersion} = require('../AmpConstants.js'); /** * AmpBoilerplateTransformer - This DOM transformer adds @@ -47,11 +45,21 @@ class AmpBoilerplateTransformer { return; // invalid doc } // amp-runtime is added by server-side-rendering - const ampRuntimeStyle = this._findAmpRuntimeStyle(head); - if (!ampRuntimeStyle) { + const ampRuntimeStylesNode = this._findAmpRuntimeStyle(head); + if (!ampRuntimeStylesNode) { return; // keep existing boilerplate } - return this._addStaticCss(ampRuntimeStyle, params); + // inline CSS + let {ampRuntimeVersion, ampRuntimeStyles} = params; + if (!ampRuntimeVersion || !ampRuntimeStyles) { + // these should be set by DomTransformer + this.log_.error( + 'Missing parameters both ampRuntimeVersion and ampRuntimeStyles need to be present' + ); + return; + } + ampRuntimeStylesNode.attribs['i-amphtml-version'] = ampRuntimeVersion; + insertText(ampRuntimeStylesNode, ampRuntimeStyles); } _findAmpRuntimeStyle(head) { @@ -64,62 +72,6 @@ class AmpBoilerplateTransformer { } return null; } - - async _addStaticCss(node, params) { - // we can always inline v0.css as the AMP runtime will take care of keeping v0.css in sync - try { - return this._inlineCss(node, params); - } catch (error) { - this.log_.error(error); - this._linkCss(node); - } - } - - _linkCss(node) { - const cssStyleNode = createElement('link', { - rel: 'stylesheet', - href: AMP_CACHE_HOST + V0_CSS_PATH, - }); - insertBefore(node.parent, cssStyleNode, node); - } - - async _inlineCss(node, params) { - let {ampRuntimeVersion, ampUrlPrefix, lts} = params; - - // TODO: If ampUrlPrefix is a relative URL, this will fall back to - // fetching the latest runtime version and boilerplate CSS from - // cdn.ampproject.org. Is this our best option? - if (ampUrlPrefix && !this._isAbsoluteUrl(ampUrlPrefix)) { - ampUrlPrefix = undefined; - ampRuntimeVersion = undefined; - } - - const version = - ampRuntimeVersion || (await this.runtimeVersion_.currentVersion({ampUrlPrefix, lts})); - const v0CssUrl = appendRuntimeVersion(ampUrlPrefix || AMP_CACHE_HOST, version) + V0_CSS_PATH; - - node.attribs['i-amphtml-version'] = version; - - // Fetch and inline contents of v0.css - const response = await this.fetch_(v0CssUrl); - if (!response.ok) { - throw new Error( - `Could not inline v0.css. Request to ${v0CssUrl} failed with status ` + - `${response.status}.` - ); - } - const body = await response.text(); - insertText(node, body); - } - - _isAbsoluteUrl(url) { - try { - new URL(url); - return true; - } catch (ex) {} - - return false; - } } module.exports = AmpBoilerplateTransformer; diff --git a/packages/optimizer/spec/end-to-end/hello-world/input.html b/packages/optimizer/spec/end-to-end/hello-world/input.html index 5d3c471c9..38a6fd00a 100644 --- a/packages/optimizer/spec/end-to-end/hello-world/input.html +++ b/packages/optimizer/spec/end-to-end/hello-world/input.html @@ -3,6 +3,7 @@ "ampUrlPrefix": "/amp", "ampRuntimeVersion": "123456789000000", "ampUrl": "https://example.com/amp-version.html", + "rtv": true, "geoApiUrl": "/geo" } --> diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/expected_output.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/expected_output.html deleted file mode 100644 index 45369351e..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/expected_output.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/input.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/input.html deleted file mode 100644 index 0caf7cc81..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_custom_host/input.html +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/expected_output.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/expected_output.html deleted file mode 100644 index 290c488c5..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/expected_output.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/input.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/input.html deleted file mode 100644 index 305153a83..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/supports_lts/input.html +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/expected_output.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/expected_output.html deleted file mode 100644 index a0aef2ac2..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/expected_output.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - - \ No newline at end of file diff --git a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/input.html b/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/input.html deleted file mode 100644 index 27b0f61a9..000000000 --- a/packages/optimizer/spec/transformers/experimental/AmpBoilerplateTransformer/uses_given_runtime_version/input.html +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - diff --git a/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/adds_runtime_version/input.html b/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/adds_runtime_version/input.html index c7a43086a..287a0bf18 100644 --- a/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/adds_runtime_version/input.html +++ b/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/adds_runtime_version/input.html @@ -1,5 +1,6 @@ diff --git a/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/rewrites_host_and_adds_version/input.html b/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/rewrites_host_and_adds_version/input.html index c1ac9a6b7..bbdc56c84 100644 --- a/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/rewrites_host_and_adds_version/input.html +++ b/packages/optimizer/spec/transformers/experimental/RewriteAmpUrls/rewrites_host_and_adds_version/input.html @@ -1,5 +1,6 @@ diff --git a/packages/optimizer/spec/transformers/valid/AmpBoilerplateTransformer/does_not_add_v0.css_if_style_amp-runtime_not_present/input.html b/packages/optimizer/spec/transformers/valid/AmpBoilerplateTransformer/does_not_add_v0.css_if_style_amp-runtime_not_present/input.html index b0f3319e9..58710d1fd 100644 --- a/packages/optimizer/spec/transformers/valid/AmpBoilerplateTransformer/does_not_add_v0.css_if_style_amp-runtime_not_present/input.html +++ b/packages/optimizer/spec/transformers/valid/AmpBoilerplateTransformer/does_not_add_v0.css_if_style_amp-runtime_not_present/input.html @@ -1,3 +1,9 @@ + diff --git a/packages/runtime-version/lib/RuntimeVersion.js b/packages/runtime-version/lib/RuntimeVersion.js index d670b75c9..9b4fdf824 100644 --- a/packages/runtime-version/lib/RuntimeVersion.js +++ b/packages/runtime-version/lib/RuntimeVersion.js @@ -77,6 +77,7 @@ class RuntimeVersion { * * @param {object} options - the options. * @param {boolean} options.canary - true if canary should be returned. + * @param {boolean} options.lts - true if lts should be returned. * @param {string} options.ampUrlPrefix - the domain & path to an AMP runtime. * @returns {Promise} a promise containing the current version. */