Hi,

Although this may not have been the original intent of this tool it is, unfortunately, the way my infrastructure is built.

We have our canonical pages on https://www.sportsmole.co.uk and our AMP pages on https://amp.sportsmole.co.uk with a front-end cache (Varnish 5.2.1) that sends mobile/tablet (touch) users to the AMP pages and desktop users to their site of choice.

I am keen to add to the mix the idea of pre-rendering in the same way that Google does with its amp cache and in the same way that others do (CloudFlare).

As a first test I decided to use the amp-toolbox optimizer to create copies of pages and place them on a separate URL http://cache.sportsmole.co.uk/ to see if I can get them to work and immediately I had an issue of the CORS config. (the https version can be found at https://d23hbvp42pjqt1.cloudfront.net/index.html)

Https://cache.sportsmole.co.uk is on an amazon S3 bucket for the moment and the CORS config is set to:

<?xml version="1.0" encoding="UTF-8"?>
<CORSConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<CORSRule>
    <AllowedOrigin>*</AllowedOrigin>
    <AllowedMethod>GET</AllowedMethod>
    <MaxAgeSeconds>3000</MaxAgeSeconds>
    <ExposeHeader>AMP-Redirect-To,AMP-Access-Control-Allow-Source-Origin</ExposeHeader>
    <AllowedHeader>Content-Type,Content-Length,Accept-Encoding,X-CSRF-Token</AllowedHeader>
</CORSRule>
</CORSConfiguration>

However when I try to load the page

Failed to load https://amp.sportsmole.co.uk/amp_ping/amp-fu2d6CFNdnwE9PNbB4sNDw0.93244027565443541/?rid=amp-fu2d6CFNdnwE9PNbB4sNDw&uri=https%3A%2F%2Fd23hbvp42pjqt1.cloudfront.net%2Findex.html&pass=PIK&_=0.22654620531193914&host=amp.sportsmole.co.uk&ht=https:&FB=OK&ref=https%3A%2F%2Fd23hbvp42pjqt1.cloudfront.net%2Findex.html&dynamic&__amp_source_origin=https%3A%2F%2Fd23hbvp42pjqt1.cloudfront.net: The 'Access-Control-Allow-Origin' header has a value 'https://amp.sportsmole.co.uk' that is not equal to the supplied origin. Origin 'https://d23hbvp42pjqt1.cloudfront.net' is therefore not allowed access. Have the server send the header with a valid value, or, if an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

Are there any tips on doing this kind of thing or is it out of hte scope of the project it is what the google cache does after all?

Thans
Alan

timestamp parameter in url is undefined.

&amp_ts=undefined

  const privateKey = req.body.privatekey;
  const url = req.body.url;

  // Generate update-cache URLs.
  const updateCacheUrlProvider = UpdateCacheUrlProvider.create(privateKey);
  updateCacheUrlProvider.calculateFromOriginUrl(url)
    .then(refreshUrlInfos => {
      // Append CORS headers.
      const requestingOrigin = req.headers.origin;
      const requestingSourceOrigin = req.query.__amp_source_origin;
      res.header('Access-Control-Allow-Origin', requestingOrigin);
      res.header('Access-Control-Allow-Credentials', 'true');
      res.setHeader('Access-Control-Expose-Headers', 'AMP-Access-Control-Allow-Source-Origin');
      res.setHeader('AMP-Access-Control-Allow-Source-Origin', requestingSourceOrigin);

    refreshUrlInfos.forEach(cacheUpdateUrlInfo => {
        console.log('Cache ID:' + cacheUpdateUrlInfo.cacheId);
        console.log('Cache Name:' + cacheUpdateUrlInfo.cacheName);
        console.log('cache-update URL:' + cacheUpdateUrlInfo.updateCacheUrl);
      });

      // Sends generated URLs on the response.
      res.send(JSON.stringify({items: refreshUrlInfos}));
    });

Seems amp-img with srcset are ignored in the preload images optimisation.

Would require this first: #63

The AMP Cache has started serving the AMP runtime with brotli compression enabled:

https://cdn.ampproject.org/rtv/011902271810270/v0/amp-accordion-0.1.js.br

We should also support this for the AMP optimizer.

This requires two changes:

1. optimizer needs an option to rewrite amp runtime urls to use brotli compression.
2. optimizer-express: needs to serve different versions depending on whether the UA accepts br: accept-encoding: gzip, deflate, br. This probably needs to be opt-in as it also requires adjusting the caching layer.

//cc @jridgewell @honeybadgerdontcare is there anything else that we should be doing?

Per ampproject/amphtml#6920, SVG data: URIs must include ;charset=utf-8:

data:image/svg+xml;charset=utf-8,${ENCODED_SVG}

