feat: 集成PageSpy #4820
feat: 集成PageSpy #4820
Conversation
|
WalkthroughThe changes in this pull request introduce a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 5
🧹 Outside diff range and nitpick comments (4)
playground/index.html (2)
30-31: Documentation: Improve comments for SDK configurationThe current comments could be more informative about the setup process and requirements.
Consider enhancing the comments:
- <!-- 可以使用自己的部署的PageSpy的sdk 修改引入的地址即可 --> - <!-- https://pagespy.mufei88.com 为vben测试sdk地址 --> + <!-- PageSpy Integration: + - For production: Deploy your own PageSpy instance and update the URLs below + - Current URLs point to Vben's test environment (not for production use) + - Required scripts: core SDK, data-harbor plugin, and rrweb plugin + - Documentation: https://pagespy.mufei88.com/docs + -->
32-43: Performance: Optimize script loading strategyThe current implementation loads scripts synchronously, which could impact page load performance.
Consider these optimizations:
- Add
deferattribute to all scripts- Use
preloadhints for critical resources- Consider dynamic loading based on user interaction
+ <!-- Preload hints for critical resources --> + <link rel="preload" href="https://pagespy.mufei88.com/page-spy/index.min.js" as="script" crossorigin="anonymous"> <script crossorigin="anonymous" + defer src="https://pagespy.mufei88.com/page-spy/index.min.js" ></script>packages/locales/src/langs/zh-CN/preferences.json (1)
31-33: LGTM! Consider adding more status messages for better UX.The new localization entries for log upload functionality are clear and well-structured. However, consider adding these additional messages for a more complete user experience:
- Upload confirmation message
- Upload error message
- Upload in progress message
{ "logUpload": "上传日志", "logUploadSuccess": "离线日志上传成功", "logUploadSuccessTitle": "上传成功", + "logUploadConfirm": "确认上传离线日志？", + "logUploadError": "日志上传失败", + "logUploadProgress": "正在上传日志..." }packages/locales/src/langs/en-US/preferences.json (1)
31-33: Consider adding error handling messages.To provide a better user experience, consider adding entries for:
- Upload failure messages
- Upload progress status
- Upload confirmation prompts
Add these entries to handle various upload states:
"logUpload": "Upload Log", "logUploadSuccess": "Log Upload Successful", "logUploadSuccessTitle": "Upload Successful", + "logUploadError": "Log Upload Failed", + "logUploadErrorTitle": "Upload Failed", + "logUploadProgress": "Uploading Log...", + "logUploadConfirm": "Are you sure you want to upload the log?"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/@core/base/icons/src/lucide.ts(1 hunks)packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue(4 hunks)packages/locales/src/langs/en-US/preferences.json(1 hunks)packages/locales/src/langs/zh-CN/preferences.json(1 hunks)playground/index.html(1 hunks)
🔇 Additional comments (5)
packages/@core/base/icons/src/lucide.ts (1)
20-20: LGTM! The CloudUpload icon export is properly integrated.
The addition is well-placed in alphabetical order and follows the existing export pattern.
Let's verify the icon usage in the codebase:
✅ Verification successful
CloudUpload icon is properly imported and utilized
The verification confirms that:
- The icon is correctly imported from
@vben/iconsinpreferences-drawer.vue - It's actively used in the template with appropriate sizing and click handler
- The usage aligns with the PR's log upload functionality objective
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify CloudUpload icon import and usage
# Expected: Should find usage in preferences-drawer.vue as mentioned in the summary
# Search for CloudUpload imports
echo "Checking CloudUpload imports:"
rg "CloudUpload.*from.*@vben/icons"
# Search for CloudUpload usage in templates
echo -e "\nChecking CloudUpload usage in vue templates:"
rg "<CloudUpload"
Length of output: 502
packages/locales/src/langs/zh-CN/preferences.json (1)
31-33: Verify corresponding English translations.
Let's ensure the English translations exist and are consistent.
✅ Verification successful
English translations are present and consistent ✅
The corresponding English translations exist in packages/locales/src/langs/en-US/preferences.json with matching keys and appropriate translations:
logUpload: "Upload Log"logUploadSuccess: "Log Upload Successful"logUploadSuccessTitle: "Upload Successful"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the corresponding English translations exist
# Expected: Should find matching keys in the English locale file
rg -A 3 '"logUpload":|"logUploadSuccess":|"logUploadSuccessTitle":' packages/locales/src/langs/en-US/preferences.json
Length of output: 311
packages/locales/src/langs/en-US/preferences.json (1)
31-33: LGTM! The new localization entries follow existing patterns.
The added entries maintain consistency with the file's existing naming conventions and message structure.
packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue (2)
220-220: 🛠️ Refactor suggestion
Add type safety for window.$harbor access
The direct access to window.$harbor lacks type safety. Consider defining a proper type interface for the harbor instance.
+interface Harbor {
+ onOfflineLog: (action: string) => void;
+}
+
+declare global {
+ interface Window {
+ $harbor?: Harbor;
+ }
+}
+
const harbor = computed(() => window.$harbor);Likely invalid or redundant comment.
220-231: Verify PageSpy integration
Let's verify the PageSpy SDK initialization and harbor instance availability.
✅ Verification successful
PageSpy integration is properly configured
The verification shows that PageSpy is correctly integrated:
- PageSpy SDK and plugins (data-harbor, rrweb) are properly included in
playground/index.html window.$harboris correctly initialized as a DataHarborPlugin instance- The harbor instance is properly accessed in
preferences-drawer.vueand theonOfflineLogmethod is being used as expected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check PageSpy SDK integration and harbor instance initialization
# Test 1: Check if PageSpy SDK is included in HTML files
echo "Checking PageSpy SDK inclusion..."
rg -l "pagespy.*sdk"
# Test 2: Check for harbor instance initialization
echo "Checking harbor instance initialization..."
rg -l "\$harbor.*=.*PageSpy"
Length of output: 246
Script:
#!/bin/bash
# Let's check the PageSpy integration more thoroughly
# Check PageSpy related imports and usage
echo "Checking PageSpy related code..."
rg -A 5 "PageSpy|pagespy" --type ts --type vue --type js
# Check harbor initialization and usage patterns
echo "Checking harbor initialization and usage..."
rg -A 5 "window\.\$harbor|harbor\s*=" --type ts --type vue --type js
# Check the playground HTML content
echo "Checking playground/index.html content..."
cat playground/index.html
Length of output: 2578
Script:
#!/bin/bash
# Let's check for Vue files specifically since the previous command had issues with Vue file type
# Check PageSpy related code in Vue files
echo "Checking PageSpy in Vue files..."
rg -A 5 "PageSpy|pagespy" --type-add 'vue:*.vue' --type vue
# Check harbor initialization and usage in Vue files
echo "Checking harbor usage in Vue files..."
rg -A 5 "window\.\$harbor|harbor\s*=" --type-add 'vue:*.vue' --type vue
# Check for offline log related functionality
echo "Checking offline log related code..."
rg -A 5 "onOfflineLog" --type-add 'vue:*.vue' --type vue
Length of output: 1611
| window.$pageSpy = new PageSpy({ | ||
| project: '<%= VITE_APP_TITLE %>', | ||
| autoRender: false, | ||
| title: 'Vben Admin', | ||
| }); |
There was a problem hiding this comment.
💡 Codebase verification
Configuration values need to be consistent with environment variables
The verification reveals inconsistencies in the configuration:
- The
projectparameter correctly usesVITE_APP_TITLEfrom environment variables - However, the
titleparameter is hardcoded as 'Vben Admin' while it should also useVITE_APP_TITLE - 'Vben Admin' is actually the default value for
VITE_APP_TITLEas seen ininternal/vite-config/src/utils/env.ts
The configuration should be updated to:
window.$pageSpy = new PageSpy({
project: '<%= VITE_APP_TITLE %>',
autoRender: false,
title: '<%= VITE_APP_TITLE %>',
});🔗 Analysis chain
Verify configuration parameters
The configuration uses hardcoded values and mixes environment variables.
Let's verify the configuration values across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for PageSpy configuration and title usage
# Check for VITE_APP_TITLE usage
echo "Checking VITE_APP_TITLE usage:"
rg "VITE_APP_TITLE" -A 2
# Check for hardcoded 'Vben Admin' title
echo "\nChecking hardcoded title usage:"
rg "'Vben Admin'" -A 2
Length of output: 6028
| <script | ||
| crossorigin="anonymous" | ||
| src="https://pagespy.mufei88.com/page-spy/index.min.js" | ||
| ></script> | ||
| <script | ||
| crossorigin="anonymous" | ||
| src="https://pagespy.mufei88.com/plugin/data-harbor/index.min.js" | ||
| ></script> | ||
| <script | ||
| crossorigin="anonymous" | ||
| src="https://pagespy.mufei88.com/plugin/rrweb/index.min.js" | ||
| ></script> |
There was a problem hiding this comment.
Security: Enhance the security of external script loading
The current implementation loads scripts from an external domain without proper security measures:
- The domain
pagespy.mufei88.comappears to be a test environment. Consider hosting these scripts on your own CDN for production. - Add Subresource Integrity (SRI) hashes to ensure script integrity.
- Pin specific versions in URLs to prevent unexpected changes.
Apply these security enhancements:
<script
crossorigin="anonymous"
+ integrity_no="sha384-[HASH]"
+ defer
- src="https://pagespy.mufei88.com/page-spy/index.min.js"
+ src="https://pagespy.mufei88.com/page-spy/1.x.x/index.min.js"
></script>Note: Replace [HASH] with actual SHA-384 hash of the file and 1.x.x with specific version.
Committable suggestion skipped: line range outside the PR's diff.
| <script> | ||
| window.$harbor = new DataHarborPlugin(); | ||
| window.$rrweb = new RRWebPlugin(); | ||
|
|
||
| [window.$harbor, window.$rrweb].forEach((p) => { | ||
| PageSpy.registerPlugin(p); | ||
| }); | ||
| // 实例化的参数都是可选的 | ||
| window.$pageSpy = new PageSpy({ | ||
| project: '<%= VITE_APP_TITLE %>', | ||
| autoRender: false, | ||
| title: 'Vben Admin', | ||
| }); | ||
| // 之后即可使用 PageSpy，前往 https://pagespy.mufei88.com 体验 | ||
| </script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve implementation robustness and environment handling
The current implementation has several areas for improvement:
- Global variables pollute the window object
- No error handling
- No environment-based conditional loading
Consider refactoring to:
<script>
+ // Only initialize in non-production environments or based on configuration
+ if (import.meta.env.DEV || window._VBEN_ADMIN_PRO_APP_CONF_.enablePageSpy) {
+ try {
+ const harbor = new DataHarborPlugin();
+ const rrweb = new RRWebPlugin();
- [window.$harbor, window.$rrweb].forEach((p) => {
+ [harbor, rrweb].forEach((p) => {
PageSpy.registerPlugin(p);
});
- window.$pageSpy = new PageSpy({
+ const pageSpy = new PageSpy({
project: '<%= VITE_APP_TITLE %>',
autoRender: false,
title: 'Vben Admin',
});
+
+ // If needed, expose through a namespaced object instead of directly on window
+ window.__VBEN__ = {
+ ...window.__VBEN__,
+ debug: { pageSpy, harbor, rrweb }
+ };
+ } catch (error) {
+ console.error('Failed to initialize PageSpy:', error);
+ }
+ }
</script>Committable suggestion skipped: line range outside the PR's diff.
| <VbenIconButton | ||
| :disabled="!harbor" | ||
| :tooltip="$t('preferences.logUpload')" | ||
| class="relative" | ||
| > | ||
| <CloudUpload class="size-4" @click="handleUploadLog" /> | ||
| </VbenIconButton> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve upload button UX and accessibility
The upload button implementation has several areas for improvement:
- Click handler should be on the button, not the icon
- Loading state should be reflected in the UI
- Missing accessibility attributes
<VbenIconButton
:disabled="!harbor"
:tooltip="$t('preferences.logUpload')"
+ :loading="isUploading"
class="relative"
+ aria-label="Upload logs"
+ @click="handleUploadLog"
>
- <CloudUpload class="size-4" @click="handleUploadLog" />
+ <CloudUpload class="size-4" />
</VbenIconButton>Committable suggestion skipped: line range outside the PR's diff.
| // 防抖 | ||
| const handleUploadLog = useThrottleFn(() => { | ||
| if (!harbor.value) { | ||
| return; | ||
| } | ||
| harbor.value.onOfflineLog('upload'); | ||
| message.copyPreferencesSuccess?.( | ||
| $t('preferences.logUploadSuccessTitle'), | ||
| $t('preferences.logUploadSuccess'), | ||
| ); | ||
| }, 5000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and user feedback in log upload
The log upload function needs improvements in several areas:
- Error handling is missing
- No loading state during upload
- Hard-coded throttle duration
- Comment should be in English for consistency
-// 防抖
+// Throttle log upload to prevent excessive calls
-const handleUploadLog = useThrottleFn(() => {
+const UPLOAD_THROTTLE_MS = 5000;
+const isUploading = ref(false);
+
+const handleUploadLog = useThrottleFn(async () => {
if (!harbor.value) {
+ message.error?.($t('preferences.logUploadNoHarbor'));
return;
}
- harbor.value.onOfflineLog('upload');
- message.copyPreferencesSuccess?.(
- $t('preferences.logUploadSuccessTitle'),
- $t('preferences.logUploadSuccess'),
- );
-}, 5000);
+ try {
+ isUploading.value = true;
+ await harbor.value.onOfflineLog('upload');
+ message.success?.(
+ $t('preferences.logUploadSuccessTitle'),
+ $t('preferences.logUploadSuccess'),
+ );
+ } catch (error) {
+ message.error?.(
+ $t('preferences.logUploadError'),
+ error instanceof Error ? error.message : 'Unknown error'
+ );
+ } finally {
+ isUploading.value = false;
+ }
+}, UPLOAD_THROTTLE_MS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 防抖 | |
| const handleUploadLog = useThrottleFn(() => { | |
| if (!harbor.value) { | |
| return; | |
| } | |
| harbor.value.onOfflineLog('upload'); | |
| message.copyPreferencesSuccess?.( | |
| $t('preferences.logUploadSuccessTitle'), | |
| $t('preferences.logUploadSuccess'), | |
| ); | |
| }, 5000); | |
| // Throttle log upload to prevent excessive calls | |
| const UPLOAD_THROTTLE_MS = 5000; | |
| const isUploading = ref(false); | |
| const handleUploadLog = useThrottleFn(async () => { | |
| if (!harbor.value) { | |
| message.error?.($t('preferences.logUploadNoHarbor')); | |
| return; | |
| } | |
| try { | |
| isUploading.value = true; | |
| await harbor.value.onOfflineLog('upload'); | |
| message.success?.( | |
| $t('preferences.logUploadSuccessTitle'), | |
| $t('preferences.logUploadSuccess'), | |
| ); | |
| } catch (error) { | |
| message.error?.( | |
| $t('preferences.logUploadError'), | |
| error instanceof Error ? error.message : 'Unknown error' | |
| ); | |
| } finally { | |
| isUploading.value = false; | |
| } | |
| }, UPLOAD_THROTTLE_MS); |
集成PageSpy 设置Drawer可上传离线日志
Summary by CodeRabbit
Release Notes
New Features
CloudUploadicon for enhanced log upload functionality.Localization
Enhancements