feat(autofix-result): refactor logstream handling with modifier #3188
+59
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Bundle ReportChanges will decrease total bundle size by 2.41kB (-0.01%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: client-array-pushAssets Changed:
Files in
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
rohitpaulk
force-pushed
the
refactor-autofixresult-logstream-handling
branch
from
68646db to
06a96d1
Compare
Replace manual logstream management in AutofixResult component with a new `logstream-did-update` modifier that handles subscription lifecycle automatically. This simplifies the component by removing explicit setup, teardown, and state syncing of Logstream instances. Update the template to use the modifier for logstream updates and track logstream content reactively. This ensures better separation of concerns and reduces boilerplate. Add `logstream-did-update` modifier that encapsulates subscribing and unsubscribing to logstreams and invokes a callback on updates. Overall, these changes improve code clarity, correctness, and maintainability around logstream update handling.
Eliminate early return on isSubscribed in subscribeTask to allow resubscription logic to proceed. This change fixes an issue where the subscription could not be re-established once terminated, improving reliability of the log stream handling.
Register a destructor for the modifier to ensure the Logstream is properly unsubscribed when the modifier is destroyed. Replace ad-hoc unsubscribe logic in modify() with a dedicated cleanup function. Use a lambda callback to invoke the provided callback with the current logstream, removing unnecessary stored callback references and improving code clarity.
Remove the unused `action` import from the logstream-did-update modifier to clean up the code and avoid unnecessary dependencies.
Set the logstream property to undefined during cleanup to prevent potential memory leaks or stale references. Also, adjust the callback invocation in modify for clearer structure while keeping the same behavior.
rohitpaulk
force-pushed
the
refactor-autofixresult-logstream-handling
branch
from
06a96d1 to
521128d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace manual logstream management in AutofixResult component with a
new
logstream-did-updatemodifier that handles subscription lifecycleautomatically. This simplifies the component by removing explicit setup,
teardown, and state syncing of Logstream instances.
Update the template to use the modifier for logstream updates and track
logstream content reactively. This ensures better separation of concerns
and reduces boilerplate.
Add
logstream-did-updatemodifier that encapsulates subscribing andunsubscribing to logstreams and invokes a callback on updates.
Overall, these changes improve code clarity, correctness, and maintainability
around logstream update handling.
Note
Refactors
AutofixResultto use a newlogstream-did-updatemodifier for streaming logs and adjustsLogstreamsubscription behavior.AutofixResultcomponent: Replaces manualLogstreamlifecycle with{{logstream-did-update ...}}; switches fromthis.logstreamto trackedthis.logstreamContent; reloads@autofixRequeston updates; simplifies template accordingly.app/modifiers/logstream-did-update.ts: New modifier that subscribes/unsubscribes toLogstream, invokes a callback on updates, and cleans up via destructor.Logstream(app/utils/logstream.ts): TweakssubscribeTaskby removing the earlyisSubscribedguard.Written by Cursor Bugbot for commit 521128d. This will update automatically on new commits. Configure here.