[dev-v5][Docs] Extend the DocApiGen and DocViewer projects#4719
[dev-v5][Docs] Extend the DocApiGen and DocViewer projects#4719
Conversation
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 99.2%
|
There was a problem hiding this comment.
Pull request overview
This PR extends the DocApiGen/DocViewer tooling to support generating and viewing API docs from multiple assemblies/XML inputs, refactoring the generator pipeline around DocumentationInput and a unified factory entrypoint.
Changes:
- Add multi-input documentation generation via
DocumentationInput+DocumentationGeneratorFactory.Create(mode, inputs)and an isolatedAssemblyLoadContext. - Update DocViewer (options/service + MarkdownViewer) to resolve API types across multiple assemblies.
- Update test infrastructure and add a multi-input integration test (core + charts) and supporting path-resolution helpers.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/Tools/FluentUI.Demo.DocViewer/Services/DocViewerService.cs | Switch API source to ApiAssemblies, add FindApiType across multiple assemblies. |
| examples/Tools/FluentUI.Demo.DocViewer/Services/DocViewerOptions.cs | Replace single ApiAssembly with ApiAssemblies (keep obsolete shim). |
| examples/Tools/FluentUI.Demo.DocViewer/Components/MarkdownViewer.razor.cs | Use DocViewerService.FindApiType instead of single-assembly scanning. |
| examples/Tools/FluentUI.Demo.DocApiGen/Program.cs | Accept multiple --dll/--xml inputs and create generators from input lists + isolated ALC. |
| examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClassOptions.cs | Switch from DocXmlReader to IDocumentationCommentProvider. |
| examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs | Read summaries via the new comment provider abstraction. |
| examples/Tools/FluentUI.Demo.DocApiGen/Models/DocumentationInput.cs | New: pairs an Assembly with its XML doc file + reader. |
| examples/Tools/FluentUI.Demo.DocApiGen/Models/DocumentationCommentProvider.cs | New: resolve member/type summaries across multiple inputs. |
| examples/Tools/FluentUI.Demo.DocApiGen/DocumentationAssemblyLoadContext.cs | New: isolated load context for doc target assemblies/dependencies. |
| examples/Tools/FluentUI.Demo.DocApiGen/Abstractions/IDocumentationCommentProvider.cs | New: abstraction for summary lookup. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/SummaryDocumentationGenerator.cs | Generate from multiple inputs; adds partial type-load handling. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/AllDocumentationGenerator.cs | Update to multi-input scanning and multi-input comment lookup. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/McpDocumentationGenerator.cs | Update to multi-input scanning and multi-input comment lookup. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/IconsEmojisGenerator.cs | Update constructor/base usage to accept inputs list. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/DocumentationGeneratorFactory.cs | Add new Create(mode, inputs) overload; single-input overload now wraps into inputs. |
| examples/Tools/FluentUI.Demo.DocApiGen/Generators/DocumentationGeneratorBase.cs | Base class now stores Inputs and PrimaryAssembly. |
| examples/Tools/FluentUI.Demo.DocApiGen.Tests/Models/SummaryMode/ApiClassPerformanceTests.cs | Update tests to build ApiClassOptions with comment provider. |
| examples/Tools/FluentUI.Demo.DocApiGen.Tests/Models/IconEmoji/IconEmojiTests.cs | Update tests to use input list for generator construction. |
| examples/Tools/FluentUI.Demo.DocApiGen.IntegrationTests/FluentUIComponentsIntegrationTests.cs | Refactor integration setup to inputs/ALC; add multi-input (core+charts) test + helper. |
| examples/Demo/FluentUI.Demo.Client/Infrastructure/ServiceCollectionExtensions.cs | Register DocViewerOptions.ApiAssemblies (array) instead of deprecated ApiAssembly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| Environment.Exit(1); | ||
| //Environment.Exit(1); |
There was a problem hiding this comment.
The exception handler no longer exits with a non-zero process code (Environment.Exit(1) is commented out). For a CLI tool, this makes failures look like success to scripts/CI, and may cause downstream steps to run with invalid output. Restore a non-zero exit code (or rethrow) after logging.
| //Environment.Exit(1); | |
| Environment.Exit(1); |
| var allTypes = Inputs.SelectMany(input => input.Assembly.GetTypes()).ToList(); | ||
| var validTypes = allTypes.Where(IsValidComponentType).ToList(); | ||
| var enumTypes = allTypes.Where(t => t.IsEnum && t.IsPublic).ToList(); |
There was a problem hiding this comment.
Inputs.SelectMany(input => input.Assembly.GetTypes()) can throw ReflectionTypeLoadException (e.g., if any dependency is missing). Summary mode already guards this and continues with ex.Types. To avoid the All generator crashing on partially loadable assemblies, wrap GetTypes() with similar handling (or factor out a shared safe GetTypes helper).
| var allTypes = Inputs | ||
| .SelectMany(input => input.Assembly.GetTypes()) | ||
| .Where(t => t.IsClass && t.IsPublic && !t.IsAbstract) | ||
| .ToList(); |
There was a problem hiding this comment.
Inputs.SelectMany(input => input.Assembly.GetTypes()) can throw ReflectionTypeLoadException when scanning types (missing dependencies, version mismatches, etc.), which would abort MCP generation entirely. Consider using the same safe GetTypes() pattern used in SummaryDocumentationGenerator to continue with the loadable types and optionally log loader exceptions.
| if (!File.Exists(chartsAssemblyPath) || !File.Exists(chartsXmlPath)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This new integration test currently becomes a no-op in this repo layout: the referenced src/Charts/.../Microsoft.FluentUI.AspNetCore.Components.Charts.dll and the Microsoft.FluentUI.AspNetCore.Components.Charts.xml file don’t exist, so the test hits return; and always passes without asserting multi-input behavior. Consider pointing the test at an assembly/docs pair that is present in-repo (or making missing prerequisites fail/explicitly skip) so CI actually exercises the multi-assembly path.
| if (!File.Exists(chartsAssemblyPath) || !File.Exists(chartsXmlPath)) | |
| { | |
| return; | |
| } | |
| Assert.True( | |
| File.Exists(chartsAssemblyPath) && File.Exists(chartsXmlPath), | |
| $"Multi-input integration test prerequisites are missing. Expected assembly: '{chartsAssemblyPath}' (exists: {File.Exists(chartsAssemblyPath)}), expected XML docs: '{chartsXmlPath}' (exists: {File.Exists(chartsXmlPath)})."); |
| } | ||
|
|
||
| _fluentUIAssembly = Assembly.LoadFrom(fluentUIAssemblyPath); | ||
| throw new FileNotFoundException($"Assembly not found. Please build the project first. Looked for: {releasePath}"); |
There was a problem hiding this comment.
GetAssemblyPath checks both Debug and Release locations, but the thrown FileNotFoundException message only reports the Release path. This makes failures harder to diagnose because the Debug path was also tried. Consider including both attempted paths in the exception message.
| throw new FileNotFoundException($"Assembly not found. Please build the project first. Looked for: {releasePath}"); | |
| throw new FileNotFoundException( | |
| $"Assembly not found. Please build the project first. Looked for: {debugPath}; {releasePath}"); |
|
|
||
| using System.Reflection; | ||
| using FluentUI.Demo.DocApiGen.Abstractions; | ||
| using FluentUI.Demo.DocApiGen.Extensions; |
There was a problem hiding this comment.
DocumentationCommentProvider has an unused using FluentUI.Demo.DocApiGen.Extensions;. With EnforceCodeStyleInBuild + TreatWarningsAsErrors, this can fail the build due to IDE0005/CS8019. Remove the unnecessary using directive.
| using FluentUI.Demo.DocApiGen.Extensions; |
| return ApiAssemblies | ||
| .SelectMany(GetTypes) | ||
| .FirstOrDefault(type => string.Equals(type.FullName, fullName, StringComparison.Ordinal) | ||
| || string.Equals(type.Name, fullName, StringComparison.Ordinal)); |
There was a problem hiding this comment.
FindApiType no longer matches generic component types when the markdown uses the unadorned name (e.g., MyComponent<T> becomes MyComponent). The previous implementation matched MyComponent against MyComponent1/MyComponent2, but the new predicate only checks exact type.Name, so generic components won’t resolve and API sections may disappear. Consider extending the lookup to also match type.Name that starts with `${fullName}`` (or explicitly check arities 1..N).
| || string.Equals(type.Name, fullName, StringComparison.Ordinal)); | |
| || string.Equals(type.Name, fullName, StringComparison.Ordinal) | |
| || type.Name.StartsWith(fullName + "`", StringComparison.Ordinal)); |
| Options = options; | ||
| ComponentsAssembly = options.ComponentsAssembly; | ||
| ResourcesAssembly = options.ResourcesAssembly; | ||
| ApiAssembly = options.ApiAssembly; | ||
| ApiAssemblies = options.ApiAssemblies; | ||
| ApiCommentSummary = options.ApiCommentSummary; |
There was a problem hiding this comment.
ApiAssemblies is assigned directly from options.ApiAssemblies, but DocViewerOptions.ApiAssemblies is settable and could be set to null by consumers/config binding. That would cause NullReferenceException in FindApiType/ApiAssembly. Consider normalizing to an empty list in the constructor (or making the option non-nullable/guarding the setter).
| path => Path.GetFileNameWithoutExtension(path), | ||
| StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| #pragma warning disable CA1416 // Validate platform compatibility |
There was a problem hiding this comment.
What will be the result when this part of code will be executed on Linux (during the publication process in ADO) ?
AssemblyDependencyResolver seems not acceptable on Linux (not tested)
| Console.WriteLine(" DocApiGen --xml MyApp.xml --dll MyApp.dll --output api-all.json --mode all"); | ||
| Console.WriteLine(); | ||
| Console.WriteLine(" # Generate Summary mode JSON from multiple assemblies"); | ||
| Console.WriteLine(" DocApiGen --xml Core.xml;Charts.xml --dll Core.dll;Charts.dll --output api-summary.json"); |
There was a problem hiding this comment.
Why not to use another mode --mode chart like for Icons, Emojis or MCP?
That will use the same logic as already used.
And in the ADO Pipelinem we need to add a new task to run the Chart generation. That's all :-)
This pull request refactors the documentation generator to support multiple documentation inputs and assemblies. You can now specify multiple dll and xml sources to generate one combined api-comments.json file.
It replaces the old single-assembly generator creation with a new approach using
DocumentationInputandGenerationMode. Some utility methods and test setup logic are also improved for better robustness. For instance, we now use a specialized (isolated) AssemblyLoadContext so errors about mismatching assembly manifests are now prevented.The PR also updates the
MarkdownViewerto read from multiple assembly documentation sourcesTest Infrastructure Improvements:
Refactored test setup to use
DocumentationInputandGenerationModewith theDocumentationGeneratorFactory.Createmethod, replacing the previous single-assembly generator creation methods. This enables easier extension to multiple assemblies and modes.Added a new test (
AllGenerator_WithMultipleDocumentationInputs_ShouldIncludeChartsAssemblyTypes) that verifies the generator can handle multiple assemblies and documentation files, ensuring types from both the core and charts assemblies are included in the output.Utility and Robustness Improvements:
Centralized assembly path resolution with a new
GetAssemblyPathmethod, supporting both Debug and Release builds, improving reliability when locating assemblies for tests.Updated project root directory detection to look for
.slnxfiles instead of.sln, reflecting the current solution file extension.Demo Service Registration:
AddFluentUIDemoServicesto use theApiAssembliesproperty (now an array) instead of the deprecatedApiAssemblyproperty, aligning with new API expectations.