Skip to content

refactor(assets-sync): move Content-Type into _headers, drop assets.toml#66

Merged
lwshang merged 1 commit into
mainfrom
lwshang/drop_assets_toml
Jun 1, 2026
Merged

refactor(assets-sync): move Content-Type into _headers, drop assets.toml#66
lwshang merged 1 commit into
mainfrom
lwshang/drop_assets_toml

Conversation

@lwshang
Copy link
Copy Markdown
Collaborator

@lwshang lwshang commented May 29, 2026

Summary

Removes the assets.toml config introduced in #64 and folds the only feature it carried — per-glob Content-Type overrides — back into _headers. The plugin now recognises Content-Type: inside any _headers block, parses the value as a MIME, and routes it to CreateAssetArguments.content_type rather than the appended response headers — so the canister still emits exactly one certified Content-Type per response.

Net diff: −634 / +287 across 21 files. Drops a config format, a parser module, an e2e fixture, a top-level design doc, and the toml workspace dep.

Why Content-Type belongs in _headers, not its own file

#64 sat on the position that Content-Type was asset metadata, not a response header, and therefore deserved its own file. That framing was driven by the canister's wire shape (appends _headers without dedup, so a Content-Type: rule would produce two on the wire) — i.e. an implementation concern, not a user concern.

  • Users' mental model wins. Every static-host tool (Netlify, Cloudflare Pages) lets users set Content-Type in _headers. To a user, it's just a header — splitting it across two files to satisfy our pipeline is the kind of design that makes them say "why is this weird."
  • The wire problem is plumbing, not architecture. Solving it in the plugin (extract Content-Type from _headers, route to content_type, exclude from the appended list) keeps the canister's append-without-dedup invariant intact and gives the user the familiar one-file experience.
  • The other speculative assets.toml fields didn't survive review. ignore belongs in the build step (don't put the file in dist/ if it shouldn't deploy). encodings has no driving use case — the current text/* + js/html → identity+gzip, everything else → identity policy covers real projects. allow_raw_access is being dropped on the canister side (selective certification via the HTTP gateway spec covers what it was for). With no remaining fields, the config file has no reason to exist.

The pivot rests on a principle: adding a config format later when a real need shows up is cheaper than carrying an unused format now.

