Solution#1771
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the current createTree base-case logic can break the intended behavior for leaf nodes and may throw for non-object data, which makes the solution not fully reliable yet. In src/scripts/main.js, the early return when Object.keys(data).length === 0 runs before any appending happens, so when createTree is called on a li for a leaf node, the parent li can end up never being integrated into the final tree structure. You should restructure the base case so that you only skip creating/appending a nested ul when data is empty, but still allow the existing li/parent to be appended to the tree (for example, create the li, append it, and then conditionally skip adding a child ul if there are no keys). In addition, at the top of createTree you should guard against non-object or null/undefined values (e.g., if (!data || typeof data !== 'object' || Object.keys(data).length === 0) { /* handle or return appropriately */ }) so that future changes or malformed input don’t cause a runtime error. Once you adjust the base case and add a safe type/null check, your recursive structure and overall implementation will match the task requirements and be more robust. Your recursion, DOM wiring, and use of keys as <li> text are all on the right track, so you’re very close—these fixes will solidify the correctness of your solution.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| if (Object.keys(data).length === 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The early return for empty data stops the function before any appending happens, which is fine for the root but also prevents proper handling of leaf nodes higher up. Consider restructuring the base case so that you skip creating/appending a nested ul when data is empty, but still allow the existing li/parent to be part of the tree.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a couple of core checklist requirements are still not met, even though your main tree-building logic and Parcel/SCSS setup look good overall. First, the task explicitly requires updating the GitHub Pages demo link by replacing <your_account> with your actual GitHub username; this link is currently missing or not updated anywhere in the project, so the requirement [CHECKLIST ITEM #5] is not satisfied. Second, inside src/scripts/main.js, your createTree function calls Object.keys(data) without a defensive guard; you should add something like if (!data || typeof data !== 'object') return; at the very top of the function to prevent future runtime errors when data might be null, undefined, or non-object [CHECKLIST ITEM #12]. Related to that, your base-case/leaf-node handling still hinges on Object.keys(data).length > 0 for all behavior; instead, create and append the li unconditionally, and only conditionally create/append a nested ul when data is a non-empty object, so leaf nodes are handled cleanly and the recursion is focused on child lists [CHECKLIST ITEM #10] and [CHECKLIST ITEM #11]. Once you add the safety check, adjust the base-case structure as described, and update the GitHub demo link to use your actual username, your solution will fully align with the assignment requirements. Keep the existing recursive structure and DOM wiring—they are very close; you just need these small but important fixes to make the implementation robust and complete.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you learn. Your recursive createTree implementation in src/scripts/main.js is structurally solid: you always create li elements from the object keys, only add nested ul elements when there are child keys, and you included a runtime guard to avoid crashes for invalid data inputs, which aligns well with the task’s robustness requirements. One important improvement is to make sure your GitHub Pages DEMO LINK in the project documentation (for example, README or description file) uses your real GitHub username instead of a placeholder like <your_account>, so it matches the required format [DEMO LINK](https://<your-username>.github.io/js_tree-from-object-DOM/) [CHECKLIST ITEM]. Also, if you ever extend the task to support primitive leaf values (like strings or numbers instead of {}), consider adjusting the guard so that li elements are still created even when data is non-object, and only the recursive step is skipped. Overall, your code is clean, readable, and meets the core behavior for building the DOM tree; with these small documentation and future-proofing tweaks, it would be fully aligned with all expectations. Keep up the good work—this is a strong implementation and a solid step in mastering recursion and DOM manipulation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| if (!data || typeof data !== 'object') { | ||
| return; |
There was a problem hiding this comment.
The runtime guard for data being null/undefined/non-object is in place and prevents Object.keys/iteration errors, which aligns with the checklist requirement to avoid crashes for invalid input.
|
|
||
| li.textContent = key; | ||
|
|
||
| createTree(li, data[key]); |
There was a problem hiding this comment.
Your recursive call createTree(li, data[key]) correctly treats each li as the parent container for potential child lists, so nested ul elements are only added when there is deeper data, matching the desired leaf-node behavior.
| if (ul.children.length > 0) { | ||
| element.append(ul); |
There was a problem hiding this comment.
Appending the ul only when it has children ensures you don’t create empty lists for leaf nodes; this satisfies the requirement that li elements always exist while nested ul elements are conditional on having child keys.
DEMO LINK