Support amp-facebook-comments, -like, and -page with one extension binary (1.0 only)

[caroqliu]

Implements #34969

In draft mode until #35027 is merged, at which point unit tests failures will also be resolved.

Open question for @ampproject/wg-caching:
Is it possible to express validation rules forcing usage of script tags importing amp-facebook-1.0.js and any of [amp-facebook-comments-0.1.js, amp-facebook-like-0.1.js, amp-facebook-page-0.1.js] to be mutually exclusive? That is, a page can import amp-facebook-1.0.js as long as it does not import any of the others, or it can import any subset of the others. If not, pages will not be broken but may need additional documentation explaining the behavior to expect.

[honeybadgerdontcare]

Open question for @ampproject/wg-caching:
Is it possible to express validation rules forcing usage of script tags importing amp-facebook-1.0.js and any of [amp-facebook-comments-0.1.js, amp-facebook-like-0.1.js, amp-facebook-page-0.1.js] to be mutually exclusive? That is, a page can import amp-facebook-1.0.js as long as it does not import any of the others, or it can import any subset of the others. If not, pages will not be broken but may need additional documentation explaining the behavior to expect.

We've accomplished this before using satisfies and excludes ( validator.proto ) in new Bento 1.0 versions. I believe you have seen it in usage. Since it's a generic condition something like this should work:

amp-facebook-1.0.js tagspec:
satisfies: "amp facebook 1.0"
excludes: "amp facebook 0.1"

amp-facebook-[comments|like|page]-0.1.js tagspecs:
satisfies: "amp facebook 0.1"
excludes: "amp facebook 1.0"

Tests ftw.

[caroqliu]

Open question for @ampproject/wg-caching:
Is it possible to express validation rules forcing usage of script tags importing amp-facebook-1.0.js and any of [amp-facebook-comments-0.1.js, amp-facebook-like-0.1.js, amp-facebook-page-0.1.js] to be mutually exclusive? That is, a page can import amp-facebook-1.0.js as long as it does not import any of the others, or it can import any subset of the others. If not, pages will not be broken but may need additional documentation explaining the behavior to expect.

We've accomplished this before using satisfies and excludes ( validator.proto ) in new Bento 1.0 versions. I believe you have seen it in usage. Since it's a generic condition something like this should work:

amp-facebook-1.0.js tagspec:
satisfies: "amp facebook 1.0"
excludes: "amp facebook 0.1"

amp-facebook-[comments|like|page]-0.1.js tagspecs:
satisfies: "amp facebook 0.1"
excludes: "amp facebook 1.0"

Tests ftw.

Thanks for this! Tests was a great call, and really helps to illustrate what I'm trying to express.

I took a stab at this in the most recent commit and am running into a couple issues:

• The script inclusion is not exclusive after applying satisfies/excludes, as can be seen with the passing cases that should be failing in extensions/amp-facebook/1.0/test/validator-amp-facebook-exclusive.html
• Use of the amp-facebook-* components is not valid with the amp-facebook-1.0.js script alone -- it appears to be running into the requires_extension line, i.e.:
  

  amphtml/extensions/amp-facebook-page/validator-amp-facebook-page.protoascii

  Line 30 in 5869aed

  requires_extension: "amp-facebook-page"

  
  I assume there's not such a thing as requires_extension_oneof like we have with other properties? Example test file that should pass but fails: extensions/amp-facebook/1.0/test/validator-amp-facebook-page.out

[amp-owners-bot]

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-facebook/validator-amp-facebook.protoascii

[honeybadgerdontcare]

requires_extension_oneof

There is not. I'll try to find time tonight to think of some alternatives.

[caroqliu]

requires_extension_oneof

There is not. I'll try to find time tonight to think of some alternatives.

I'm going to move the validation changes to a separate PR for discussion, given the other changes have been approved. Thanks!