Skip to content

refactor(css-syntax): drop the hardcoded type slug map - #877

Open
caugner wants to merge 4 commits into
csssyntax-resolve-type-linksfrom
csssyntax-drop-redundant-slug-map
Open

refactor(css-syntax): drop the hardcoded type slug map#877
caugner wants to merge 4 commits into
csssyntax-resolve-type-linksfrom
csssyntax-drop-redundant-slug-map

Conversation

@caugner

@caugner caugner commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

Remove the five-entry hardcoded type slug map from SyntaxRenderer::render_node, along with the now-unreachable !skip(slug) guard in the same branch. Rename skip() to should_skip_expansion() to say what its remaining call sites govern.

Motivation

The map (<color> to color_value, <contrast-color()> to color_value/contrast-color, …) encoded MDN's page layout inside css-syntax, which has no business knowing it. The CSS_FEATURE_INDEX resolver added in #874 derives all five from the page tree via its _value/_function and leaf aliases, so the map now only duplicates that knowledge in the wrong crate.

Additional details

Verified as a no-op: building all 1256 en-US Web/CSS pages before and after produces byte-identical index.json output. (The only diff is element ordering in metadata.json, which is nondeterministic and identical as a set.)

should_skip_expansion() (formerly skip()) itself stays. Its two other call sites govern constituent expansion rather than linking, and because they keep <color> and <gradient> out of get_nodes_for_syntaxes, those types never reach SyntaxRenderer::constituents and the !skip(slug) guard in render_node could never fire.

test_render_terms previously rendered with resolver: None, whose default-category fallback produced links MDN does not have (e.g. Values/contrast-color()). It now resolves through a stub standing in for the page index, so the snapshot shows the nested color_value/… paths for the two function types and leaves undocumented types unlinked.

Related issues and pull requests

Depends on: #874, which adds the resolver that makes the map redundant.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

239094c was deployed to: https://rari-pr877.review.mdn.allizom.net/

@caugner
caugner marked this pull request as ready for review September 4, 2026 09:58
@caugner
caugner requested a review from a team as a code owner September 4, 2026 09:58
@caugner
caugner requested a review from LeoMcA September 4, 2026 09:58
Comment thread crates/css-syntax/src/syntax.rs Outdated
Comment thread crates/css-syntax/src/syntax.rs Outdated
Comment thread crates/css-syntax/src/syntax.rs Outdated
@caugner
caugner marked this pull request as draft September 4, 2026 13:35
@caugner
caugner marked this pull request as ready for review September 4, 2026 13:58
`render_node` special-cased five type names (`<color>` -> `color_value`,
`<contrast-color()>` -> `color_value/contrast-color`, …) to patch up MDN's
page layout. The `CSS_FEATURE_INDEX` resolver now derives all five from the
page tree via its `_value`/`_function` and leaf aliases, so the map only
duplicates that knowledge in the crate that shouldn't own it.

Also drop the `!skip(slug)` guard from the same branch: `skip()` already
keeps `<color>` and `<gradient>` out of `get_nodes_for_syntaxes`, so they
never reach `SyntaxRenderer::constituents` and the guard could not fire.
`skip()` itself stays, since its other two call sites govern constituent
expansion rather than linking.

Verified as a no-op: building all 1256 en-US `Web/CSS` pages before and
after produces byte-identical `index.json` output.

The `test_render_terms` snapshot changes because it renders with
`resolver: None`, which now links the two function types under their bare
names instead of the mapped ones. That path only applies to callers without a
page index.
The name now says what the check governs, so the doc comment
explaining that it is about expansion rather than linking is redundant.
It described a removed special case rather than the current code.
With `resolver: None` the snapshot asserted links that MDN does not have,
such as `Values/device-cmyk()` for a function documented under
`Values/color_value/device-cmyk`. Resolve through a stub that only knows
the pages that exist, so the snapshot shows the nested function paths and
leaves undocumented types unlinked.
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.

3 participants