Other design points

  • Parser shape. HeaderRule gains content_type: Option<Mime>. Parsing a Content-Type: line is case-insensitive, validates as Mime, rejects empty values, and rejects duplicates within the same block. Other headers in the same block continue to flow through headers as usual (assets-sync/src/headers.rs).
  • Resolver. New content_type_for(key, rules) walks rules in declaration order and returns the first matching content_type — first-match-wins because Content-Type is single-valued (accumulation semantics make no sense for it). Other matching rules still contribute their non-Content-Type headers (assets-sync/src/headers.rs).
  • Pipeline. prepare_asset now takes &[HeaderRule] (instead of the deleted &AssetConfig) and applies content_type_for before encoders_for — so a .did declared as text/plain still picks up gzip and still triggers drift detection on re-deploy. The header_content_type_override_applies_to_prepare_asset unit test pins this end-to-end (assets-sync/src/sync.rs).
  • sync() signature. Lost its files: &[(String, String)] parameter — no consumer remains. The plugin entry no longer threads input.files through; if a future feature needs inline files, it just reads them.
  • Deletions. assets-sync/src/asset_config.rs, ASSETS-TOML.md, the toml workspace dep, the assets-toml e2e fixture, and the assets-toml-only fields from the developer-docs cargo-cult example (the e2e fixture now demonstrates .did only, the one extension mime_guess genuinely can't classify).

Test plan

New coverage in headers.rs tests: Content-Type routes to dedicated field not headers, case-insensitive, coexists with other headers, block-with-only-Content-Type is valid, rejects duplicate / invalid / empty values; content_type_for returns None / first-match-wins / skips rules without a content_type declaration.

🤖 Generated with Claude Code

raymondk added a commit to dfinity/developer-docs that referenced this pull request Jun 1, 2026
#280)

## Summary
- Pulls `canister.wasm` and `plugin.wasm` from the
[dfinity/certified-assets `snapshot-pr69-7571cba`
release](https://github.com/dfinity/certified-assets/releases/tag/snapshot-pr69-7571cba)
via `url:` + `sha256:` so `icp` verifies integrity before use.
- Adds `public/_headers` and `public/_redirects` to exercise the new
asset-canister tooling's headers and redirects support.
- Drops `assets.toml` and the `files:` entry in `icp.yaml`. With
[certified-assets PR
#66](dfinity/certified-assets#66),
`Content-Type` is read directly from `_headers`, so the separate TOML
file is no longer needed. The single non-default override (`.did` →
`text/plain; charset=utf-8`) now lives in `public/_headers`.
- Drops `public/.ic-assets.json5` — the legacy `headers.Content-Type`
workaround (which produced duplicate `Content-Type` headers in the
certified response) is fully superseded.
- Bumps `icp-cli` to v0.2.7.

## Notes
- To demo cross-domain redirects: add a line like `/xxx
https://google.com 301` in `public/_redirects` and deploy — visiting
`/xxx` should bounce to `https://google.com`.

## Test plan
- [x] `icp deploy` downloads both wasms, verifies SHA256, and deploys
- [x] Headers in `public/_headers` are served on matching paths (CSP,
cache rules)
- [x] Same-origin redirects in `public/_redirects` resolve (e.g. `/*
/404.html 404`)
- [x] Cross-domain redirect works after adding `/xxx https://google.com
301`
- [x] `curl -I` on `*.md` returns `Content-Type: text/markdown;
charset=utf-8` (exactly one header)
- [x] `curl -I` on `*.did`, `*.sh`, and `llms.txt` returns
`Content-Type: text/plain; charset=utf-8` (exactly one header)

---------

Co-authored-by: Raymond Khalife <raymond.khalife@dfinity.org>
lwshang added a commit that referenced this pull request Jun 1, 2026
> **Superseded by #66.** The dedicated `assets.toml` config was scrapped
in favour of folding per-glob `Content-Type` overrides into `_headers` —
same feature, single familiar file, no new format to learn. See #66 for
the design rationale and the replacement implementation.

## Summary (what this PR would have done)

Added an opt-in `assets.toml` config file (passed inline via the
manifest's `files:` field) that let users override an asset's
`Content-Type` by glob pattern. v1 scoped to this one field; broader
`[[asset]]` knobs (`ignore`, `encodings`, `allow_raw_access`) were
sketched in `ASSETS-TOML.md` for follow-ups.

Replaced by #66 which expresses the same overrides inside `_headers`
(e.g. `/*.did\n Content-Type: text/plain; charset=utf-8`). The plugin
extracts `Content-Type` from `_headers` and routes it into
`CreateAssetArguments.content_type` so the canister still emits exactly
one certified `Content-Type` per response.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Content-Type` is now declared inside any `_headers` block: the parser
routes it to a dedicated `HeaderRule.content_type` field (Mime-validated)
and the sync pipeline feeds it into `CreateAssetArguments.content_type`
instead of the appended response headers — so the canister still emits
exactly one certified Content-Type per response.

Removes the `assets.toml` config file introduced in 4788ef6. From a
user's mental model Content-Type is just a header, and the only
remaining justification for a second config file (sketched v2 fields
`ignore` / `encodings` / `allow_raw_access`) is either covered by the
build pipeline, satisfied by current defaults, or no longer in scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lwshang lwshang marked this pull request as ready for review June 1, 2026 19:54
@lwshang lwshang requested a review from a team as a code owner June 1, 2026 19:54
@lwshang lwshang changed the base branch from lwshang/perf to main June 1, 2026 19:54
@lwshang lwshang force-pushed the lwshang/drop_assets_toml branch from 719f27a to b092222 Compare June 1, 2026 19:54
@lwshang lwshang enabled auto-merge (squash) June 1, 2026 19:54
@lwshang lwshang merged commit de2b245 into main Jun 1, 2026
11 checks passed
@lwshang lwshang deleted the lwshang/drop_assets_toml branch June 1, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants