diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 72bd4ec1..36a4e36f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,6 +8,9 @@ * Fix `Markdown.ToMd` serialising italic spans with asterisks incorrectly as bold spans. [#1102](https://github.com/fsprojects/FSharp.Formatting/pull/1102) * Fix `Markdown.ToMd` serialising ordered list items with incorrect numbering and formatting. [#1102](https://github.com/fsprojects/FSharp.Formatting/pull/1102) +### Changed +* `fsdocs build` now pre-computes the navigation menu structure (filter/group/sort) once per build rather than once per output page, reducing work from O(n²) to O(n) for sites with n pages. The filesystem check for custom menu templates is also cached per build. [#1129](https://github.com/fsprojects/FSharp.Formatting/pull/1129) + ## [22.0.0-alpha.2] - 2026-03-13 ### Added diff --git a/src/fsdocs-tool/BuildCommand.fs b/src/fsdocs-tool/BuildCommand.fs index b16f5730..7c7127db 100644 --- a/src/fsdocs-tool/BuildCommand.fs +++ b/src/fsdocs-tool/BuildCommand.fs @@ -705,114 +705,122 @@ type internal DocContent ``type`` = "content" } | _ -> () |] - member _.GetNavigationEntries - ( - input, - docModels: (string * bool * LiterateDocModel) list, - currentPagePath: string option, - ignoreUncategorized: bool - ) = - let modelsForList = - [ for thing in docModels do - match thing with - | (inputFileFullPath, isOtherLang, model) when + /// Pre-computes the expensive navigation structure (filter/group/sort) once, returning a + /// cheap render function that generates nav HTML for any given current page path. + /// This avoids O(n²) work when building a site with n pages, since the structure + /// (grouping, sorting, templating check) is the same for every page. + member _.GetNavigationEntriesFactory + (input, docModels: (string * bool * LiterateDocModel) list, ignoreUncategorized: bool) + : string option -> string = + + // Pre-compute: filter eligible models, keeping paths for active-page detection + let baseModels = + [ for (inputFileFullPath, isOtherLang, model) in docModels do + if not isOtherLang && model.OutputKind = OutputKind.Html - && (Path.GetFileNameWithoutExtension(inputFileFullPath) <> "index") - -> - { model with - IsActive = - match currentPagePath with - | None -> false - | Some currentPagePath -> currentPagePath = inputFileFullPath } - | _ -> () ] - - let excludeUncategorized = + && Path.GetFileNameWithoutExtension(inputFileFullPath) <> "index" + then + yield (inputFileFullPath, model) ] + + let filteredBase = if ignoreUncategorized then - List.filter (fun (model: LiterateDocModel) -> model.Category.IsSome) + baseModels |> List.filter (fun (_, model) -> model.Category.IsSome) else - id - - let modelsByCategory = - modelsForList - |> excludeUncategorized - |> List.groupBy (fun (model) -> model.Category) - |> List.sortBy (fun (_, ms) -> - match ms.[0].CategoryIndex with + baseModels + + // Pre-sort items within each category (independent of active page) + let orderGroup items = + items + |> List.sortBy (fun (_, model: LiterateDocModel) -> Option.defaultValue Int32.MaxValue model.Index) + + // Pre-compute: group by category, sort categories, sort items within each group + let sortedGroups = + filteredBase + |> List.groupBy (fun (_, model) -> model.Category) + |> List.sortBy (fun (_, items) -> + match (snd items.[0]).CategoryIndex with | Some s -> (try int32 s with _ -> Int32.MaxValue) | None -> Int32.MaxValue) - - let orderList (list: (LiterateDocModel) list) = - list - |> List.sortBy (fun model -> Option.defaultValue Int32.MaxValue model.Index) - - if Menu.isTemplatingAvailable input then - let createGroup (isCategoryActive: bool) (header: string) (items: LiterateDocModel list) : string = - //convert items into menuitem list - let menuItems = - orderList items - |> List.map (fun (model: LiterateDocModel) -> - let link = model.Uri(root) - let title = System.Web.HttpUtility.HtmlEncode model.Title - - { Menu.MenuItem.Link = link - Menu.MenuItem.Content = title - Menu.MenuItem.IsActive = model.IsActive }) - - Menu.createMenu input isCategoryActive header menuItems - // No categories specified - if modelsByCategory.Length = 1 && (fst modelsByCategory.[0]) = None then - let _, items = modelsByCategory.[0] - createGroup false "Documentation" items + |> List.map (fun (cat, items) -> cat, orderGroup items) + + // Cache filesystem check — same result for all pages in a build + let useTemplating = Menu.isTemplatingAvailable input + + // Cheap render function: only sets IsActive and generates HTML (no sorting/grouping) + fun (currentPagePath: string option) -> + let modelsByCategory = + sortedGroups + |> List.map (fun (cat, items) -> + cat, + items + |> List.map (fun (path, model) -> + { model with + IsActive = + match currentPagePath with + | None -> false + | Some cp -> cp = path })) + + if useTemplating then + let createGroup (isCategoryActive: bool) (header: string) (items: LiterateDocModel list) : string = + let menuItems = + items + |> List.map (fun (model: LiterateDocModel) -> + let link = model.Uri(root) + let title = System.Web.HttpUtility.HtmlEncode model.Title + + { Menu.MenuItem.Link = link + Menu.MenuItem.Content = title + Menu.MenuItem.IsActive = model.IsActive }) + + Menu.createMenu input isCategoryActive header menuItems + + if modelsByCategory.Length = 1 && (fst modelsByCategory.[0]) = None then + let _, items = modelsByCategory.[0] + createGroup false "Documentation" items + else + modelsByCategory + |> List.map (fun (header, items) -> + let header = Option.defaultValue "Other" header + let isActive = items |> List.exists (fun m -> m.IsActive) + createGroup isActive header items) + |> String.concat "\n" else - modelsByCategory - |> List.map (fun (header, items) -> - let header = Option.defaultValue "Other" header - let isActive = items |> List.exists (fun m -> m.IsActive) - createGroup isActive header items) - |> String.concat "\n" - else - [ - // No categories specified - if modelsByCategory.Length = 1 && (fst modelsByCategory.[0]) = None then - li [ Class "nav-header" ] [ !!"Documentation" ] - - for model in snd modelsByCategory.[0] do - let link = model.Uri(root) - let activeClass = if model.IsActive then "active" else "" - - li - [ Class $"nav-item %s{activeClass}" ] - [ a [ Class "nav-link"; (Href link) ] [ encode model.Title ] ] - else - // At least one category has been specified. Sort each category by index and emit - // Use 'Other' as a header for uncategorised things - for (cat, modelsInCategory) in modelsByCategory do - let modelsInCategory = orderList modelsInCategory - - let categoryActiveClass = - if modelsInCategory |> List.exists (fun m -> m.IsActive) then - "active" - else - "" - - match cat with - | Some c -> li [ Class $"nav-header %s{categoryActiveClass}" ] [ !!c ] - | None -> li [ Class $"nav-header %s{categoryActiveClass}" ] [ !!"Other" ] + [ if modelsByCategory.Length = 1 && (fst modelsByCategory.[0]) = None then + li [ Class "nav-header" ] [ !!"Documentation" ] - for model in modelsInCategory do + for model in snd modelsByCategory.[0] do let link = model.Uri(root) let activeClass = if model.IsActive then "active" else "" li [ Class $"nav-item %s{activeClass}" ] - [ a [ Class "nav-link"; (Href link) ] [ encode model.Title ] ] ] - |> List.map (fun html -> html.ToString()) - |> String.concat " \n" + [ a [ Class "nav-link"; (Href link) ] [ encode model.Title ] ] + else + for (cat, modelsInCategory) in modelsByCategory do + let categoryActiveClass = + if modelsInCategory |> List.exists (fun m -> m.IsActive) then + "active" + else + "" + + match cat with + | Some c -> li [ Class $"nav-header %s{categoryActiveClass}" ] [ !!c ] + | None -> li [ Class $"nav-header %s{categoryActiveClass}" ] [ !!"Other" ] + + for model in modelsInCategory do + let link = model.Uri(root) + let activeClass = if model.IsActive then "active" else "" + + li + [ Class $"nav-item %s{activeClass}" ] + [ a [ Class "nav-link"; (Href link) ] [ encode model.Title ] ] ] + |> List.map (fun html -> html.ToString()) + |> String.concat " \n" /// Processes and runs Suave server to host them on localhost module Serve = @@ -2027,14 +2035,17 @@ type CoreBuildOptions(watch) = let actualDocModels = docModels |> List.map fst |> List.choose id let extrasForSearchIndex = docContent.GetSearchIndexEntries(actualDocModels) - let navEntriesWithoutActivePage = - docContent.GetNavigationEntries( + // Pre-compute the navigation structure once; returned closure cheaply + // generates per-page nav HTML by only re-applying active-page flags. + let getNavEntries = + docContent.GetNavigationEntriesFactory( this.input, actualDocModels, - None, ignoreUncategorized = this.ignoreuncategorized ) + let navEntriesWithoutActivePage = getNavEntries None + let headTemplateContent = let headTemplatePath = Path.Combine(this.input, "_head.html") @@ -2078,14 +2089,8 @@ type CoreBuildOptions(watch) = match optDocModel with | None -> globals | Some(currentPagePath, _, _) -> - // Update the nav entries with the current page doc model - let navEntries = - docContent.GetNavigationEntries( - this.input, - actualDocModels, - Some currentPagePath, - ignoreUncategorized = this.ignoreuncategorized - ) + // Use the pre-computed factory closure (only sets IsActive, no re-sorting) + let navEntries = getNavEntries (Some currentPagePath) globals |> List.map (fun (pk, v) -> diff --git a/tests/FSharp.Literate.Tests/DocContentTests.fs b/tests/FSharp.Literate.Tests/DocContentTests.fs index 5a4ef7e9..5f706abd 100644 --- a/tests/FSharp.Literate.Tests/DocContentTests.fs +++ b/tests/FSharp.Literate.Tests/DocContentTests.fs @@ -338,6 +338,244 @@ let ``ipynb notebook evaluates`` () = ipynbOut |> shouldContainText "10007" *) +// -------------------------------------------------------------------------------------- +// Tests for GetNavigationEntriesFactory (pre-computed navigation structure, PR #1129) +// -------------------------------------------------------------------------------------- + +/// Build a minimal LiterateDocModel entry for navigation tests. +let private makeNavDocModel + (title: string) + (inputPath: string) + (category: string option) + (categoryIndex: int option) + (index: int option) + = + (inputPath, + false, + { Title = title + Substitutions = [] + IndexText = None + Category = category + CategoryIndex = categoryIndex + Index = index + OutputPath = Path.GetFileNameWithoutExtension(inputPath) + ".html" + OutputKind = OutputKind.Html + IsActive = false }) + +/// DocContent instance whose root URL is empty string (URIs = bare output paths). +let private makeDocContentForNav () = + DocContent( + Path.GetTempPath(), + Map.empty, + lineNumbers = None, + evaluate = false, + substitutions = [], + saveImages = None, + watch = false, + root = "", + crefResolver = (fun _ -> None), + onError = failwith + ) + +// Use the test source directory as the `input` folder; it contains no +// _menu_template.html / _menu-item_template.html files, so Menu.isTemplatingAvailable +// returns false and we exercise the built-in HTML generation path. +let private navInput = __SOURCE_DIRECTORY__ + +[] +let ``GetNavigationEntriesFactory - empty docModels produces empty string`` () = + let dc = makeDocContentForNav () + let factory = dc.GetNavigationEntriesFactory(navInput, [], ignoreUncategorized = false) + factory None |> shouldEqual "" + +[] +let ``GetNavigationEntriesFactory - single uncategorized model renders Documentation header`` () = + let dc = makeDocContentForNav () + let models = [ makeNavDocModel "Getting Started" "/docs/getting-started.md" None None None ] + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory None + html |> shouldContainText "Documentation" + html |> shouldContainText "getting-started.html" + +[] +let ``GetNavigationEntriesFactory - None currentPagePath marks no item as active`` () = + let dc = makeDocContentForNav () + let models = [ makeNavDocModel "Page 1" "/docs/page1.md" None None None ] + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + factory None |> shouldNotContainText "active" + +[] +let ``GetNavigationEntriesFactory - matching currentPagePath marks item as active`` () = + let dc = makeDocContentForNav () + let models = [ makeNavDocModel "Page 1" "/docs/page1.md" None None None ] + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + factory (Some "/docs/page1.md") |> shouldContainText "active" + +[] +let ``GetNavigationEntriesFactory - non-matching currentPagePath does not mark item as active`` () = + let dc = makeDocContentForNav () + let models = [ makeNavDocModel "Page 1" "/docs/page1.md" None None None ] + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + factory (Some "/docs/other.md") |> shouldNotContainText "active" + +[] +let ``GetNavigationEntriesFactory - exactly one item is active among multiple pages`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Page 1" "/docs/page1.md" None None None + makeNavDocModel "Page 2" "/docs/page2.md" None None None + makeNavDocModel "Page 3" "/docs/page3.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory (Some "/docs/page2.md") + // "nav-item active" should appear exactly once + let activeCount = html.Split([| "nav-item active" |], System.StringSplitOptions.None).Length - 1 + + activeCount |> shouldEqual 1 + +[] +let ``GetNavigationEntriesFactory - excludes isOtherLang models`` () = + let dc = makeDocContentForNav () + + let otherLangModel = + ("/docs/other-lang.md", + true, // isOtherLang = true + { Title = "Other Language Page" + Substitutions = [] + IndexText = None + Category = None + CategoryIndex = None + Index = None + OutputPath = "other-lang.html" + OutputKind = OutputKind.Html + IsActive = false }) + + let factory = dc.GetNavigationEntriesFactory(navInput, [ otherLangModel ], ignoreUncategorized = false) + factory None |> shouldNotContainText "Other Language Page" + +[] +let ``GetNavigationEntriesFactory - excludes non-HTML output models`` () = + let dc = makeDocContentForNav () + + let latexModel = + ("/docs/report.md", + false, + { Title = "LaTeX Report" + Substitutions = [] + IndexText = None + Category = None + CategoryIndex = None + Index = None + OutputPath = "report.tex" + OutputKind = OutputKind.Latex + IsActive = false }) + + let factory = dc.GetNavigationEntriesFactory(navInput, [ latexModel ], ignoreUncategorized = false) + factory None |> shouldNotContainText "LaTeX Report" + +[] +let ``GetNavigationEntriesFactory - excludes files named index`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Home" "/docs/index.md" None None None + makeNavDocModel "Guide" "/docs/guide.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory None + html |> shouldNotContainText "Home" + html |> shouldContainText "Guide" + +[] +let ``GetNavigationEntriesFactory - ignoreUncategorized true excludes uncategorized models`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Categorized Doc" "/docs/cat.md" (Some "Tutorials") None None + makeNavDocModel "Uncategorized Doc" "/docs/uncat.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = true) + let html = factory None + html |> shouldContainText "Categorized Doc" + html |> shouldNotContainText "Uncategorized Doc" + +[] +let ``GetNavigationEntriesFactory - ignoreUncategorized false includes all models`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Categorized Doc" "/docs/cat.md" (Some "Tutorials") None None + makeNavDocModel "Uncategorized Doc" "/docs/uncat.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory None + html |> shouldContainText "Categorized Doc" + html |> shouldContainText "Uncategorized Doc" + +[] +let ``GetNavigationEntriesFactory - categories are ordered by CategoryIndex`` () = + let dc = makeDocContentForNav () + + // Beta has CategoryIndex 2, Alpha has CategoryIndex 1 → Alpha should appear first + let models = + [ makeNavDocModel "Beta Doc" "/docs/b.md" (Some "Beta") (Some 2) None + makeNavDocModel "Alpha Doc" "/docs/a.md" (Some "Alpha") (Some 1) None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory None + let alphaIdx = html.IndexOf("Alpha", System.StringComparison.Ordinal) + let betaIdx = html.IndexOf("Beta", System.StringComparison.Ordinal) + Assert.That(alphaIdx, Is.LessThan(betaIdx)) + +[] +let ``GetNavigationEntriesFactory - items within a category are ordered by Index`` () = + let dc = makeDocContentForNav () + + // "Second" has Index 2, "First" has Index 1 → "First" should appear first + let models = + [ makeNavDocModel "Second Item" "/docs/second.md" (Some "Guides") None (Some 2) + makeNavDocModel "First Item" "/docs/first.md" (Some "Guides") None (Some 1) ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let html = factory None + let firstIdx = html.IndexOf("First Item", System.StringComparison.Ordinal) + let secondIdx = html.IndexOf("Second Item", System.StringComparison.Ordinal) + Assert.That(firstIdx, Is.LessThan(secondIdx)) + +[] +let ``GetNavigationEntriesFactory - calling factory multiple times returns identical results`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Page 1" "/docs/page1.md" None None None + makeNavDocModel "Page 2" "/docs/page2.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + factory None |> shouldEqual (factory None) + +[] +let ``GetNavigationEntriesFactory - successive calls with different page paths set correct active state`` () = + let dc = makeDocContentForNav () + + let models = + [ makeNavDocModel "Page 1" "/docs/page1.md" None None None + makeNavDocModel "Page 2" "/docs/page2.md" None None None ] + + let factory = dc.GetNavigationEntriesFactory(navInput, models, ignoreUncategorized = false) + let htmlPage1 = factory (Some "/docs/page1.md") + let htmlPage2 = factory (Some "/docs/page2.md") + + // The two views differ + htmlPage1 |> shouldNotEqual htmlPage2 + + // Each view has exactly one active item + let countActive (html: string) = + html.Split([| "nav-item active" |], System.StringSplitOptions.None).Length - 1 + + countActive htmlPage1 |> shouldEqual 1 + countActive htmlPage2 |> shouldEqual 1 + // -------------------------------------------------------------------------------------- // Tests for LlmsTxt module (FsDocsGenerateLlmsTxt MSBuild property, on by default) // --------------------------------------------------------------------------------------