fix(patch): cherry-pick 44cdb3e to release/v0.36.0-preview.6-pr-24235 to patch version v0.36.0-preview.6 and create version 0.36.0-preview.7#24265
Conversation
gemini-cli
Bot
added
the
status/need-issue
|
Size Change: -1.56 kB (-0.01%) Total Size: 26.2 MB
ℹ️ View Unchanged
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the console message handling system to use a global store instead of component-local state. This change ensures that console messages and error counts are maintained independently of the UI component lifecycle, improving data consistency and simplifying state management across the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the console message management from a component-local state to a global store using useSyncExternalStore. This change ensures that message history and error counts are preserved even when UI components are unmounted. Key additions include an initializeConsoleStore function for setup and a batching mechanism to handle high-frequency log updates. Review feedback suggests addressing a potential ReferenceError caused by the Temporal Dead Zone in the global store initialization and ensuring that clearing the error count also accounts for pending messages in the processing queue.
| export function initializeConsoleStore() { | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| timeoutId = null; | ||
| } | ||
| messageQueue = []; | ||
| globalConsoleMessages = []; | ||
| globalErrorCount = 0; | ||
| notifyListeners(); | ||
|
|
||
| // Safely detach first to ensure idempotency and prevent listener leaks | ||
| coreEvents.off(CoreEvent.ConsoleLog, handleConsoleLog); | ||
| coreEvents.off(CoreEvent.Output, handleOutput); | ||
|
|
||
| coreEvents.on(CoreEvent.ConsoleLog, handleConsoleLog); | ||
| coreEvents.on(CoreEvent.Output, handleOutput); | ||
| } |
There was a problem hiding this comment.
The initializeConsoleStore function references handleConsoleLog and handleOutput, which are defined later in the file using const. While this is safe when called from other modules (like gemini.tsx) after this module has finished evaluating, it could trigger a ReferenceError due to the Temporal Dead Zone if it were ever called during module initialization. For better robustness and to follow standard hoisting expectations, consider using function declarations for the event handlers or moving their definitions above initializeConsoleStore.
| const clearErrorCount = useCallback(() => { | ||
| startTransition(() => { | ||
| dispatch('CLEAR'); | ||
| }); | ||
| globalErrorCount = 0; | ||
| notifyListeners(); | ||
| }, []); |
There was a problem hiding this comment.
The clearErrorCount function resets the globalErrorCount but does not clear the messageQueue. If there are pending error messages in the queue when clearErrorCount is called, they will be processed in the next processQueue execution and immediately increment the count again. Consider clearing the messageQueue or filtering out errors from it when resetting the count to ensure a consistent user experience.
|
This item has been automatically marked as stale due to 60 days of inactivity. It will be closed in 14 days if no further activity occurs. Thank you! |
4 participants
This PR automatically cherry-picks commit 44cdb3e to patch version v0.36.0-preview.6 in the preview release to create version 0.36.0-preview.7.