Skip to content

Commit

Permalink
Revert "optimizer: Make dynamic component URL rewrites optional (ampp…
Browse files Browse the repository at this point in the history
…roject#518)"

This reverts commit 81e525d.

Do not allow "partial self-hosting". All AMP components should come from
the same origin to ensure version locking support is maintained. An
alternative solution for amp-geo is to fall back to an API request
instead of requiring cache modification when amp-geo-0.1.js is served.
  • Loading branch information
mdmower committed Jan 20, 2020
1 parent 4d475ff commit 5a8bf3d
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 123 deletions.
21 changes: 3 additions & 18 deletions packages/optimizer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,6 @@ It's possible to rewrite the AMP framework and component imports to a different

**Notice:** The behavior of `ampUrlPrefix` when used in conjunction with `ampRuntimeVersion` changed beginning with version 1.1.2. Prior to 1.1.2, `rtv/{rtv}/` was automatically appended to `ampUrlPrefix` when `ampRuntimeVersion` was specified. Since version 1.1.2, `ampUrlPrefix` is not modified when `ampRuntimeVersion` is also specified.

- `rewriteDynamicComponents`: When used in conjunction with `ampUrlPrefix`, this option can be set to `false` to prevent [dynamic AMP components](https://github.com/ampproject/amphtml/blob/master/spec/amp-cache-guidelines.md#guidelines-adding-a-new-cache-to-the-amp-ecosystem) from having their URLs rewritten.

Examples:
```
const ampOptimizer = require('@ampproject/toolbox-optimizer');
Expand All @@ -206,27 +204,14 @@ const originalHtml = `
...
`
// this will rewrite https://cdn.ampproject.org/v0.js to /amp/v0.js and will
// rewrite dynamic component https://cdn.ampproject.org/v0/amp-geo-0.1.js to
// /amp/v0/amp-geo-0.1.js
// this will rewrite https://cdn.ampproject.org/v0.js to /amp/v0.js
const optimizedHtmlA = await ampOptimizer.transformHtml(originalHtml, {
ampUrlPrefix: '/amp'
});
// this will rewrite https://cdn.ampproject.org/v0.js to /amp/v0.js and will
// not rewrite dynamic component https://cdn.ampproject.org/v0/amp-geo-0.1.js
// this will rewrite https://cdn.ampproject.org/v0.js to /amp/v0.js
const optimizedHtmlB = await ampOptimizer.transformHtml(originalHtml, {
ampUrlPrefix: '/amp',
rewriteDynamicComponents: false
});
// this will rewrite https://cdn.ampproject.org/v0.js to
// /amp/001515617716922/v0.js and will rewrite dynamic component
// https://cdn.ampproject.org/v0/amp-geo-0.1.js to
// https://cdn.ampproject.org/v0/rtv/001515617716922/amp-geo-0.1.js
const optimizedHtmlC = await ampOptimizer.transformHtml(originalHtml, {
ampRuntimeVersion: '001515617716922',
ampUrlPrefix: '/amp/001515617716922',
rewriteDynamicComponents: false
ampUrlPrefix: '/amp'
});
```
6 changes: 0 additions & 6 deletions packages/optimizer/lib/AmpConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,5 @@
module.exports = {
AMP_TAGS: ['amp', '⚡', '⚡4ads'],
AMP_CACHE_HOST: 'https://cdn.ampproject.org',
// Should be kept up to date with dynamic components listed here:
// https://github.com/ampproject/amphtml/blob/master/spec/amp-cache-guidelines.md#guidelines-adding-a-new-cache-to-the-amp-ecosystem
AMP_DYNAMIC_COMPONENTS: {
'custom-element': ['amp-geo'],
'custom-template': [],
},
appendRuntimeVersion: (prefix, version) => prefix + '/rtv/' + version,
};
40 changes: 6 additions & 34 deletions packages/optimizer/lib/transformers/RewriteAmpUrls.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,13 @@
*/
'use strict';

const {
AMP_CACHE_HOST,
AMP_DYNAMIC_COMPONENTS,
appendRuntimeVersion,
} = require('../AmpConstants.js');
const {AMP_CACHE_HOST, appendRuntimeVersion} = require('../AmpConstants.js');
const {findMetaViewport} = require('../HtmlDomHelper');

/**
* RewriteAmpUrls - rewrites AMP runtime URLs.
*
* This transformer supports several parameters:
* This transformer supports two parameters:
*
* * `ampRuntimeVersion`: specifies a
* [specific version](https://github.com/ampproject/amp-toolbox/tree/master/runtime-version")
Expand All @@ -39,14 +35,9 @@ const {findMetaViewport} = require('../HtmlDomHelper');
* URLs being re-written from `https://cdn.ampproject.org/v0.js` to
* `/amp/v0.js`. This option is experimental and not recommended.
*
* * `rewriteDynamicComponents`: optionally disable rewriting of
* [dynamically generated components](https://github.com/ampproject/amphtml/blob/master/spec/amp-cache-guidelines.md#guidelines-adding-a-new-cache-to-the-amp-ecosystem).
* For example: `https://cdn.ampproject.org/v0/amp-geo-0.1.js` returns
* different content depending on the country from which the request was made.
*
* All parameters are optional. If no option is provided, runtime URLs won't be
* re-written. You can combine `ampRuntimeVersion` and `ampUrlPrefix` to rewrite
* AMP runtime URLs to versioned URLs on a different origin.
* Both parameters are optional. If no option is provided, runtime URLs won't be
* re-written. You can combine both parameters to rewrite AMP runtime URLs
* to versioned URLs on a different origin.
*
* This transformer also adds a preload header for the AMP runtime (v0.js) to trigger HTTP/2
* push for CDNs (see https://www.w3.org/TR/preload/#server-push-(http/2)).
Expand All @@ -61,22 +52,13 @@ class RewriteAmpUrls {
if (params.ampRuntimeVersion && !params.ampUrlPrefix) {
ampUrlPrefix = appendRuntimeVersion(ampUrlPrefix, params.ampRuntimeVersion);
}
let dynamicAmpUrlPrefix = ampUrlPrefix;
if (params.rewriteDynamicComponents === false) {
dynamicAmpUrlPrefix = AMP_CACHE_HOST;
if (params.ampRuntimeVersion) {
dynamicAmpUrlPrefix = appendRuntimeVersion(dynamicAmpUrlPrefix, params.ampRuntimeVersion);
}
}

let node = head.firstChild;
let referenceNode = findMetaViewport(head);

while (node) {
if (node.tagName === 'script' && this._usesAmpCacheUrl(node.attribs.src)) {
const isDynamicComponent = this._isDynamicComponent(node);
node.attribs.src = this._replaceUrl(node.attribs.src,
isDynamicComponent ? dynamicAmpUrlPrefix : ampUrlPrefix);
node.attribs.src = this._replaceUrl(node.attribs.src, ampUrlPrefix);
referenceNode = this._addPreload(tree, head, referenceNode, node.attribs.src, 'script');
} else if (node.tagName === 'link' &&
node.attribs.rel === 'stylesheet' &&
Expand Down Expand Up @@ -112,16 +94,6 @@ class RewriteAmpUrls {
parent.insertAfter(preload, node);
return preload;
}

_isDynamicComponent(script) {
if (!script || !script.attribs || script.tagName !== 'script') {
return false;
}

return Object.keys(AMP_DYNAMIC_COMPONENTS).some((type) => {
return script.attribs[type] && AMP_DYNAMIC_COMPONENTS[type].includes(script.attribs[type]);
});
}
}

module.exports = RewriteAmpUrls;

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 5a8bf3d

Please sign in to comment.