Support versioned runtime out-of-the-box, e.g. by introducing option: 'versionRuntime'.

Open question: should the middleware set the same headers as the AMP cache?

The update-cache specification allows developers to invalidate content on the AMP Cache.

This library should implement the pieces needed to make calls to update-cache easy for developers.

In the project readme it says, “You can find the currently supported transformations here.” But what are the not-supported transformations? If I incorporate the optimizer into my stack, what optimizations will be missing which are performed by the Google AMP Cache? There are currently 4 open issues but there is no mention of the the image optimizations documented in Google AMP Cache, Optimizations for Slow Networks, and the Need for Speed. The image optimizations are likewise not mentioned in the AMP Cache Guidelines nor AMP cache modifications best practices. There feels like a fragmentation of all the optimizations which may be performed by Google's AMP Cache. Having an exhaustive list of what exactly is done would be helpful for others who want to understand how their content is modified and for those who want to stand up their own caches.

I'm looking to identify what could be implemented at the hosting layer for WordPress installs to get (close to) the same speed benefits for browsers on origin that browsers do on an AMP cache. In addition to documenting which optimizations are not included in the amp-toolbox, having everything documented would be key to getting documentation for an “AMP Cache for Hosts”: providing guidelines for hosts to be able to integrate AMP caching optimizations into their own caching infrastructure so that users visiting origin directly on native AMP sites get almost all the same speed benefits as users accessing a site via an AMP cache

See what we can do to optimise font loading from the other supported font services:

Typography.com: https://cloud.typography.com
Fonts.com: https://fast.fonts.net
Typekit: https://use.typekit.net
Font Awesome: https://maxcdn.bootstrapcdn.com, https://use.fontawesome.com

//cc @westonruter @dakkad

The example code below returns error when I execute on node.

SyntaxError: await is only valid in async function

const ampOptimiser = require('amp-toolbox-optimizer');

// retrieve the latest runtime version
const runtimeVersion = require('amp-toolbox-runtime-version');

// retrieve the latest version
const ampRuntimeVersion = await runtimeVersion.currentVersion();

// The input string
const originalHtml = `
<!doctype html>
<html ⚡>
...
`

// Additional options can be passed as the second argument
const optimizedHtml = await ampOptimizer.transformHtml(originalHtml, {
  ampUrl: 'canonical.amp.html',
  ampRuntimeVersion: ampRuntimeVersion
});

console.log(optimizedHtml);

The default config for the optimizer is not exposed. When a developer needs to add a custom config it is necessary to re-add the whole configuration, like in the example below:

ampOptimizer.setConfig({
  transformers: [
    new CustomTransformer(),
    'AddAmpLink',
    'ServerSideRendering',
    'RemoveAmpAttribute',
    // needs to run after ServerSideRendering
    'AmpBoilerplateTransformer',
    // needs to run after ServerSideRendering
    'ReorderHeadTransformer',
    // needs to run after ReorderHeadTransformer
    'RewriteAmpUrls'
  ]
});

The new API should enable developers to add new Transformers. A proposal would be:

const config = new ConfigBuilder()
  .default() // Adds the default configuration
  .addPreloadImages() // Easy to enable non-default transformers
  .addTransformer(new CustomTransformer()) // Enables adding user authored transformers.
  .build();
ampOptimizer.setConfig(config);

Change the Transformer API to return Promises. This is in order enable the transformation to run asynchronously, and will prepare the API for transformations that require I/O operations, for instance.

Using the optimizer-express package, every image that is successfully rendered with AddBlurryImagePlaceholders is logged to the console - and with longer image urls creates a lot of noise in the logs. Could we add an option to suppress this output?

eg.
AMP Optimizer Adding blurry image placeholder for (src / resolved): https://www.example.com/longurl/image.jpg https://www.example.com/longurl/image.jpg

JSDoc doesn't match implementation: https://github.com/ampproject/amp-toolbox/blob/master/ssr-express/lib/AmpSsrMiddleware.js#L35

Wondering whether there's a performance benefit of moving link preload tags to the top of the document.

V0 loads quickly but other preloads e.g. fonts which we put in are after our custom CSS and the optimiser moves elements around so we can't alter the position of the preload tags.

Not sure how much impact but from the test below looks like the font could start 0.5 seconds earlier at same time as v0?

Test results:
http://www.webpagetest.org/video/compare.php?tests=180530_XG_e4621d79c21b9eed5a9444c6126f25ef-r:2-c:0

Provide an express middleware that fetches the list of AMP caches from caches.json and adds the corresponding CORS headers.

//cc @andreban

It seems that Google AMP Cache add target="_top" to all a links. Review if desired for amp-toolbox and if so implement a transformation so that it matches the AMP Cache one.

Create a re-usable express middleware, that can be installed as a Node package.

