Refactor (packages/ui/src/v2/components/line-comment-v2.tsx): Function with high complexity - #61
Open
riyakadakia wants to merge 3 commits into
Open
Refactor (packages/ui/src/v2/components/line-comment-v2.tsx): Function with high complexity#61riyakadakia wants to merge 3 commits into
riyakadakia wants to merge 3 commits into
Conversation
…ndling logic from the main component into reusable modules. - Extract mention state management into useMentionHandler custom hook, removing complex state logic from the component and making it reusable across other components that need mention functionality. - Extract pure utility functions (extractMentionQuery, insertMention) into mention-utils.ts, separating business logic from UI concerns and making the logic easier to test and maintain. - Simplify LineCommentEditorV2 component from ~200 lines to ~100 lines, focusing it solely on rendering and event handling with no impact to public API or component behavior.
Add 7 new unit tests to mention-utils.test.ts covering mention extraction, insertion, and cursor positioning logic. Achieve 100% code coverage on the utilities module, ensuring the core refactored logic works correctly and preventing future regressions.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
P1B: Starter Task: Refactoring PR
Use this pull request template to briefly answer the questions below in one to two sentences each.
Feel free to delete this text at the top after filling out the template.
1. Issue
Link to the associated GitHub issue:
#57
Full path to the refactored file:
packages/ui/src/v2/components/line-comment-v2.tsx
What do you think this file does?
I think this file is used as a comment input form for adding comments to specific lines of code. I think this is the case because there is a textarea section, submit and cancel buttons as well as keyboard shortcuts including Enter (which submits comment), escape and arrow keys for navigation.
What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
I refactored the mention state components and mention helper functions and moved them into a new file to simplify the file. I specifically moved closeMention, currentMention, selectMention, selectActiveMention, and syncMention functions into a helper file called useMentionhandler.ts.
Which Qlty‑reported issue did you address?
I addressed the Qlty-reported issue of Function with high complexity (Count = 55) in LineCommentEditorV2.
2. Refactoring
How did the specific issue you chose impact the codebase’s maintainability?
The specific issue I choose was a function with high complexity which made it hard to maintain the codebase for several reasons. First since the logic had so many lines of code all baked into the return it was difficult to understand. Secondly, it was hard to test because it had so many condiditional branches and state interactions. Since the mention handling logic was all within the component is also reduced the reusability of the code.
What changes did you make to resolve the issue?
The changes I made to resolved these maintainability issues included refactoring the component by moving the mention handling logic into a separate file. I created two new files called useMentionHandler.ts which was a custom hook that encapsulates all the mention state management including the mentionOpen functions and more. Secondly, I created another file called mention-utils.ts which has all the utility functions including extractMentionQuery and insertMention. These function don't have dependencies so separating them into another file makes them much more easily testable. Using these two files I simplified the original line-comment-v2.tsx file.
How do your changes improve maintainability? Did you consider alternatives?
My changes separate the important functions of the code and encapsulates the functions better so now each component has a single responsibility instead of combining a lot of different functionality into one. My changes also make the functions more testable as previously there weren't tests for this component as it needed a complex setup. Now that the code is broken up into neater components is a lot easier to read and reuse as well. I actually did first try to move the functions out of the return and encapsulate them within the same file but when I tested the qlty smells again it reduced the complexity from 55 to 35 but it seems like to get the complexity down even more I needed to refactor the code into different components in different files. Doing this got the qlty smell to completely vanish.
3. Validation
How did you validate that the change is correct?
I ran the existing tests and all 27 existing tests still passed proving the the componenet's behavior remained unchanged. I additionally added 7 new comprehensive unit tests to the mention-utils.test.ts which covers mention query extraction, mention insertion and tests edge cases. Running these additional tests I achieved a 100% code coverage on mention-utils.ts. I also ran qlty smells again and the entire code smell I had worked on no longer showed up.
Attach a screenshot of the test coverage showing the lines were executed by the tests.
The red files are files I did not touch.
Attach a screenshot showing the tests that cover the change passing during CI
Attach a screenshot of
qlty smells --no-snippets <full/path/to/file.ts>showing fewer reported issues after the changes.