Should Bento component implementations be compatible with Amp Services?

[scottrippey]

Many Amp components rely on Services for many of their features. These services can be purely utility (eg. platform), while some maintain state or cache (eg. xhr), and some have multiple implementations to work across document boundaries (eg. viewer).

To convert these Amp components to Bento, we need to decide how these features will be ported.

And if Bento components are intended to be used in Amp documents, then it’s especially important to determine how Bento components can utilize Amp-specific logic when necessary (like xhr batching, preconnects, templates, amp-cache awareness, etc) so they can be feature-complete replacements.

[scottrippey]

Approach #1: Abstraction

Many existing Bento components use optional props to keep Amp-specific features separate from the Bento implementation.

For example, with amp-date-countdown , the Bento component has an overridable render property. The AMP-wrapper for this component overrides the render function, and uses the Template Service to render the data.

Advantages

Many components have already been ported this way.

Sometimes the abstraction is a useful public API (like the custom render function).

Limitations

This approach only works fine for small-scale abstractions. A custom render method is a simple, self-contained abstraction.

However, some components rely heavily on multiple Services, and it would be very difficult to attempt to abstract away all service calls. For example, amp-app-banner depends on 6 services with 12 method calls, and there is no clear boundary where an abstraction could be made.

Additionally, some Bento components have already been ported WITHOUT certain Service-powered features. For example, the Bento version of amp-sidebar does not utilize any of the actions or timer or url service calls that the original version used; possibly because these service calls were too difficult to abstract and port.

[alanorozco]

I agree that these abstractions are more specific than render in a lot of cases. However, we've found that defining them granularly allows flexibility in two ways:

1. Services may be tied to AMP-related specifics. For example, there's a viewer service for which there is no notion of in Bento.

2. Picking and choosing allows us to eschew the use of some methods in cases that a native implementation works just fine in all modes. This is the case for timer and url.

In the case of amp-app-banner specifically, some Service uses are obsolete:

• platform can use a default implementation in all modes, so it doesn't require drilling down an abstraction.
• url is the same
• viewer-related logic is only required on the AMP layer, either by short-circuiting a layoutCallback, or by wrapping in an AMP-specific Preact component.

Other Services have equivalent abstractions that are arguably as well defined as render:

• storage can be abstracted as props onDismiss and isDismissed, the latter may take promises.
• xhr can be abstracted as prop getManifest
• preconnect can be replaced with a getPreconnects method on the AMP layer, like amp-twitter & friends.

[scottrippey]

Overall this makes sense.

• Some services are AMP-only (and would therefore only exist in the AMP layer)
• Some services are just utility (and could be replaced with standalone utilities)
• The remaining services can be abstracted. The component's public API can be extended to support overriding certain behaviors with AMP-specific logic.

One thing that maybe I'm not understanding fully, concerning "utility services" ... in my last talk with @jridgewell , he explained how many of these utility services are "doc-level" services, tied to a specific document (which is necessary due to FIE). So any time we try to use document, like document.querySelector or document.createElement, we cannot simply use the global document; we have to figure out which document is associated with the current component first.
So, the point of services like url or docInfo, is to ensure the canonicalUrl or a doc's <meta> tag is scoped to the correct document.
Is this behavior still required, or is this information obsolete?

[scottrippey]

Approach #2: Service injection

For components that rely heavily on Services, we could start by creating “standalone” versions of these services (with similar APIs). Then, the Bento component would use these standalone services, unless the AMP-wrapper provided/injected a different implementation.

Advantages

This would allow Bento components to be ported more easily, since the service calls would remain about the same. It would allow Bento components to take advantage of features that only exist in AMP documents (eg. history, xhr).

Limitations

Implementing the standalone versions might be ugly; there might be a lot of methods that are not applicable (therefore are just noops), or could be needlessly async (so the API signature matches).

It might be challenging to ensure that the “standalone” services can be tree-shaken (dead code removal).

Example

function BentoComponent({ AmpServices }) {
  const xhr = AmpServices?.get('xhr') || xhrStandalone;
  const doc = AmpServices?.get('doc') || docStandalone;
  const url = AmpServices?.get('url') || urlStandalone;

  // Example usage:
  const result = getAppInfo({ xhr, doc, url });
  return <>{result}</>;
}

[alanorozco]

As mentioned in my other comment, AMP services are too coarse to include in the Bento implementation. Tree-shaking is particularly problematic due to standalone alternatives needing to be just as coarse.

[scottrippey]

Apart from tree-shaking, what other objections do you have to this approach? Tree-shaking can be solved, but I'd like to know of other fundamental issues.

For example:

import { fetchJson } from '#preact/utils/xhr';
const xhrStandalone = { fetchJson };

[samouri]

I was only able to make the second half of the design review. Can anyone summarize why we ended up leaning towards Option 2 instead of Option 1 (especially given Alan's comment).

I do see how (2) would allow us to think less and move faster, but I'm worried about increasing the complexity within Bento to serve AMP, as well as long-term performance due to worse DCE.

If end up choosing (2), then I think we should also rename AmpService --> Service and ensure that the exact object shape is well-typed and enforced.

[scottrippey]

Approach #3: Document-level scoping of utils

As discussed in approach 1 above, there are "AMP-only" services (not needed by Bento), and there are "utility" services (useful to Bento).
The primary issue with these utility services is the "document scope" issue. In AMP mode, we are not assured that the global document is the correct scope. In React-land, however, we do not have have access to the web-component element either, so we cannot find the correct scope.

Instead of injecting full AMP services into Bento components (approach 2) to solve this issue, we can instead inject just the ownerDocument (when in AMP mode). All our utilities that need document can then use this ownerDocument instead.

Advantages

• This completely decouples Bento components from AMP services, but allows us to implement document-scoping with a minimal API footprint.
• We do not need extra abstractions in order to achieve doc-scoping
• API is consistent across Bento components
• API is clear, even outside AMP mode
• No bloat

Limitations

• Only solves the problem of document-scoping
• Does not solve other Services (approach 1 - abstraction - will still be necessary)

Example:

import urlUtils from '#preact/utils/url';
import docInfo from '#preact/utils/docInfo';

function BentoComponent({ 
  ownerDocument = document, // only overridden in AMP mode
  url, // some prop
}) {
  const parsedUrl = urlUtils.parse(ownerDocument, url);
  const canonicalUrl = docInfo.canonicalUrl(ownerDocument)
  // ... etc ...
  return <>{result}</>;
}

[jridgewell]

This approach would be fine with me too. The benefit I see is that we don't need to maintain two separate service implementations for the Bento component, just that will have the different behaviors inside the function calls.

[scottrippey]

FWIW: Approach #3 was implemented here: ampproject/amphtml#37465

However, after further discussions, we have dropped support for AMP mode within Bento components. Therefore, this issue is no longer relevant, and the PR above was abandoned.