facebook / facebook/astryx

Template Grading Rubric: image/CSS exceptions can't score A, and Image Handling cites the wrong CDN

Open
#2,584 2 comments 0 reactions 1 assignee Claimed by @rubyycheung View on GitHub
bug documentation
Dominant language
TypeScript
Stars
13k
Forks
1.1k
Avg merge
1d 15h
Merged PRs (30d)
690

Description

## Summary

The **Template Grading Rubric** in the [Contributing-Templates wiki](https://github.com/facebookexperimental/xds/wiki/Contributing-Templates#template-grading-rubric) has two internal contradictions that cause it to penalize templates for following its own documented rules. Surfaced while grading and improving the `classic-gallery` page template.

---

## Issue 1 — "Necessary exceptions" are still capped below A (the ceiling bug)

The rubric explicitly lists `` (with an `xds_oss` CDN src) and `` wrapping `XDSFormLayout` as **necessary exceptions** that should **not** be penalized:

> **Necessary exceptions** (don't penalize):
> - `` with xds_oss CDN src (XDS doesn't have a general Image component)
> - `` wrapping `XDSFormLayout` for native submission semantics

But the scoring **tables** only award full marks at *literally zero* raw elements / declarations:

**XDS Component Purity (30 pts)**
| Raw HTML elements | Points |
|---|---|
| 0 | 30 |
| 1–2 (all necessary) | 25 |

**Custom CSS (15 pts)**
| Total custom style declarations | Points |
|---|---|
| 0 | 15 |
| 1–3 (justified — no XDS alternative) | 12 |

### Why this is a contradiction

Any **image gallery** needs at least one ``. A cover-fit image also irreducibly needs `width: 100%`, `height: 100%`, `object-fit: cover` (3 declarations) plus the `stylex.props()` call site — none of which have an XDS prop equivalent (there is no XDS `Image` component).

Result: a *perfectly built* image template is hard-capped at roughly:
- Purity **25/30** (one necessary ``)
- Custom CSS **5/15** (cover-fit lands in the "4–10" band)

…i.e. **~85 (B) maximum, and an A (90+) is impossible**, no matter how clean the code is. The rubric says "don't penalize" the exception, but the math penalizes it ~15 points and blocks the top grade.

This affects a whole class of templates (galleries, hero images, anything media-heavy). Notably the **official `AspectRatioImageGallery` block** (`packages/cli/templates/blocks/components/AspectRatio/AspectRatioImageGallery.tsx`) uses `` with inline `style={{ objectFit: 'cover', ... }}` — it would also be capped by its own rubric.

### Suggested fix
- Make the "necessary exception" actually neutral in the tables, e.g.:
- **Purity:** "0 raw HTML *(excluding necessary ``/`` exceptions)* → 30."
- **Custom CSS:** exclude the irreducible image cover-fit declarations (`width/height/object-fit` on an `` inside `XDSAspectRatio`) from the count, or add an "image cover-fit only" full-credit row.
- Alternatively, cap the *deduction* for documented exceptions at 0 so a clean gallery can reach A.

---

## Issue 2 — Image Handling cites `scontent…fbcdn.net`, contradicting the wiki's own image guide

**Category 6 — Image Handling (5 pts)** says:

> - **Correct**: CDN URLs from `xds_oss` asset set (`https://scontent.xx.fbcdn.net/...`).

But the same wiki's **"How to Use Images in Templates"** section says the opposite — use the **permanent `lookaside.facebook.com`** URLs and explicitly **warns against** the `scontent…fbcdn.net` signed URLs:

> Use the **permanent lookaside URL** — it never expires and requires no authentication:
> `https://lookaside.facebook.com/assets/{set}/{asset}.{ext}`
>
> > ⚠️ **Do NOT use the signed CDN URLs** (the `https://scontent.xx.fbcdn.net/...?oh=...&oe=...` URLs). Those contain time-limited tokens that expire after a few days.

So the rubric grades "correct" using the exact URL form the guide says is wrong (and which expires). A template using the recommended `lookaside.facebook.com` URLs technically doesn't match the rubric's stated "correct" pattern.

### Suggested fix
Update Category 6 to cite `https://lookaside.facebook.com/assets/xds_oss/...` as the correct form and list expiring `scontent…fbcdn.net` signed URLs under **Incorrect**, matching the image guide.

---

## Repro / context

Found while grading `classic-gallery` (`packages/cli/templates/pages/classic-gallery/page.tsx`). After refactoring it to be genuinely clean (raw `

` frame → `XDSAspectRatio`, layout via `contentWidth`/`padding` props, only the irreducible image cover-fit CSS left), it still tops out at **85/100** purely because of Issue 1.

## Acceptance criteria
- [ ] A flawless image/`` template can score **A (90+)** under the rubric.
- [ ] Category 6 "correct" example uses the permanent `lookaside.facebook.com` URL; expiring `scontent…fbcdn.net` URLs are listed as incorrect.
- [ ] Rubric prose and scoring tables agree on the treatment of documented exceptions.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.