ampproject / ampproject/amphtml

FR: Release extension-specific CSS files separately from extension binaries

Open
#34,878 1 comment 0 reactions 0 assignees View on GitHub
Stale Type: Feature Request
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

### Description

Bento AMP components provide "pre-upgrade" styles that are made available to ensure layout stability inside the component before it is defined and upgraded. Because v0 and the AMP boilerplate provide this level of layout stability already on AMP documents, the pre-upgrade styles are only needed for Bento components used on non-AMP pages.

The way this is documented on amp.dev is for authors to include a separate `` tag in addition to the `` tag for the component:
<img width="632" alt="Screen Shot 2021-06-15 at 3 00 02 PM" src="https://user-images.githubusercontent.com/10456171/122118010-8e1ed900-cdf5-11eb-9c1f-c380b3b751e9.png">

This PR requests the following build features regarding CSS:
- Support for building and releasing one or more CSS files `!== "amp-foo.css"` via `options.cssBinaries` that is independent of `options.hasCss`.
- Support for the above to be released with its given name(s) instead of being written to `amp-foo.css` when `!options.hasCss` in a way that avoids collisions if multiple extensions with identical .css files are used at once on a page.
- Support for releasing both `amp-foo.css` and extension-specific CSS files `!== "amp-foo.css"` when `options.cssBinaries` and `options.hasCss` are both configured. (Currently, when using both, they write to a single `amp-foo.css`.)

### Use case: Exclusively pre-upgrade styles
`amp-fit-text` has **exclusively** pre-upgrade styles currently living in [`amp-fit-text.css`](https://github.com/ampproject/amphtml/blob/main/extensions/amp-fit-text/1.0/amp-fit-text.css), which we currently recommend to be included on non-AMP pages:
```html
<script async custom-element="amp-fit-text" src="https://cdn.ampproject.org/v0/amp-fit-text-1.0.js">

```
This FR would allow us to rename and release the CSS payload as `custom-element.css` and recommend to be included on non-AMP pages as something like the following, given an original name of `custom-element.css` and a desire not to collide a common CSS filename across multiple extensions:
```html

```

### Use case: Separate pre-upgrade and universal styles
`amp-social-share` has both pre-upgrade and universal styles currently living together in [`amp-social-share.css`](https://github.com/ampproject/amphtml/blob/main/extensions/amp-social-share/1.0/amp-social-share.css), which we want to be able to separate into [`custom-element.css`](https://github.com/ampproject/amphtml/blob/0c15f513d1cdc4acd06a2a7d0d30c3bad943df06/extensions/amp-social-share/1.0/amp-social-share.css#L17-L36) and `amp-social-share.css` -- the former to be recommended to be included on non-AMP pages as the following:
```html

```
and the latter to be registered and bundled with the AMP component for use on AMP pages and non-AMP pages alike:
```js
import {CSS} from '../../../build/amp-social-share-1.0.css';

AMP.extension(TAG, '1.0', (AMP) => {
AMP.registerElement(TAG, AmpSocialShare, CSS);
});
```

/cc @ampproject/wg-infra @alanorozco

### Alternatives Considered

- We could allow all pre-upgrade styles to be redundantly provided as `amp-foo.css` with `options.hasCss: true` and registered via `AMP.registerElement`.
- This requires no infrastructure changes and involves including a `dist/v0/amp-foo-1.0.css` file that is 300 bytes for components that have exclusively pre-upgrade styles that are not needed on AMP pages. An example of a component that currently does this is `amp-fit-text:1.0`.
- We could allow all pre-upgrade styles to be released as `amp-foo.css` with `options.hasCss:true` and **not registered** via `AMP.registerElement` for components that have exclusively pre-upgrade styles that are not needed on AMP pages.
- This requires no infrastructure changes and saves us the aforementioned bytes and redundancy. An example of a component that currently does this is `amp-video:1.0`.
- It is not flexible if we decide to release Bento implementations with a different custom element tag name, i.e. `bento-foo` instead.
- It is a bit of a misnomer since it is not used on AMP documents.
- We could allow all pre-upgrade styles to be released as `amp-foo.css` when one does not exist already but can be created from `options.cssBinaries`. This can already be achieved by writing to `amp-foo.css` instead of the files listed in `options.cssBinaries`, so doesn't get us anything particularly more.

None of the above options allow for separating pre-upgrade styles from universal styles, meaning components that do have universal styles will have to include their pre-upgrade styles for the additional 300 bytes. However, this only implicates a minority of Bento components, i.e. `amp-accordion`, `amp-selector`, and `amp-social-share`.

### Additional Context

_No response_

Contributor guide

Open the contributing guide

Research direction

Start by locating the build code that handles options.cssBinaries and options.hasCss, then inspect the referenced amp-fit-text.css and amp-social-share.css files. Verify how CSS filenames and multiple extensions are emitted and how AMP.registerElement receives universal styles. Done means separate named CSS files can be released without collisions while existing amp-foo.css output still works when both options are configured.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
build-system, release
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.