🚀 prod #8
🚀 prod #8
Conversation
fix(storybook): resolve typing and script issues
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR removes three internal documentation markdown files from the repository. While titled "🚀 prod", the changes consist entirely of deleting documentation files that are not referenced elsewhere in the codebase.
Key changes:
- Complete removal of internal/working documentation files
- No code or functional changes
- No impact on the published package or user-facing documentation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| DATATABLE.md | Removes 536 lines of DataTable component documentation (appears to be internal/draft documentation not referenced in the main README) |
| COMPONENTS_EXAMPLES.md | Removes 360 lines of component usage examples (internal documentation not linked from the main README) |
| AUDIT_REPORT.md | Removes 2,251 lines of detailed audit report and best practices analysis (internal report dated 2025-12-14, not part of public documentation) |
Note: These files are not referenced anywhere in the codebase (README.md, package.json, or other files), suggesting they were internal working documents or drafts that are being cleaned up before production deployment.
Comments suppressed due to low confidence (1)
DATATABLE.md:1
- The PR title "🚀 prod" is vague and doesn't describe what changes are being made. Consider using a more descriptive title that clearly indicates documentation files are being removed, such as "docs: remove unused documentation files" or "chore: clean up internal documentation (DATATABLE.md, COMPONENTS_EXAMPLES.md, AUDIT_REPORT.md)". This helps with project history and makes it easier to understand what was changed when reviewing the commit log.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <template> | ||
| <!-- Root element con classe dal theme --> |
There was a problem hiding this comment.
There's duplicate template code on lines 67-68. Line 67 contains a complete template closing tag </template> followed by line 68 which starts another template section. This will cause a syntax error. One of these template tags should be removed.
| */ | ||
| import { computed } from 'vue' | ||
| import componentTheme from '@/themes/button' // Cambia con il tuo theme |
There was a problem hiding this comment.
The import statement on line 13 references '@/themes/button' but the comment says to change it to your own theme. Since this is a template file, this import will fail for anyone copying this template unless they happen to have a button theme. Consider using a more generic placeholder like '@/themes/component' or adding a comment to explicitly replace this.
| import componentTheme from '@/themes/button' // Cambia con il tuo theme | |
| import componentTheme from '@/themes/component' // Cambia con il tuo theme |
| // // Mock router-link for Storybook | ||
| // setup((app) => { | ||
| // app.component('router-link', { | ||
| // props: ['to'], | ||
| // template: '<a :href="to"><slot /></a>' | ||
| // }) | ||
| // }) |
There was a problem hiding this comment.
Storybook setup is commented out, which will break any stories that rely on router-link components. If router-link mocking is no longer needed, this code should be removed entirely rather than commented out. If it's still needed, it should be uncommented.
| }) | ||
| const emit = defineEmits(['update:show']) | ||
| const emit = defineEmits<{ 'update:show': [] }>() |
There was a problem hiding this comment.
The emit definition for 'update:show' is missing the payload type. If this emit doesn't send any payload, it should be defined as an empty array with no type parameters. However, this is unusual for an 'update:show' event which typically passes a boolean value. Consider using 'update:show': [value: boolean] instead.
| const emit = defineEmits<{ 'update:show': [] }>() | |
| const emit = defineEmits<{ 'update:show': [value: boolean] }>() |
| const slots = defineSlots() | ||
| const emits = defineEmits(['close']) | ||
| const emits = defineEmits<{ close: [] }>() |
There was a problem hiding this comment.
The emit definition for 'close' is missing the payload type. If the event sends no data, the empty array is correct, but typically close events may include an Event object. Consider if you need close: [event?: MouseEvent] for better type safety.
| 'row:click': [any] | ||
| 'row:hover': [any] | ||
| 'row:select': [any] |
There was a problem hiding this comment.
The DataTable emits have inconsistent tuple syntax. Lines 88-94 use square bracket syntax [SortingState] while lines 95-97 use [any]. For consistency and better type safety, all emits should use the same pattern and 'any' should be replaced with specific types like [row: any].
| 'row:click': [any] | |
| 'row:hover': [any] | |
| 'row:select': [any] | |
| 'row:click': [row: any] | |
| 'row:hover': [row: any] | |
| 'row:select': [row: any] |
| const handleRowClick = (row: any, event: MouseEvent | KeyboardEvent) => { | ||
| const target = event.target as HTMLElement | ||
| if (target.closest('button') || target.closest('a')) return | ||
| emit('row:click', row.original) | ||
| props.onSelect?.(row.original) | ||
| if (props.enableRowSelection) { | ||
| row.toggleSelected() | ||
| emit('row:select', row.original) | ||
| } | ||
| } |
There was a problem hiding this comment.
The handleRowClick function signature changed to accept an event parameter, but the event is being passed from template using $event. The event parameter type is MouseEvent | KeyboardEvent, but the function accesses event.target assuming it exists. This could be improved by adding a type guard or properly handling both event types.
|
|
||
| **Current**: | ||
| ```vue | ||
| <div v-else-if="tab.content" class="p-4" v-html="tab.content" /> |
There was a problem hiding this comment.
The use of v-html="tab.content" directly renders arbitrary HTML content and creates a clear XSS risk if tab.content can contain or be influenced by user-generated input. An attacker could inject <script> tags or malicious event handlers into tab.content, which would execute in the context of your application and compromise user data or sessions. To mitigate this, avoid using v-html for untrusted data by switching to safe text rendering or, if HTML is strictly required, enforce robust server-side or well-reviewed client-side sanitization and constrain the allowed markup to a minimal, safe subset.
| <div v-else-if="tab.content" class="p-4" v-html="tab.content" /> | |
| <div v-else-if="tab.content" class="p-4" v-html="sanitizeHtml(tab.content)" /> |
No description provided.