This middleware can expand on the one that is available on the existing demo (https://github.com/ampproject/amp-toolbox/blob/master/ssr/demo/express/app.js)

Image preload can result in <link rel="preload" href="{{image}}?w311h205" as="image">
When there is an amp-list mustache template e.g.

<amp-list src="https://www.standard.co.uk/pwamp/layout_component/sidebar-videos" items="articles" class="amp-list list-videos" layout="fixed-height" width="auto" height="370" media="(min-width: 930px)">
    <template type="amp-mustache">
        <a class="amp-list-item" href="{{link}}" data-vars-event-id="c23" data-vars-item-name="Popular Videos - {{link}}">
            <amp-img src="{{image}}?w311h205" width="96" height="64" layout="responsive" class="list-item-img"></amp-img>
            <span class="list-item-text">{{title}}</span>
        </a>
    </template>
</amp-list>

e.g. > in CSS becomes >

Can been seen on this page https://www.independent.co.uk/news/world/americas/donald-trump-tweets-red-hen-restaurant-sarah-huckabee-sanders-a8415711.html

Search for '>' in the inline CSS

"Putting such keyframes declarations at the bottom of the document in the <style amp-keyframes> allows them to exceed size limitations. And since keyframes are not render-blocking, it also avoids blocking first contentful paint to parse them."

https://www.ampproject.org/docs/fundamentals/spec#keyframes-stylesheet

We're seeing an issue with the bind 'on' being removed front end on the optimised version. The issue occurs inside amp-list on the optimised version. The lightbox works on the valid amp version

How to replicate: Scroll to comments section at bottom of articles and click 'reply' on a comment

Note some AMP bind items work e.g. the 'Subscribe' link will launch the lightbox.

AMP version: Reply

Inspect element on optimised version: Reply

Optimised amp: https://www.independent.co.uk/news/world/asia/trump-kim-meeting-live-updates-latest-time-date-location-stream-a8392686.html

Valid amp: https://www.independent.co.uk/news/world/asia/trump-kim-meeting-live-updates-latest-time-date-location-stream-a8392686.html?amp

Using amp-optimizer-express I'm getting the error:

AMP Optimizer cannot perform SSR: unsupported layout intrinsic
AMP Optimizer cannot remove boilerplate: unsupported layout

But I can't find any reason why it wouldn't be supported according to: https://www.ampproject.org/docs/design/responsive/control_layout

Enable by default, can be disabled via config "Access-Control-Allow-Credentials", "true"

Implement CLI for the library update-cache. Usage should be something like the following:

update-cache https://amp.example.com privateKey.pem

My app is ok with locar serve but I got an error on cors.js when trying to deploy to Firebase cloud :
Any thoughts please ?

The error log :

Function failed on loading user code. Error message: Code in file index.js can't be loaded. Is there a syntax error in your code? Detailed stack trace: /user_code/node_modules/amp-toolbox-cors/lib/cors.js:46 return async (request, response, next) => { ^ SyntaxError: Unexpected token ( at createScript (vm.js:56:10) at Object.runInThisContext (vm.js:97:10) at Module._compile (module.js:549:28) at Object.Module._extensions..js (module.js:586:10) at Module.load (module.js:494:32) at tryModuleLoad (module.js:453:12) at Function.Module._load (module.js:445:3) at Module.require (module.js:504:17) at require (internal/module.js:20:19) at Object.<anonymous> (/user_code/node_modules/amp-toolbox-cors/index.js:18:18)

The AMP Runtime (https://cdn.ampproject.org/v0.js) is cached for 30 mins. By using the versioned version, the caching time can be extended (eg: 1 year), which will improve re-use of the runtime.

The transformer will use the runtime-version component to query the latest version of the AMP runtime, and replace the URLs on the SSR version with the versioned one.

Overview

Refactor the optimizer to achieve two things:

• Separate transformers that result in invalid AMPHTML (e.g. AmpBoilerplateTransformer) from the rest.
• Modify the parser to make it more generic, specifically to make it compatible with posthtml

Background

There are several good practices (e.g. font preconnect) that are beneficial for all AMP pages. The optimizer should offer these transformations that transform "valid AMPHTML into better valid AMPHTML" in addition to offering further optimizations which result in invalid AMPHTML, which comes with a different tradeoff (speed over losing things that come with having valid AMP, such as the cache).

Making the parser compatible with posthtml would make it easy for the optimizer to integrate with an existing large posthtml ecosystem. For example, this allows users to easily combine posthtml transformations with amp-toolbox and allow the optimizer to use existing posthtml integrations, such as express, Koa and Hapi middlewares and integrations with gulp, grunt and webpack.

Proposal

• Separate transformers into two groups:
  • "preamp", for those that make existing valid AMP more efficient while remaining valid
  • "optimizer", which results in invalid AMPHTML
• Refactor TreeParser.js:
  • Replace extensions to Node prototype with utility functions.
  • Either one of these options:
    • Replace existing parser with posthtml-parser (should be compatible with htmlparser2 interface).
    • Replace existing parser with an adapter that's compatible with both htmlparser2 and PostHTML AST Format.

Thoughts / comments welcome!

preloads need to be declared after

meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">

otherwise media queries cannot be evaluated (when combined with preload)

We need to find a new way to determine the canary version.

//cc @jridgewell

see #132

TextEncoder is not supported on some browsers, thus it would be good to remove this and use something else.

cc @sebastianbenz @kristoferbaxter

Currently, if the input page already preloads v0.js, a second rtv preload will be added:

<link rel="preload" as="script" href="https://cdn.ampproject.org/v0.js">
<link rel="preload" href="https://cdn.ampproject.org/rtv/001535038987988/v0.js" as="script">

//@dakkad

The Cloudflare AMP Cache is missing in the list of AMP Caches. ;)
https://amp.cloudflare.com/

So at first I thought it might have been just me, but after doing another fresh clone / install of the project, I keep getting this error:

I think it may have to do with splitting the packages and introducing learna?

Hello, I need some help on enabling minification on https://github.com/ampproject/amp-toolbox/blob/master/packages/optimizer-express/demo/proxy.js
I tried to use html-minifier with a customTransformer, Middleware, but can't make it works.

I want to use this options from html-minifier:

const minify = require('html-minifier').minify;
minify(content, {
    collapseWhitespace: true,
    minifyCSS: true,
    removeComments: true,
  });

I also tried https://www.npmjs.com/package/express-minify-html as suggested by @torch2424 on slack

IS it possible to call REST API from amp html ?

https://github.com/ampproject/amp-toolbox/blob/master/ssr/README.md

"There are two sample implementations available:
express: a sample express server serving server-side-rendered AMP pages"

express links to a 404

As a feature request, it would be great if there were serverless version of some of the cors middleware, something that can be used with middy.

Hi,

I just got a Varnish error on the ampproject site (https://www.ampproject.org/) and so I can see this is something the team use :)

I'm a great fan of Varnish and wondered if anyone had thought of creating a version of the amp-toolbox to sit as part of the amp-toolbox project it seems like a perfect place to include such a tool as it would take all the complexity out of implementation and provide a way to make it transparent to the back end and the front end user.

As discussed with @sebastianbenz:
We saw this happen on nextjs.org/blog where when width is set it calculates a placeholder that is too high / out of shape, causing it to fill up empty space that is removed when the client-side AMP runtime kicks in. Which in turn causes a flicker when loading the page, here's an example with javascript disabled:

It only happened with <amp-timeago width="100" height="15" layout="responsive"> for example.

I've fixed it by setting width to 0: https://github.com/zeit/next-site/pull/305/files#diff-5a193e4adc43516f19490e0dab4c86c0R111

Which opts into the bailout here:

A number of optimiser modules could make use of logs to warn of potential issue cases for example:

<link rel=prefetch href=/img/asset.png />
<link rel=prefetch href=/img/asset.png as=image />

In this case it is hard for the duplicate filter to decide which is the correct definition. It is clear that there is a duplication but the optional type as attribute validity can't be determined as part of a filter task. In this case offering warnings to the developer to fix issues would be preferable.

Adding logging support across the project to allow consistent messaging.

We should make sure that our API uses this field.

[ { id: 'google',
    name: 'Google AMP Cache',
    docs: 'https://developers.google.com/amp/cache/',
    cacheDomain: 'cdn.ampproject.org',
    updateCacheApiDomainSuffix: 'cdn.ampproject.org',
    thirdPartyFrameDomainSuffix: 'ampproject.net' },
  { id: 'cloudflare',
    name: 'Cloudflare AMP Cache',
    docs: 'https://amp.cloudflare.com/',
    cacheDomain: 'cdn.cloudflare.com',
    updateCacheApiDomainSuffix: 'amp.cloudflare.com',
    thirdPartyFrameDomainSuffix: 'cloudflareamp.net' } ]

Might be worth moving all the amp-toolbox-* packages into an @amptoolbox/* scope (more info) for clarity, and to prevent amp-toolbox-foo from being created in the same "namespace". (I don't think there's anything to prevent this at the moment?)

The scopes @amp, @amphtml or @ampproject may be better/worse, perhaps depending on whether amphtml-validator is also moved.

• Avoid duplicating preloads (e.g. Google fonts)
• rewrite existing v0.js preload if one already exists (for versioned URLs)

See sample here: https://amp-by-example-ssr2.appspot.com/components/amp-img/