-
Notifications
You must be signed in to change notification settings - Fork 4k
StreamlitMarkdown - module level plugin cache
#13152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
📈 Frontend coverage change detectedThe frontend unit test (vitest) coverage has increased by 0.0700%
🎉 Great job on improving test coverage! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the lazy loading mechanism for markdown plugins in StreamlitMarkdown to use module-level caching instead of component-level state. The changes improve React StrictMode compatibility, reduce redundant plugin loads across component instances, and consolidate duplicate loading logic into a reusable hook.
Key Changes
- Module-level plugin caching: Plugins are now cached at module scope and shared across all component instances, preventing redundant loads
- Custom
useLazyPluginhook: Consolidates three nearly-identicaluseEffectblocks into a single reusable hook with proper cleanup and shared in-flight promise handling - Extraction to
utils.ts: Moves all plugin-related logic (types, loaders, cache, helpers, hook) to a dedicated utilities file with comprehensive test coverage
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
frontend/lib/src/components/shared/StreamlitMarkdown/utils.ts |
New file containing plugin loading infrastructure: module-level caches, lazy loaders for katex/raw/emoji plugins, plugin extraction logic, error-handling wrappers, and the useLazyPlugin hook |
frontend/lib/src/components/shared/StreamlitMarkdown/utils.test.ts |
Comprehensive test suite for the new utilities covering plugin extraction, wrapper functions, lazy loading hook behavior, cleanup, and error scenarios |
frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.tsx |
Refactored to use useLazyPlugin hook instead of three separate useEffect blocks, with wrapped plugins for better error handling |
frontend/lib/src/components/shared/StreamlitMarkdown/StreamlitMarkdown.tsx
Outdated
Show resolved
Hide resolved
frontend/lib/src/components/shared/StreamlitMarkdown/utils.test.ts
Outdated
Show resolved
Hide resolved
frontend/lib/src/components/shared/StreamlitMarkdown/utils.test.ts
Outdated
Show resolved
Hide resolved
5014008 to
1ae1a8c
Compare
lukasmasuch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Describe your changes
React's
StrictModeexposed some opportunities to improve the recently added lazy loading forStreamlitMarkdown's plugins:rehype-katex,rehype-raw, &remark-emojiComponent is now StrictMode compatible, with improved performance (plugins load once per app, not per component instance), and better handle plugin load failure (render as plain text).
This PR better makes the following changes:
useLazyPluginhook: Consolidated three nearly-identical useEffect blocks into a single reusable hook with proper cleanup handlingutils.ts: Moved all plugin-related logic (types, loaders, cache, helpers, hook) to a dedicated fileTesting Plan