Skip to content

Commit

Permalink
Support script based templates
Browse files Browse the repository at this point in the history
Make sure to always also support the script based template syntax:

```
<script type="text/plain" template="amp-mustache">
  Hello {{world}}!
</script>
```

Using the template helper should also make SSR more performant as it
avoids a full parent traversal for each node.
  • Loading branch information
sebastianbenz committed Jan 15, 2021
1 parent 5dbdc56 commit d570011
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 31 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ jobs:
with:
path: |
node_modules
*/*/node_modules
key: ${{ runner.os }}-${{ hashFiles('**/package-lock.json') }}
- name: clean
run: npx lerna clean --yes
- name: install
run: npx lerna bootstrap --hoist
- name: build
Expand Down
2 changes: 0 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 15 additions & 13 deletions packages/linter/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const UA = {

export function cli(argv: string[], logger = console, cmd = "amplint") {
program
.storeOptionsAsProperties()
// .version(version)
.usage(`${cmd} [options] URL|copy_as_cURL`)
.option(
Expand Down Expand Up @@ -101,26 +102,27 @@ export function cli(argv: string[], logger = console, cmd = "amplint") {

program.parse(options);

// Add your custom types to commander
const customProgram = (program as unknown) as {
userAgent: string;
format: string;
force: LintMode | "auto";
url: string;
headers: { [k: string]: string };
showPassing: boolean;
reportMode: boolean;
};

const url = program.args[0];
if (!url) {
program.help();
} else {
program.url = url;
customProgram.url = url;
}

program.headers = headers;
customProgram.headers = headers;

return easyLint(
(program as unknown) as {
userAgent: string;
format: string;
force: LintMode | "auto";
url: string;
headers: { [k: string]: string };
showPassing: boolean;
reportMode: boolean;
}
)
return easyLint(customProgram)
.then(logger.info.bind(logger))
.catch((e) => {
logger.error(e.stack || e.message || e);
Expand Down
3 changes: 3 additions & 0 deletions packages/optimizer/lib/AmpConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ module.exports = {
AMP_RUNTIME_CSS_PATH: '/v0.css',
appendRuntimeVersion: (prefix, version) => prefix + '/rtv/' + version,
isTemplate: (node) => {
if (!node) {
return false;
}
if (node.tagName === 'template') {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const PIXEL_TARGET = 60;
const MAX_BLURRED_PLACEHOLDERS = 100;
const DEFAULT_CACHED_PLACEHOLDERS = 30;
const CACHE_ALL_PLACEHOLDERS = -1;
const {isTemplate} = require('../AmpConstants');

const ESCAPE_TABLE = {
'#': '%23',
Expand Down Expand Up @@ -126,7 +127,7 @@ class AddBlurryImagePlaceholders {
for (let node = body; node !== null; node = nextNode(node)) {
const {tagName} = node;
let src;
if (tagName === 'template') {
if (isTemplate(node)) {
node = skipNodeAndChildren(node);
continue;
}
Expand Down
10 changes: 9 additions & 1 deletion packages/optimizer/lib/transformers/AutoExtensionImporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
*/
'use strict';

const {nextNode, insertAfter, createElement, firstChildByTag} = require('../NodeUtils');
const {
nextNode,
insertAfter,
createElement,
firstChildByTag,
hasAttribute,
} = require('../NodeUtils');
const {findMetaViewport} = require('../HtmlDomHelper');
const {AMP_FORMATS, AMP_CACHE_HOST} = require('../AmpConstants');

Expand Down Expand Up @@ -274,6 +280,8 @@ class AutoExtensionImporter {
// Add custom templates (e.g. amp-mustache)
if (node.tagName === 'template' && node.attribs.type) {
allRequiredExtensions.add(node.attribs.type);
} else if (node.tagName === 'script' && hasAttribute(node, 'template')) {
allRequiredExtensions.add(node.attribs.template);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/optimizer/lib/transformers/OptimizeImages.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
const {hasAttribute, nextNode, firstChildByTag} = require('../NodeUtils');
const {skipNodeAndChildren} = require('../HtmlDomHelper');
const {isValidImageSrcURL} = require('../URLUtils');
const {isTemplate} = require('../AmpConstants');

// Don't generate srcset's for images with width smaller than MIN_WIDTH_TO_ADD_SRCSET_IN_RESPONSIVE_LAYOUT
// this avoids generating srcsets for images with a responsive layout where width/height define the aspect ration.
Expand Down Expand Up @@ -161,7 +162,7 @@ class OptimizeImages {
let node = body;
const imageOptimizationPromises = [];
while (node !== null) {
if (node.tagName === 'template') {
if (isTemplate(node)) {
node = skipNodeAndChildren(node);
} else {
if (node.tagName === 'amp-img') {
Expand Down
8 changes: 4 additions & 4 deletions packages/optimizer/lib/transformers/PreloadHeroImage.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const {
} = require('../NodeUtils');
const {findMetaViewport, skipNodeAndChildren} = require('../HtmlDomHelper');
const {isValidImageSrcURL} = require('../URLUtils');
const amphtml = require('../AmpConstants');
const {isTemplate, isAmpStory} = require('../AmpConstants');

// Images smaller than 150px are considered tiny
const TINY_IMG_THRESHOLD = 150;
Expand Down Expand Up @@ -72,13 +72,13 @@ class PreloadHeroImage {
);
heroImageCount = DATA_HERO_MAX;
}
const isAmpStory = amphtml.isAmpStory(head);
const isStory = isAmpStory(head);
for (let i = 0; i < heroImageCount; i++) {
const heroImage = heroImages[i];
this.generatePreload(heroImage, head, referenceNode);
// AMP Stories don't support ssr'd amp-img yet
// See https://github.com/ampproject/amphtml/issues/29850
if (!isAmpStory) {
if (!isStory) {
this.generateImg(heroImage.ampImg);
}
}
Expand Down Expand Up @@ -137,7 +137,7 @@ class PreloadHeroImage {
if (!heroImageCandidate && heroImages.length === 0) {
heroImageCandidate = this.isCandidateHeroImage(node);
}
if (amphtml.isTemplate(node)) {
if (isTemplate(node)) {
// Ignore images inside templates
node = skipNodeAndChildren(node);
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/optimizer/lib/transformers/PreloadImages.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

const {createElement, nextNode, insertAfter, firstChildByTag} = require('../NodeUtils');
const {findMetaViewport, skipNodeAndChildren} = require('../HtmlDomHelper');
const {isTemplate} = require('../AmpConstants');

// Maximum number of images that will be preloaded.
const MAX_PRELOADED_IMAGES = 5;
Expand Down Expand Up @@ -50,7 +51,7 @@ class PreloadImages {
if (preloadImageMap.size >= imagePreloadCount) {
break;
}
if (node.tagName === 'template') {
if (isTemplate(node)) {
node = skipNodeAndChildren(node);
} else {
this.addImage(preloadImageMap, node);
Expand Down
17 changes: 11 additions & 6 deletions packages/optimizer/lib/transformers/ServerSideRendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ const {
nextNode,
firstChildByTag,
} = require('../NodeUtils');
const {skipNodeAndChildren} = require('../HtmlDomHelper');
const {isRenderDelayingExtension, isCustomElement} = require('../Extensions.js');
const {applyLayout} = require('./ApplyLayout.js');
const {isTemplate} = require('../AmpConstants');
const ApplyCommonAttributes = require('./ApplyCommonAttributes');

class ServerSideRendering {
Expand Down Expand Up @@ -64,18 +66,13 @@ class ServerSideRendering {
// where possible, but while we're at this we also look for reasons
// not to remove the boilerplate.
let canRemoveBoilerplate = true;
for (let node = body; node; node = nextNode(node)) {
for (let node = body; node; node = this.nextNonTemplateNode(node)) {
applyCommonAttributes.addNode(node);
// Skip tags that are not AMP custom elements.
if (!isCustomElement(node)) {
continue;
}

// Skip tags inside a template tag.
if (this._hasAncestorWithTag(node, 'template')) {
continue;
}

// amp-experiment is a render delaying extension iff the tag is used in
// the doc. We check for that here rather than checking for the existence
// of the amp-experiment script in IsRenderDelayingExtension below.
Expand Down Expand Up @@ -206,6 +203,14 @@ class ServerSideRendering {
return false;
}
}

nextNonTemplateNode(node) {
if (isTemplate(node)) {
return skipNodeAndChildren(node);
} else {
return nextNode(node);
}
}
}

module.exports = ServerSideRendering;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!doctype html>
<html >
<head>
<meta charset="utf-8">
<title>My AMP Page</title>
<link rel="canonical" href="self.html">
<meta name="viewport" content="width=device-width,minimum-scale=1">
<script async src="https://cdn.ampproject.org/v0/amp-mustache-0.2.js" custom-template="amp-mustache"></script>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<script type="text/plain" template="amp-mustache">
Hello {{world}}!
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!doctype html>
<html >
<head>
<meta charset="utf-8">
<title>My AMP Page</title>
<link rel="canonical" href="self.html" />
<meta name="viewport" content="width=device-width,minimum-scale=1">
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<script type="text/plain" template="amp-mustache">
Hello {{world}}!
</script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,11 @@
<template>
<amp-img height="42" layout="responsive" width="42"></amp-img>
</template>
<script type="text/plain" template="amp-mustache">
<amp-img height="42" layout="responsive" width="42"></amp-img>
</script>
<amp-img height="42" layout="responsive" width="42" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
<i-amphtml-sizer style="display:block;padding-top:100%"></i-amphtml-sizer>
</amp-img>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
<html >
<head></head>
<body>
<template><amp-img height=42 layout=responsive width=42></amp-img></template>
<template>
<amp-img height="42" layout="responsive" width="42"></amp-img>
</template>
<script type="text/plain" template="amp-mustache">
<amp-img height="42" layout="responsive" width="42"></amp-img>
</script>
<amp-img height="42" layout="responsive" width="42"></amp-img>
</body>
</html>

0 comments on commit d570011

Please sign in to comment.