Conversation
freelinex
commented
Feb 14, 2026
freelinex
commented
- DEMO LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical, blocking issues in src/scripts/main.js that can break functionality and automated tests, even though the core recursion and DOM-building logic are correct. First, the top-level DOM selection and immediate invocation (const tree = document.querySelector('#tree'); and createTree(tree, food); around lines 21 and 44) will throw in test/headless environments where document or #tree are not available; this needs to be moved behind a browser-only guard or into a separate entry file so the module can be safely imported. Second, createTree is not exported, which prevents the test harness from importing and calling it directly; you should export the function (per the project’s module pattern) so tests can verify its behavior.
The following are important but non-blocking improvements you should also address while you’re editing: avoid using for...in over data and instead iterate Object.keys(data) (or check hasOwnProperty) so inherited properties don’t leak into your tree, and add a small guard that validates element before calling append to produce clearer errors. Additionally, consider tightening the object-type check so arrays are handled explicitly (e.g., !Array.isArray(data[key])) and skip creating/appending an empty <ul> when an object has no own keys, to avoid unnecessary DOM nodes. Overall, your recursion and tree-building logic look good; once you isolate the DOM side effects, export the function, and harden the iteration/guards, this will be in great shape.
✨ 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
src/scripts/main.js
Outdated
| // WRITE YOUR CODE HERE | ||
| const ul = document.createElement('ul'); | ||
|
|
||
| for (const key in data) { |
There was a problem hiding this comment.
Using for...in can iterate inherited properties. Prefer iterating the object's own keys to avoid unexpected items, e.g. for (const key of Object.keys(data)) { ... } or check Object.prototype.hasOwnProperty.call(data, key).
src/scripts/main.js
Outdated
| if ( | ||
| typeof data[key] === 'object' && | ||
| data[key] !== null && | ||
| Object.keys(data[key]).length > 0 |
There was a problem hiding this comment.
The condition checks that data[key] is a non-null object with keys — that's fine, but consider simplifying/clarifying the guard and being explicit about object type. Note that typeof [] === 'object' (arrays qualify) — if arrays aren't expected, you may want to ensure !Array.isArray(data[key]) as well.
src/scripts/main.js
Outdated
| data[key] !== null && | ||
| Object.keys(data[key]).length > 0 | ||
| ) { | ||
| createTree(li, data[key]); |
There was a problem hiding this comment.
Recursive call createTree(li, data[key]); correctly implements recursion (satisfies the "Hint: use Recursion" requirement). Ensure recursion termination is guaranteed by the guard above (non-empty object check).
| ul.append(li); | ||
| } | ||
|
|
||
| element.append(ul); |
There was a problem hiding this comment.
Appending to element assumes element is a valid DOM node. Add a light validation or guard (e.g., if (!element || typeof element.append !== 'function') throw new Error('Invalid element')) to produce clearer errors when misused or when DOM node is missing.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one explicit task requirement is still not met, even though your core implementation is solid. Your createTree(element, data) function in src/scripts/main.js satisfies the functional requirements: it’s recursive, validates inputs, builds the correct nested ul/li structure from object keys, and is safely exportable and guarded for non-DOM environments. However, the project checklist also requires that the demo link in the description/README be updated from the placeholder https://<your_account>.github.io/js_tree-from-object-DOM/ to your actual GitHub Pages URL, and this is currently not done; that makes the task formally incomplete.
Critical issue to fix before this can be completed:
- Update the README/description demo URL: replace
<your_account>with your GitHub username (for example,https://your-username.github.io/js_tree-from-object-DOM/) and ensure the page is actually published and accessible.
Minor suggestions (non-blocking once the above is fixed): you may consider adjusting the behavior on invalid elements (throw vs. fail quietly) if your tests indicate a different expectation, and optionally add an ES module export for createTree for more flexibility. Once you correct the demo link, your solution will fully satisfy both the functional and checklist requirements—nice work on the DOM tree logic and environment guards.
✨ 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
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
3 participants
