Include all nodes in tree.toJSInfoForTest #832
Conversation
WalkthroughThe change in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (7)
src/document/crdt/tree.ts (7)
Line range hint
151-151: Specify explicit types to avoid implicit 'any'.Consider specifying explicit types for variables to enhance type safety and code maintainability.
Line range hint
154-154: Avoid non-null assertions.Non-null assertions may lead to runtime errors if not handled properly. Consider adding necessary checks or refactor the code to ensure the objects are not null before accessing their properties.
Also applies to: 155-155, 160-160, 242-242, 547-547, 550-550, 591-591, 596-596, 890-890, 891-891, 896-896, 974-974, 1039-1039
Line range hint
326-328: Simplify control flow by removing unnecessary else clause.- else { - return; - }Removing the unnecessary else clause simplifies the control flow and enhances code readability.
Line range hint
452-452: Avoid assignments within expressions.Assignments within expressions can lead to unclear code. Consider separating the assignment from the expression for better readability and maintainability.
Line range hint
722-722: Use optional chaining for safer property access.- if (node.parent && !node.parent.removedAt) { + if (node.parent?.removedAt) {Using optional chaining (
?.) ensures that the code does not throw ifnode.parentis undefined.
Line range hint
724-735: Prefer template literals over string concatenation.- return '<' + node.type + attrs + '>' + node.children.map(toXML).join('') + '</' + node.type + '>'; + return `<${node.type}${attrs}>${node.children.map(toXML).join('')}</${node.type}>`;Using template literals improves readability and maintainability of the code by reducing complexity and potential errors in string concatenation.
Line range hint
877-877: Specify a type instead of using 'any'.Using 'any' reduces the benefits of TypeScript's type system. Specify a more precise type to enhance code quality and error-checking capabilities.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/document/crdt/tree.ts (1 hunks)
Additional Context Used
Biome (20)
src/document/crdt/tree.ts (20)
151-151: This variable implicitly has the any type.
154-154: Forbidden non-null assertion.
155-155: Forbidden non-null assertion.
160-160: Forbidden non-null assertion.
242-242: Forbidden non-null assertion.
326-328: This else clause can be omitted because previous branches break early.
452-452: The assignment should not be in an expression.
547-547: Forbidden non-null assertion.
550-550: Forbidden non-null assertion.
591-591: Forbidden non-null assertion.
596-596: Forbidden non-null assertion.
722-722: Change to an optional chain.
724-735: Template literals are preferred over string concatenation.
877-877: Unexpected any. Specify a different type.
890-890: Forbidden non-null assertion.
891-891: Forbidden non-null assertion.
891-891: Forbidden non-null assertion.
896-896: Forbidden non-null assertion.
974-974: Forbidden non-null assertion.
1039-1039: Forbidden non-null assertion.
Additional comments not posted (1)
src/document/crdt/tree.ts (1)
1359-1362: Ensure correct iteration over all children nodes.The modification to iterate over
node.allChildreninstead ofnode.childrenensures that all child nodes are included in the output, aligning with the PR's objective. This change should be reflected in the unit tests to verify the new behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #832 +/- ##
=======================================
Coverage 80.46% 80.46%
=======================================
Files 59 59
Lines 4535 4535
Branches 918 918
=======================================
Hits 3649 3649
Misses 619 619
Partials 267 267 ☔ View full report in Codecov by Sentry. |
hackerwins
changed the title
What this PR does / why we need it?
Modify tree.toJSInfoForTest to include all children nodes to display deleted nodes in devtools.
Any background context you want to provide?
What are the relevant tickets?
Fixes #
Checklist
Summary by CodeRabbit