Harden root-served localization deploy routing#230
Harden root-served localization deploy routing#230PrzemyslawKlys wants to merge 2 commits intomainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
PR Review: Harden root-served localization deploy routing. Overall this is a well-structured feature addition with good test coverage of the main scenarios. A few issues worth addressing before merge. Bug: Regex in RewriteQuotedHtmlAttribute - the character class excludes both double-quote and single-quote from the value match regardless of which quote type opened the attribute; an href containing an apostrophe would be truncated. Dead code in IsLocalizedFallbackCopy - the final null check is unreachable. Performance: ResolveLocalizationConfig called per-attribute per-page - consider resolving once per render pass. Redundant null check in ResolvePublicRouteForLanguage. RootPath in ThemeRenderContext appears unused in this diff. Unrelated changes bundled in the PR: BuildCrawlMetaHtml refactor and the EmitXDefault test. Missing test coverage for collection-level filtering, RenderAtRoot=false regression, and single-quoted HTML attributes. Minor nit: duplicate StartsWith guard in RebaseRouteForSelectedLanguageRootBuild. |
|
Follow-up pushed in |
PR Review: Harden root-served localization deploy routingGood overall direction. The Issues1. Regex compiled on every call (
2. Redundant double-lookup in The method calls 3. Previously a single resolved alternate suppressed all hreflang output. Now it emits both a language tag and x-default. The new test 4.
5. The schema diff adds 6. The new code joins Minor / Nit
SummaryThe core routing logic is sound and the new tests cover the main scenarios well. The primary items to address: regex performance (matters for throughput on large sites), the |
Summary
renderAtRootsupport for root-served language deploymentslanguageAsRootbuildsTesting
Build_LocalizedPages_SupportSingleLanguageDomainStyleBuild|FullyQualifiedNameBuild_LocalizedPages_RebaseInternalLinks_ForRootLanguageBuild|FullyQualifiedNameBuild_LocalizedPages_EmitAlternateHeadLinks_ForRootServedLanguage|FullyQualifiedNameSitemap_Generate_EmitsLocalizedAlternates_WithRootServedLanguageBaseUrls"