feat(website): Improve performance of large repository output using Ace Editor #313
Conversation
Deploying repomix with
|
| Latest commit: |
10b1f39
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://447033bb.repomix.pages.dev |
| Branch Preview URL: | https://feat-website-ace.repomix.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #313 +/- ##
=======================================
Coverage 90.12% 90.12%
=======================================
Files 48 48
Lines 2471 2471
Branches 514 514
=======================================
Hits 2227 2227
Misses 244 244 ☔ View full report in Codecov by Sentry. |
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive refactoring of the repository packing and result viewing functionality in the client-side application. The changes focus on modularizing the The request handling logic has been centralized in the Additional utility functions have been introduced in Sequence DiagramsequenceDiagram
participant User
participant TryItUrlInput
participant TryItPackOptions
participant RequestHandlers
participant PackService
participant TryItResultViewer
User->>TryItUrlInput: Enter Repository URL
TryItUrlInput->>RequestHandlers: Validate URL
RequestHandlers-->>TryItUrlInput: Validation Result
User->>TryItPackOptions: Configure Packing Options
TryItPackOptions->>RequestHandlers: Track Option Changes
User->>TryItUrlInput: Submit URL
TryItUrlInput->>RequestHandlers: Handle Pack Request
RequestHandlers->>PackService: Pack Repository
PackService-->>RequestHandlers: Return Pack Result
RequestHandlers->>TryItResultViewer: Display Result
TryItResultViewer->>User: Show Packed Repository Content
Possibly related PRs
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: 3
🧹 Nitpick comments (14)
website/client/components/utils/resultViewer.ts (4)
6-8: Add input validation and consistent date formatting.The current implementation might throw for invalid dates and produce inconsistent formats across different locales.
Consider this implementation:
export function formatTimestamp(timestamp: string): string { + if (!timestamp || isNaN(Date.parse(timestamp))) { + return ''; + } - return new Date(timestamp).toLocaleString(); + return new Date(timestamp).toLocaleString('en-US', { + year: 'numeric', + month: 'short', + day: 'numeric', + hour: '2-digit', + minute: '2-digit' + }); }
13-22: Add type annotation for error handling.The implementation looks good with proper error handling and analytics integration.
Consider adding error type annotation:
- } catch (err) { + } catch (err: unknown) {
27-41: Improve MIME type handling and extension mapping.While the implementation works, it could be more maintainable and accurate with proper MIME types.
Consider this enhancement:
+const FORMAT_CONFIG = { + markdown: { ext: 'md', mime: 'text/markdown' }, + xml: { ext: 'xml', mime: 'application/xml' }, + text: { ext: 'txt', mime: 'text/plain' } +} as const; + export function downloadResult(content: string, format: string): void { - const blob = new Blob([content], { type: 'text/plain' }); + const config = FORMAT_CONFIG[format as keyof typeof FORMAT_CONFIG] || FORMAT_CONFIG.text; + const blob = new Blob([content], { type: config.mime }); const url = window.URL.createObjectURL(blob); const a = document.createElement('a'); - const extension = format === 'markdown' ? 'md' : format === 'xml' ? 'xml' : 'txt'; a.href = url; - a.download = `repomix-output.${extension}`; + a.download = `repomix-output.${config.ext}`;
46-54: Add return type annotation for better type safety.The editor options look good for handling large repository output.
Consider adding type annotation:
-export function getEditorOptions() { +export function getEditorOptions(): { + readOnly: boolean; + wrap: boolean; + showPrintMargin: boolean; + fontSize: string; + useWorker: boolean; +} {website/client/components/ResultViewer.vue (3)
22-32: Add type safety to event handling.The copy functionality looks good but could benefit from stronger typing.
Consider this enhancement:
-async function handleCopy(event: Event) { +async function handleCopy(event: MouseEvent) {
35-40: Add type safety to download event handling.Similar to the copy handler, the download functionality could benefit from stronger typing.
Consider this enhancement:
-function handleDownload(event: Event) { +function handleDownload(event: MouseEvent) {
111-118: Enhance accessibility for the editor component.The Ace Editor integration looks good, but could benefit from accessibility improvements.
Consider adding ARIA attributes:
<div class="editor-container"> <VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" :options="editorOptions" + aria-label="Repository content viewer" + role="textbox" + aria-readonly="true" /> </div>website/client/components/utils/validation.ts (2)
3-3: Consider creating an issue to track the TODO.The TODO comment indicates a need to share validation logic with the core module. This could prevent logic duplication and ensure consistency.
Would you like me to create an issue to track sharing the validation logic with
src/cli/actions/remoteAction.ts?
7-11: Consider adding test cases for edge cases in the shorthand format.The regex pattern correctly validates the GitHub repository shorthand format. However, consider adding validation for:
- Maximum length of username and repository names
- Reserved GitHub usernames
- Special characters that GitHub doesn't allow
website/client/components/utils/requestHandlers.ts (2)
5-9: Consider adding validation for handler options.The
RequestHandlerOptionsinterface could benefit from runtime validation to ensure callback functions are properly typed.+function isValidHandlerOptions(options: unknown): options is RequestHandlerOptions { + if (typeof options !== 'object' || options === null) return false; + const { onSuccess, onError, signal } = options as RequestHandlerOptions; + return ( + (onSuccess === undefined || typeof onSuccess === 'function') && + (onError === undefined || typeof onError === 'function') && + (signal === undefined || signal instanceof AbortSignal) + ); +}
63-68: Consider adding type validation for analytics action.The
handleOptionChangefunction should validate that the providedanalyticsActionis a valid enum value.export function handleOptionChange(value: boolean | string, analyticsAction: AnalyticsActionType): void { + if (!Object.values(AnalyticsAction).includes(analyticsAction)) { + console.warn(`Invalid analytics action: ${analyticsAction}`); + return; + } if (typeof value === 'boolean') {website/client/components/TryIt.vue (3)
Line range hint
134-143: Add debouncing to URL input handler.The URL input handler could benefit from debouncing to prevent excessive validation checks during rapid typing.
+import { debounce } from 'lodash-es'; + +const debouncedValidation = debounce((value: string) => { + url.value = value; +}, 300); + function handleUrlInput(event: Event) { const input = event.target as HTMLInputElement; - url.value = input.value; + debouncedValidation(input.value); }
Line range hint
144-149: Add keyboard accessibility for form submission.The keyboard event handler should also handle the Space key for better accessibility.
function handleKeydown(event: KeyboardEvent) { - if (event.key === 'Enter' && isValidUrl.value && !loading.value) { + if ((event.key === 'Enter' || event.key === ' ') && isValidUrl.value && !loading.value) { + event.preventDefault(); handleSubmit(); } }
Line range hint
156-167: Enhance accessibility for URL input field.The URL input field could benefit from additional ARIA attributes and validation feedback.
<input :value="url" @input="handleUrlInput" @keydown="handleKeydown" type="text" placeholder="GitHub repository URL or user/repo (e.g., yamadashy/repomix)" class="repository-input" :class="{ 'invalid': url && !isValidUrl }" aria-label="GitHub repository URL" + aria-invalid="url && !isValidUrl" + aria-describedby="url-error" + autocomplete="off" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
website/client/package-lock.jsonis excluded by!**/package-lock.jsonwebsite/server/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
website/client/components/ResultViewer.vue(4 hunks)website/client/components/TryIt.vue(3 hunks)website/client/components/api/client.ts(0 hunks)website/client/components/utils/analytics.ts(0 hunks)website/client/components/utils/requestHandlers.ts(1 hunks)website/client/components/utils/resultViewer.ts(1 hunks)website/client/components/utils/validation.ts(1 hunks)website/client/package.json(1 hunks)website/server/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- website/client/components/api/client.ts
- website/client/components/utils/analytics.ts
✅ Files skipped from review due to trivial changes (1)
- website/server/package.json
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
website/client/components/ResultViewer.vue (1)
268-272: LGTM! Good responsive styling.The editor container styling is well-implemented with proper use of CSS variables and responsive design considerations.
website/client/package.json (2)
14-15: LGTM! Dependencies are properly ordered.The dependencies are correctly ordered alphabetically, and the versions are specified using the caret notation for flexible minor version updates.
15-15: Verify vue3-ace-editor version and dependencies.The addition of vue3-ace-editor aligns with the PR objective of improving large repository output performance.
Let's verify the package version and check for any security advisories:
✅ Verification successful
✅ vue3-ace-editor dependency is verified and secure
The package version (2.2.4) is the latest available, and both vue3-ace-editor and its peer dependency ace-builds show no security vulnerabilities. The package is actively maintained with recent updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check vue3-ace-editor package information and dependencies # Check latest version and release date echo "Checking vue3-ace-editor package info..." npm view vue3-ace-editor time.modified versions dist-tags # Check for any security advisories echo "Checking for security advisories..." npm audit vue3-ace-editor # Check peer dependencies echo "Checking peer dependencies..." npm view vue3-ace-editor peerDependenciesLength of output: 963
Script:
#!/bin/bash # Check ace-builds security advisories and version echo "Checking ace-builds package info..." npm view ace-builds time.modified versions dist-tags echo "Checking for ace-builds in package.json..." cat website/client/package.json | grep -A 2 -B 2 "ace-builds" echo "Checking for security advisories for ace-builds..." npm audit ace-buildsLength of output: 1811
yamadashy
force-pushed
the
feat/website-ace
branch
from
90a0f55 to
14aaf98
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/client/components/ResultViewer.vue (3)
15-15: Consider memoizing editor options.Since
editorOptionsis static configuration, consider usingcomputed()or moving it outside the component setup to prevent unnecessary recalculation.-const editorOptions = getEditorOptions(); +const editorOptions = computed(() => getEditorOptions());
111-118: Consider dynamic language mode for syntax highlighting.The editor's language mode is hardcoded to 'text', which might not provide optimal syntax highlighting for different content formats (e.g., JSON, YAML, etc.).
- <VAceEditor - v-model:value="result.content" - :lang="'text'" - :style="{ height: '100%', width: '100%' }" - :options="editorOptions" - /> + <VAceEditor + v-model:value="result.content" + :lang="result.format.toLowerCase()" + :style="{ height: '100%', width: '100%' }" + :options="editorOptions" + />
Line range hint
1-286: Consider lazy-loading the Ace Editor for performance.The component shows good architecture and implementation. For further optimization, consider lazy-loading the Ace Editor to reduce the initial bundle size, especially since it's only needed when results are available.
-import { VAceEditor } from 'vue3-ace-editor'; +const VAceEditor = defineAsyncComponent(() => + import('vue3-ace-editor').then(mod => mod.VAceEditor) +);Don't forget to import
defineAsyncComponent:-import { computed, ref } from 'vue'; +import { computed, ref, defineAsyncComponent } from 'vue';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
website/client/components/ResultViewer.vue(4 hunks)website/client/components/TryIt.vue(2 hunks)website/client/components/api/client.ts(0 hunks)website/client/components/utils/analytics.ts(0 hunks)website/client/components/utils/requestHandlers.ts(1 hunks)website/client/components/utils/resultViewer.ts(1 hunks)website/client/components/utils/validation.ts(1 hunks)website/client/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- website/client/components/api/client.ts
- website/client/components/utils/analytics.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- website/client/package.json
- website/client/components/utils/validation.ts
- website/client/components/utils/resultViewer.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (11)
website/client/components/ResultViewer.vue (4)
22-32: LGTM! Well-structured copy handler.The copy handler follows best practices:
- Prevents default event behavior
- Includes null checks
- Provides user feedback
- Uses extracted utility function
35-40: LGTM! Well-structured download handler.The download handler follows similar good practices as the copy handler.
268-272: LGTM! Well-structured styles.The editor container styles maintain consistency with the design system and provide proper containment for the Ace Editor.
4-4: LGTM! Good separation of concerns.The extraction of utility functions and integration of the Ace Editor shows good architectural decisions for maintainability.
Let's verify the package dependency:
Also applies to: 6-6
✅ Verification successful
Import statement is properly configured with required dependency ✓
The
vue3-ace-editorpackage is correctly added as a dependency inwebsite/client/package.json, validating the import statement.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if vue3-ace-editor is properly added to package.json grep -r "vue3-ace-editor" website/package.jsonLength of output: 100
Script:
#!/bin/bash # Find all package.json files and search for vue3-ace-editor fd package.json --type f --exec grep -l "vue3-ace-editor" {} \;Length of output: 96
website/client/components/utils/requestHandlers.ts (3)
1-10: LGTM! Well-structured interface definition.The
RequestHandlerOptionsinterface is well-designed with clear optional properties for success/error callbacks and abort signal.
63-69: LGTM! Clean and focused implementation.The function handles both boolean and string values appropriately, with consistent analytics tracking.
47-52: 🛠️ Refactor suggestionImprove error handling for AbortError.
The current error handling for AbortError could be more robust.
Apply this diff to improve the error handling:
- const errorMessage = err instanceof Error ? err.message : 'An unexpected error occurred'; - - if (errorMessage === 'AbortError') { + let errorMessage: string; + if (err instanceof Error) { + if (err.name === 'AbortError') { + errorMessage = 'Request was cancelled'; + } else { + errorMessage = err.message; + } + } else { + errorMessage = 'An unexpected error occurred'; + } + + if (err instanceof Error && err.name === 'AbortError') { onError?.('Request was cancelled'); return; }Likely invalid or redundant comment.
website/client/components/TryIt.vue (4)
1-8: LGTM! Well-organized imports and modern Vue setup.Using Vue 3's script setup syntax with proper imports organization.
89-131: LGTM! Consistent event handling implementation.The event handlers are well-structured with:
- Clear separation of concerns
- Consistent analytics tracking
- Proper state management
Line range hint
144-669: LGTM! Well-structured template with proper accessibility.The template implementation shows:
- Good attention to accessibility with proper ARIA labels
- Responsive design with appropriate breakpoints
- Clear visual feedback for user interactions
51-56:⚠️ Potential issueFix the timeout implementation to properly handle errors.
The current timeout implementation has two issues:
- Throwing an error in a setTimeout callback won't be caught by try-catch
- The error won't be properly propagated to the UI
Apply this diff to fix the timeout handling:
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + requestController.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest( + url.value, + format.value, + { + removeComments: removeComments.value, + removeEmptyLines: removeEmptyLines.value, + showLineNumbers: showLineNumbers.value, + fileSummary: fileSummary.value, + directoryStructure: directoryStructure.value, + includePatterns: includePatterns.value ? includePatterns.value.trim() : undefined, + ignorePatterns: ignorePatterns.value ? ignorePatterns.value.trim() : undefined, + outputParsable: outputParsable.value, + }, + { + onSuccess: (response) => { + result.value = response; + }, + onError: (errorMessage) => { + error.value = errorMessage; + }, + signal: requestController.signal, + }, + ), + timeoutPromise + ]); + } catch (err) { + error.value = err instanceof Error ? err.message : 'An unexpected error occurred'; + }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.npmignore(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
website/client/components/TryIt.vue (3)
44-49:⚠️ Potential issueFix timeout handling mechanism.
The current timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
Apply this diff to fix the timeout handling:
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + requestController.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest( + url.value, + format.value, + { + removeComments: removeComments.value, + removeEmptyLines: removeEmptyLines.value, + showLineNumbers: showLineNumbers.value, + fileSummary: fileSummary.value, + directoryStructure: directoryStructure.value, + includePatterns: includePatterns.value ? includePatterns.value.trim() : undefined, + ignorePatterns: ignorePatterns.value ? ignorePatterns.value.trim() : undefined, + outputParsable: outputParsable.value, + }, + { + onSuccess: (response) => { + result.value = response; + }, + onError: (errorMessage) => { + error.value = errorMessage; + }, + signal: requestController.signal, + }, + ), + timeoutPromise + ]); + } catch (err) { + error.value = err instanceof Error ? err.message : 'An error occurred'; + }
44-49:⚠️ Potential issueFix timeout handling mechanism.
The timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + requestController.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest( + url.value, + format.value, + { + removeComments: removeComments.value, + removeEmptyLines: removeEmptyLines.value, + showLineNumbers: showLineNumbers.value, + fileSummary: fileSummary.value, + directoryStructure: directoryStructure.value, + includePatterns: includePatterns.value ? includePatterns.value.trim() : undefined, + ignorePatterns: ignorePatterns.value ? ignorePatterns.value.trim() : undefined, + outputParsable: outputParsable.value, + }, + { + onSuccess: (response) => { + result.value = response; + }, + onError: (errorMessage) => { + error.value = errorMessage; + }, + signal: requestController.signal, + }, + ), + timeoutPromise + ]); + } catch (err) { + error.value = err instanceof Error ? err.message : 'An error occurred'; + }
44-49:⚠️ Potential issueImprove timeout handling mechanism.
The timeout implementation has potential issues that were previously identified.
🧹 Nitpick comments (9)
website/client/components/TryItUrlInput.vue (4)
32-72: Consider enhancing keyboard accessibility.While the component has good accessibility features like ARIA labels, consider adding
role="search"to the input group and handling theEscapekey to clear the input.- <div class="input-group"> + <div class="input-group" role="search"> <div class="url-input-container"> <input :value="url" @input="handleUrlInput" - @keydown="handleKeydown" + @keydown="(e) => { + handleKeydown(e); + if (e.key === 'Escape') emit('update:url', ''); + }" type="text"
35-44: Consider adding debounce to URL input validation.The URL validation runs on every input change, which could be performance-intensive for rapid typing.
+import { debounce } from 'lodash-es'; + function handleUrlInput(event: Event) { const input = event.target as HTMLInputElement; - emit('update:url', input.value); + debouncedEmit(input.value); } + +const debouncedEmit = debounce((value: string) => { + emit('update:url', value); +}, 300);
149-154: Consider adding a min-width for mobile responsiveness.The current mobile layout might become too cramped for very small screens.
@media (max-width: 640px) { .url-input-container { flex-direction: column; + min-width: 280px; } }
22-25: Consider debouncing the URL input handler.For better performance, consider debouncing the URL input handler to reduce the number of validation calls and re-renders.
+import { debounce } from 'lodash-es'; + +const debouncedHandleUrlInput = debounce((value: string) => { + emit('update:url', value); +}, 300); + function handleUrlInput(event: Event) { const input = event.target as HTMLInputElement; - emit('update:url', input.value); + debouncedHandleUrlInput(input.value); }website/client/components/TryItPackOptions.vue (3)
30-73: Consider extracting analytics tracking logic.The event handlers mix UI updates with analytics tracking. Consider using a composable for analytics:
// useAnalytics.ts export function useAnalytics() { return { trackOptionChange: (value: unknown, action: AnalyticsAction) => { handleOptionChange(value, action); } }; } // In component: const { trackOptionChange } = useAnalytics(); function handleFormatChange(newFormat: 'xml' | 'markdown' | 'plain') { emit('update:format', newFormat); trackOptionChange(newFormat, AnalyticsAction.FORMAT_CHANGE); }
30-73: Consider grouping related handlers using a factory function.The current implementation has repetitive handler functions that follow the same pattern.
+function createOptionHandler(emitName: string, action: AnalyticsAction) { + return (value: boolean | string) => { + emit(`update:${emitName}`, value); + handleOptionChange(value, action); + }; +} + -function handleFormatChange(newFormat: 'xml' | 'markdown' | 'plain') { - emit('update:format', newFormat); - handleOptionChange(newFormat, AnalyticsAction.FORMAT_CHANGE); -} +const handleFormatChange = createOptionHandler('format', AnalyticsAction.FORMAT_CHANGE);
35-38: Add validation for include/ignore patterns.Consider validating the glob patterns to ensure they are properly formatted.
+import { isValidGlobPattern } from './utils/validation'; + function handleIncludePatternsUpdate(patterns: string) { + const validPatterns = patterns.split(',') + .map(p => p.trim()) + .filter(p => p && isValidGlobPattern(p)) + .join(','); - emit('update:includePatterns', patterns); + emit('update:includePatterns', validPatterns); handleOptionChange(patterns, AnalyticsAction.UPDATE_INCLUDE_PATTERNS); }Also applies to: 40-43
website/client/components/TryItResultViewer.vue (2)
268-270: Consider adding loading state for the editor.Large files might take time to load in the editor.
.editor-container { height: 100%; width: 100%; font-family: var(--vp-font-family-mono); + position: relative; } + +.editor-loading { + position: absolute; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); +}
111-118: Consider adding error boundary for editor crashes.Large files might cause the editor to crash. Consider adding an error boundary component to gracefully handle such cases.
+<script setup lang="ts"> +import { onErrorCaptured, ref } from 'vue'; + +const editorError = ref<Error | null>(null); + +onErrorCaptured((err) => { + editorError.value = err; + return false; +}); +</script> <div class="editor-container"> + <div v-if="editorError" class="editor-error"> + <p>Failed to load editor. Falling back to plain text.</p> + <pre><code>{{ result.content }}</code></pre> + </div> + <div v-else> <VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" :options="editorOptions" /> + </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
website/client/components/TryIt.vue(2 hunks)website/client/components/TryItPackOptions.vue(1 hunks)website/client/components/TryItResultViewer.vue(4 hunks)website/client/components/TryItUrlInput.vue(1 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- website/client/components/TryItUrlInput.vue
- website/client/components/TryItResultViewer.vue
- website/client/components/TryItPackOptions.vue
- website/client/components/TryIt.vue
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test coverage
- GitHub Check: Cloudflare Pages
🔇 Additional comments (11)
website/client/components/TryItUrlInput.vue (4)
1-30: LGTM! Well-structured component setup.The component follows Vue.js best practices:
- Uses TypeScript for type safety
- Properly defines props and emits
- Implements clean validation logic
- Follows the single responsibility principle
149-154: LGTM! Responsive design implementation.The mobile-first approach with proper breakpoints is well implemented.
1-30: LGTM! Well-structured component setup with TypeScript.The component follows Vue.js best practices:
- Strong typing with TypeScript
- Clear prop and emit definitions
- Proper validation using computed properties
- Clean event handling
1-74: LGTM! Well-structured component with proper TypeScript integration.The component is well-organized with proper type definitions, computed properties, and event handling.
website/client/components/TryItResultViewer.vue (4)
4-6: LGTM! Clean dependency management.The imports are well-organized and properly typed.
268-270: LGTM! Proper editor container styling.The container styles ensure the editor fills the available space while maintaining the monospace font.
4-6: LGTM! Good separation of concerns.Clean imports with proper file extensions and type imports.
111-118: LGTM! Good integration of Ace Editor for improved performance.The Ace Editor integration with proper styling and options setup will help improve performance for large repository outputs.
website/client/components/TryIt.vue (2)
90-107: LGTM! Clean component composition.The refactoring to use separate components for URL input, pack options, and result viewing improves code organization and maintainability.
90-107: LGTM! Well-structured component composition.Good separation of concerns using child components with proper prop bindings.
website/client/components/TryItPackOptions.vue (1)
1-74: LGTM! Well-organized component with proper analytics tracking.The component is well-structured with proper type definitions and analytics integration.
yamadashy
force-pushed
the
feat/website-ace
branch
from
2db6c0f to
59f31bf
Compare
yamadashy
force-pushed
the
feat/website-ace
branch
from
59f31bf to
9d2df19
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
website/client/components/TryIt.vue (1)
44-49:⚠️ Potential issueImprove timeout handling mechanism.
The current timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
Apply this diff to improve the timeout handling:
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + currentRequest.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest(/*...*/), + timeoutPromise + ]); + } catch (err) { + // Error handling + }
🧹 Nitpick comments (2)
website/client/components/TryItUrlInput.vue (2)
68-71: Enhance accessibility for validation messages.Add
aria-liveto announce validation messages to screen readers.- <div v-if="url && !isValidUrl" class="url-warning"> + <div v-if="url && !isValidUrl" class="url-warning" aria-live="polite">
35-44: Add autocomplete attribute for better UX.Consider adding
autocomplete="off"to prevent unwanted suggestions for repository URLs.type="text" + autocomplete="off" placeholder="GitHub repository URL or user/repo (e.g., yamadashy/repomix)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
website/client/components/TryIt.vue(2 hunks)website/client/components/TryItPackOptions.vue(1 hunks)website/client/components/TryItResultViewer.vue(4 hunks)website/client/components/TryItUrlInput.vue(1 hunks)website/client/components/utils/resultViewer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/client/components/utils/resultViewer.ts
👮 Files not reviewed due to content moderation or server errors (4)
- website/client/components/TryItPackOptions.vue
- website/client/components/TryIt.vue
- website/client/components/TryItUrlInput.vue
- website/client/components/TryItResultViewer.vue
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (23)
website/client/components/TryItPackOptions.vue (9)
1-74: LGTM! Well-structured script setup.The implementation follows Vue's best practices with proper TypeScript types and consistent handler patterns.
190-203: Consider improving tooltip accessibility.The tooltip implementation could be enhanced for better accessibility.
211-371: LGTM! Well-organized styles.The styles follow best practices with proper use of CSS variables and responsive design.
1-74: LGTM! Well-structured script setup.The implementation follows Vue's best practices with proper TypeScript types, clear prop definitions, and consistent analytics tracking.
190-203: Improve tooltip accessibility.The tooltip implementation could be enhanced for better accessibility.
Apply this diff to improve accessibility:
<div class="parsable-option"> <span>Output Parsable Format</span> <div class="tooltip-container"> <HelpCircle :size="16" class="help-icon" + role="button" + tabindex="0" + @keydown.enter="() => {}" aria-label="More information about parsable format" /> - <div class="tooltip-content"> + <div + class="tooltip-content" + role="tooltip" + aria-hidden="true" + > Whether to escape the output based on the chosen style schema. Note that this can increase token count. <div class="tooltip-arrow"></div> </div> </div> </div>
211-371: LGTM! Well-organized styles.The styles are well-structured with proper use of CSS variables, clear organization, and responsive design considerations.
1-74: LGTM! Well-structured script section.The script section demonstrates good practices:
- Props and emits are properly typed
- Handlers follow a consistent pattern
- Analytics tracking is integrated
190-203: Improve tooltip accessibility.The tooltip implementation could be enhanced for better accessibility.
Apply this diff to improve accessibility:
<div class="parsable-option"> <span>Output Parsable Format</span> <div class="tooltip-container"> <HelpCircle :size="16" class="help-icon" + role="button" + tabindex="0" + @keydown.enter="() => {}" aria-label="More information about parsable format" /> - <div class="tooltip-content"> + <div + class="tooltip-content" + role="tooltip" + aria-hidden="true" + > Whether to escape the output based on the chosen style schema. Note that this can increase token count. <div class="tooltip-arrow"></div> </div> </div> </div>
211-371: LGTM! Well-structured styles.The styles demonstrate good practices:
- Scoped to the component
- Proper use of CSS variables
- Responsive design implementation
website/client/components/TryIt.vue (6)
44-49: Improve timeout handling mechanism.The current timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
28-28: Add cleanup for request controller.Consider cleaning up the request controller when the component is unmounted.
28-28: Add cleanup for request controller.Consider cleaning up the request controller when the component is unmounted.
Apply this diff:
+import { onUnmounted } from 'vue'; + let requestController: AbortController | null = null; + +onUnmounted(() => { + if (requestController) { + requestController.abort(); + requestController = null; + } +});
28-28: Add cleanup for request controller.Consider cleaning up the request controller when the component is unmounted.
Apply this diff to add cleanup:
+import { onUnmounted } from 'vue'; + let requestController: AbortController | null = null; + +onUnmounted(() => { + if (requestController) { + requestController.abort(); + requestController = null; + } +});
90-114: LGTM! Well-structured template.The template demonstrates good practices:
- Components are properly integrated
- Props and events are correctly bound
44-49:⚠️ Potential issueImprove timeout handling mechanism.
The current timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
Apply this diff to improve the timeout handling:
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + requestController.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest(/*...*/), + timeoutPromise + ]); + } catch (err) { + // Error handling + }Likely invalid or redundant comment.
website/client/components/TryItResultViewer.vue (5)
111-118: Consider configuring Ace Editor for large files.While Ace Editor is a good choice for performance, consider adding these configurations for better handling of large files.
111-118: Consider adding error boundary for editor initialization.The Ace Editor initialization might fail in certain environments.
111-118: Consider configuring Ace Editor for large files and adding error boundary.The Ace Editor implementation could be improved for better handling of large files and error cases.
Apply these diffs:
- Configure Ace Editor for large files:
<VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" - :options="editorOptions" + :options="{ + ...editorOptions, + maxLines: 5000, + minLines: 10, + wrap: true, + useWorker: false, + showPrintMargin: false + }" />
- Add error boundary for editor initialization:
<div class="editor-container"> + <ErrorBoundary v-slot="{ error }"> + <div v-if="error" class="editor-error"> + Failed to load editor: {{ error }} + </div> <VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" :options="editorOptions" /> + </ErrorBoundary> </div>
111-118: Consider configuring Ace Editor for large files.While Ace Editor is a good choice for performance, consider adding these configurations for better handling of large files.
Apply this diff to improve the editor configuration:
<VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" - :options="editorOptions" + :options="{ + ...editorOptions, + maxLines: 5000, + minLines: 10, + wrap: true, + useWorker: false, + showPrintMargin: false + }" />Consider adding error boundary for editor initialization.
The Ace Editor initialization might fail in certain environments.
Apply this diff to add error handling:
<div class="editor-container"> + <ErrorBoundary v-slot="{ error }"> + <div v-if="error" class="editor-error"> + Failed to load editor: {{ error }} + </div> <VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" :options="editorOptions" /> + </ErrorBoundary> </div>
268-270: LGTM! Well-structured editor container styles.The styles demonstrate good practices:
- Proper dimensions
- Font family configuration
website/client/components/TryItUrlInput.vue (3)
1-154: LGTM! Well-implemented URL input component.The implementation follows best practices with:
- Strong TypeScript typing
- Clear validation logic
- Good error handling and user feedback
- Proper accessibility attributes
1-73: LGTM! Well-implemented URL input component.The implementation demonstrates good practices:
- Proper URL validation
- Clear error handling
- Good user feedback
75-154: LGTM! Well-structured styles.The styles demonstrate good practices:
- Proper use of CSS variables
- Responsive design implementation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (2)
721-731: Consider adding information about data persistence.The privacy policy clearly outlines data collection and network usage. However, it could be beneficial to explicitly mention whether any data is cached or temporarily stored during remote repository processing.
### Network Usage Repomix operates fully offline after installation. The only cases where an internet connection is needed are: - Installation via npm/yarn. - Using the `--remote` flag to process remote repositories. - Checking for updates (manually triggered). + +When processing remote repositories, files are temporarily cloned to a system temporary directory and are automatically cleaned up after processing.
732-734: Consider expanding security considerations.While the security section is clear, it could be enhanced by mentioning the built-in security checks that prevent sensitive information leakage.
### Security Considerations Since all processing is local, Repomix is safe to use with private and internal repositories. +Additionally, Repomix includes built-in security checks using Secretlint to prevent accidental exposure of sensitive information in the generated output.website/client/components/TryItPackOptions.vue (1)
111-118: Add pattern validation for include/ignore inputs.Consider adding real-time validation for glob patterns to provide immediate feedback to users.
+import { isValidGlobPattern } from './utils/validation.js'; + +const includePatternError = ref<string | null>(null); +const ignorePatternError = ref<string | null>(null); + +function validatePattern(pattern: string): string | null { + if (!pattern) return null; + return isValidGlobPattern(pattern) ? null : 'Invalid glob pattern'; +} <input :value="includePatterns" - @input="event => handleIncludePatternsUpdate((event.target as HTMLInputElement).value)" + @input="event => { + const value = (event.target as HTMLInputElement).value; + includePatternError.value = validatePattern(value); + handleIncludePatternsUpdate(value); + }" type="text" class="repository-input" placeholder="Comma-separated patterns to include. e.g., src/**/*.ts" aria-label="Include patterns" + :aria-invalid="!!includePatternError.value" + :aria-errormessage="includePatternError.value || undefined" /> +<span v-if="includePatternError" class="error-message">{{ includePatternError }}</span>Also applies to: 123-130
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)website/client/components/TryIt.vue(2 hunks)website/client/components/TryItPackOptions.vue(1 hunks)website/client/components/TryItResultViewer.vue(4 hunks)website/client/components/TryItUrlInput.vue(1 hunks)website/client/components/utils/resultViewer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- website/client/components/TryItUrlInput.vue
- website/client/components/utils/resultViewer.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
README.md (2)
Line range hint
706-719: LGTM! Well-structured contribution section.The contribution section effectively encourages community involvement and includes a nice visual representation of contributors.
735-738: LGTM! Clear liability disclaimer.The liability disclaimer is well-written and covers the necessary aspects of warranty and responsibility.
website/client/components/TryIt.vue (2)
28-28: 🛠️ Refactor suggestionAdd cleanup for request controller.
Consider cleaning up the request controller when the component is unmounted.
+import { onUnmounted } from 'vue'; + let requestController: AbortController | null = null; + +onUnmounted(() => { + if (requestController) { + requestController.abort(); + requestController = null; + } +});Likely invalid or redundant comment.
44-49:⚠️ Potential issueImprove timeout handling mechanism.
The current timeout implementation has potential issues:
- The error is thrown but not caught in the try-catch block
- The timeout error might not be properly propagated to the UI
Consider using Promise.race for better timeout handling:
- const timeoutId = setTimeout(() => { - if (requestController) { - requestController.abort('Request timed out'); - throw new Error('Request timed out'); - } - }, TIMEOUT_MS); + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + if (requestController) { + requestController.abort(); + reject(new Error('Request timed out')); + } + }, TIMEOUT_MS); + }); + + try { + await Promise.race([ + handlePackRequest( + url.value, + format.value, + { + removeComments: removeComments.value, + removeEmptyLines: removeEmptyLines.value, + showLineNumbers: showLineNumbers.value, + fileSummary: fileSummary.value, + directoryStructure: directoryStructure.value, + includePatterns: includePatterns.value ? includePatterns.value.trim() : undefined, + ignorePatterns: ignorePatterns.value ? ignorePatterns.value.trim() : undefined, + outputParsable: outputParsable.value, + }, + { + onSuccess, + onError, + signal: requestController.signal, + } + ), + timeoutPromise + ]); + } catch (err) { + onError(err.message); + }Likely invalid or redundant comment.
website/client/components/TryItResultViewer.vue (1)
111-118: 🛠️ Refactor suggestionOptimize Ace Editor for large files.
While Ace Editor is a good choice for performance, consider these optimizations for better handling of large files:
- Configure editor for large files:
<VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" - :options="editorOptions" + :options="{ + ...editorOptions, + maxLines: 5000, + minLines: 10, + wrap: true, + useWorker: false, + showPrintMargin: false + }" />
- Add error boundary for editor initialization:
<div class="editor-container"> + <ErrorBoundary v-slot="{ error }"> + <div v-if="error" class="editor-error"> + Failed to load editor: {{ error }} + </div> <VAceEditor v-model:value="result.content" :lang="'text'" :style="{ height: '100%', width: '100%' }" :options="editorOptions" /> + </ErrorBoundary> </div>Likely invalid or redundant comment.
website/client/components/TryItPackOptions.vue (1)
190-203: 🛠️ Refactor suggestionImprove tooltip accessibility.
The tooltip implementation should be enhanced for better accessibility.
<div class="parsable-option"> <span>Output Parsable Format</span> <div class="tooltip-container"> <HelpCircle :size="16" class="help-icon" + role="button" + tabindex="0" + @keydown.enter="() => {}" aria-label="More information about parsable format" /> - <div class="tooltip-content"> + <div + class="tooltip-content" + role="tooltip" + aria-hidden="true" + > Whether to escape the output based on the chosen style schema. Note that this can increase token count. <div class="tooltip-arrow"></div> </div> </div> </div>Likely invalid or redundant comment.
related: #312
Checklist
npm run testnpm run lint