Improve marked js file discovery in BitMarkdownService (#11318) #11320
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a FrameworkReference to Microsoft.AspNetCore.App and reorganizes PrivateAssets for dependencies. Refactors BitMarkdownService to use IServiceProvider and IWebHostEnvironment’s WebRootFileProvider for marked.js resolution, updates script path to _content, renames fields/methods, and adjusts client/server parsing flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as Component
participant Svc as BitMarkdownService
participant Env as IWebHostEnvironment
participant FS as WebRootFileProvider (IFileProvider)
participant JS as IJSRuntime
participant Jint as Jint Engine
UI->>Svc: Parse(markdown)
alt Server-side parsing
Svc->>Env: Resolve via IServiceProvider
Env->>FS: GetFileInfo("_content/.../marked-15.0.7.js")
FS-->>Svc: FileInfo(PhysicalPath)
Svc->>Svc: ReadMarkedScriptContent (semaphore)
Svc->>Jint: RunJint(markdown, scriptContent)
Jint-->>Svc: html
Svc-->>UI: html
else Client-side parsing
Svc->>JS: Ensure marked loaded from "_content/.../marked-15.0.7.js"
JS-->>Svc: ready
Svc->>JS: marked.parse(markdown)
JS-->>Svc: html
Svc-->>UI: html
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/BlazorUI/Bit.BlazorUI.Extras/Bit.BlazorUI.Extras.csproj (1)
24-28: Use of FrameworkReference looks fine; consider a couple of refinements.
- PrivateAssets on FrameworkReference is harmless but redundant; FrameworkReferences aren’t transitive. Consider dropping it to reduce noise.
- If the sole motivation is IWebHostEnvironment (server-only), this reference is appropriate. Otherwise, evaluate whether a narrower package reference (e.g., Hosting.Abstractions) would suffice.
src/BlazorUI/Bit.BlazorUI.Extras/Services/BitMarkdownService.cs (4)
12-12: Avoid hardcoding the marked.js version in code.Consider centralizing the version (e.g., MSBuild property or generated constants) so bumps don’t require code edits. Alternatively, expose a single stable path (e.g., marked/marked.js) and map/version it at build time.
86-86: Prefer GetService with null-check to avoid hard failures in atypical hosts.Using GetRequiredService will throw if IWebHostEnvironment isn’t registered (e.g., tests, non-HTTP hosts). Since you resolve it only on server path, a soft failure is nicer.
Apply this diff:
- var env = serviceProvider.GetRequiredService<Microsoft.AspNetCore.Hosting.IWebHostEnvironment>(); + var env = serviceProvider.GetService<Microsoft.AspNetCore.Hosting.IWebHostEnvironment>(); + if (env is null) + { + Console.Error.WriteLine("IWebHostEnvironment is not available in this host."); + return _markedScriptContent = string.Empty; + }
31-31: Minor: remove redundant async/await wrapper in Task.Run.No need for an async lambda here.
Apply this diff:
- html = await Task.Run(async () => await RunJint(markdown, cancellationToken), cancellationToken); + html = await Task.Run(() => RunJint(markdown, cancellationToken), cancellationToken);
35-36: Use ILogger instead of Console.Error for diagnostics.Library consumers can route logs appropriately; Console writes are easy to miss.
If you want, I can wire ILogger and update these call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (2)
src/BlazorUI/Bit.BlazorUI.Extras/Bit.BlazorUI.Extras.csproj(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Services/BitMarkdownService.cs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build and test
msynk
deleted the
11318-blazorui-markdownservice-maked-file-discovery-improvements
branch
1 participant
closes #11318
Summary by CodeRabbit