Skip to content

add sitemap slash configuration#6291

Merged
adhami3310 merged 2 commits intomainfrom
add-sitemap-slash-configuration
Apr 7, 2026
Merged

add sitemap slash configuration#6291
adhami3310 merged 2 commits intomainfrom
add-sitemap-slash-configuration

Conversation

@adhami3310
Copy link
Copy Markdown
Member

No description provided.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 6, 2026

Merging this PR will not alter performance

✅ 8 untouched benchmarks


Comparing add-sitemap-slash-configuration (3c7048b) with main (1110287)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 6, 2026

Greptile Summary

This PR adds a configurable trailing_slash option ("always", "never", "preserve") to the SitemapPlugin, defaulting to "preserve" for full backwards compatibility. The option is threaded through all URL-construction paths and SitemapPlugin is converted to a frozen dataclass to cleanly hold the new field.

  • New TrailingSlashOption type (Literal["always", "never", "preserve"]) controls whether trailing slashes are appended, stripped, or left untouched in generated sitemap URLs
  • SitemapPlugin is now a @dataclass (frozen=True, slots=True) with trailing_slash defaulting to "preserve"
  • All three code paths in generate_links_for_sitemap (dynamic/404 routes, explicit loc override, and route-derived URLs) correctly receive and apply the option
  • Three new unit tests cover all three modes, but the combination of deploy_url=None + index route + trailing_slash="never" is not tested and produces an empty-string loc
  • slots=True on the dataclass has no practical effect because the base class Plugin does not define __slots__, so the memory optimization is silently negated

Confidence Score: 5/5

Safe to merge; both findings are minor P2 suggestions that do not affect the primary user path

The core trailing-slash logic is correct and well-tested for all common cases. The two issues — an empty loc for an obscure edge case (no deploy_url + index + never) and a no-op slots=True — are both P2 and non-blocking.

packages/reflex-base/src/reflex_base/plugins/sitemap.py — the empty-loc edge case at lines 76-77 is worth a follow-up fix

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/plugins/sitemap.py Adds trailing_slash option to sitemap URL generation; edge case: index route with no deploy_url and 'never' mode produces empty loc string
tests/units/plugins/test_sitemap.py Updates existing tests to pass trailing_slash and adds three new tests for all three modes; missing coverage for no-deploy-url + index + 'never' combination

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pre_compile called] --> B[get unevaluated_pages from context]
    B --> C[add_save_task with sitemap_task + trailing_slash]
    C --> D[sitemap_task runs]
    D --> E[generate_links_for_sitemap]
    E --> F{For each page}
    F --> G{Dynamic route or 404?}
    G -->|Yes, no loc| H[Skip / warn]
    G -->|Yes, has loc| I[configuration_with_loc]
    G -->|No, has loc override| I
    G -->|No, use route| J[derive loc from route]
    J --> I
    I --> K{deploy_url set?}
    K -->|Yes, relative URL| L[prepend deploy_url]
    K -->|No or absolute URL| M[use as-is]
    L --> N{trailing_slash}
    M --> N
    N -->|always| O[append slash if missing]
    N -->|never| P[rstrip slash]
    N -->|preserve| Q[no change]
    O --> R[SitemapLink dict]
    P --> R
    Q --> R
    R --> S[generate_xml]
    S --> T[sitemap.xml]
Loading

Reviews (1): Last reviewed commit: "add sitemap slash configuration" | Re-trigger Greptile

@adhami3310 adhami3310 merged commit 46ef879 into main Apr 7, 2026
55 of 56 checks passed
@adhami3310 adhami3310 deleted the add-sitemap-slash-configuration branch April 7, 2026 16:57
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