Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the ReasoningContent and Response components to include a shikiTheme prop for the Streamdown component. Feedback suggests avoiding the spread of unrelated props to Streamdown in ReasoningContent to prevent React warnings and potential DOM attribute issues. Additionally, it is recommended to extract the shikiTheme array into a shared constant to avoid duplication and prevent unnecessary re-renders caused by inline array definitions in memoized components.
| {...props} | ||
| > | ||
| <Streamdown {...props}>{children}</Streamdown> | ||
| <Streamdown shikiTheme={['one-light', 'one-dark-pro']} {...props}>{children}</Streamdown> |
There was a problem hiding this comment.
Spreading props onto the Streamdown component is problematic here because props contains properties intended for CollapsibleContent (such as forceMount or asChild). Passing these unrelated props to Streamdown can lead to React warnings about invalid DOM attributes or unexpected behavior. Additionally, the shikiTheme array is defined inline within a memoized component, which creates a new reference on every render and can bypass memoization optimizations. Consider extracting the theme array to a constant.
| <Streamdown shikiTheme={['one-light', 'one-dark-pro']} {...props}>{children}</Streamdown> | |
| <Streamdown shikiTheme={['one-light', 'one-dark-pro']}>{children}</Streamdown> |
| export const Response = memo( | ||
| ({ className, ...props }: ResponseProps) => ( | ||
| <Streamdown className={cn('size-full [&>*:first-child]:mt-0 [&>*:last-child]:mb-0', className)} {...props} /> | ||
| <Streamdown className={cn('size-full [&>*:first-child]:mt-0 [&>*:last-child]:mb-0', className)} shikiTheme={['one-light', 'one-dark-pro']} {...props} /> |
There was a problem hiding this comment.
The shikiTheme configuration ['one-light', 'one-dark-pro'] is duplicated here and in reasoning.tsx. To improve maintainability and ensure consistency across the application, consider extracting this to a shared constant. Furthermore, defining the array inline in a memoized component creates a new reference on every render, which may cause unnecessary re-renders of the Streamdown component if it performs shallow prop comparisons.
Greptile SummaryThis PR fixes syntax highlighting in dark mode by passing
Confidence Score: 5/5Safe to merge — the change is a one-prop addition to two leaf components with no side effects beyond visual appearance. Both files receive a single well-typed prop addition. The theme strings are valid Shiki bundle themes. The Response component correctly allows callers to override the theme via spread props. In ReasoningContent the theme is not overridable from the outside, but that is a minor limitation rather than a defect. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Browser detects color-scheme] --> B{Light or Dark?}
B -- Light --> C[Shiki applies 'one-light' theme]
B -- Dark --> D[Shiki applies 'one-dark-pro' theme]
C --> E[Streamdown renders code blocks]
D --> E
E --> F[Response component]
E --> G[ReasoningContent component]
Reviews (1): Last reviewed commit: "fix: request detail preview in dark mode" | Re-trigger Greptile |
No description provided.