Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: Request changes — The current implementation does not fully meet the core path-parsing requirement.
Critical issues to fix (blockers):
- In
src/createServer.js, the path splitting logic (around the code usingpath.slice(1).split('/')) does not remove empty segments. This returns['']for/and leaves empty entries for repeated or trailing slashes (e.g.,/a//b,/a/b/), violating the requirement thatpartscontain only non-empty segments. Update this to split on'/'and filter out empties, e.g.,path.split('/')followed by filtering, so/yields[]and repeated/trailing slashes are ignored.
Minor improvements (non-blockers):
- When extracting the query,
req.url.split('?')can leavequeryStringundefined. Make intent explicit by defaulting to an empty string, e.g.,const [pathname, queryString = ''] = req.url.split('?'), and pass that toURLSearchParams. - Optionally add a short comment noting that
Object.fromEntries(new URLSearchParams(...).entries())will keep the last value for repeated keys, which is acceptable here.
You’ve done well setting status 200, Content-Type: application/json, and returning the required { parts, query } structure. With the path normalization fix, this should fully comply. Keep it up!
✨ 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/createServer.js
Outdated
| const normalizeParts = path.slice(1).startsWith('/') | ||
| ? path.slice(2).split('/') | ||
| : path.slice(1).split('/'); |
There was a problem hiding this comment.
Path splitting logic is incorrect and won't satisfy the requirement to ignore empty segments. The current code only attempts to handle a leading double slash and otherwise returns path.slice(1).split('/'), which yields [''] for the root path / and leaves empty strings from repeated or trailing slashes (e.g. /a//b or /a/b/). The task requires an array of non-empty segments (for / -> []). Consider splitting on '/' and filtering out empty strings (for example, path.split('/').filter(Boolean)) or otherwise ensuring empty segments are removed.
src/createServer.js
Outdated
| const [path, queryString] = req.url.split('?'); | ||
| const params = new URLSearchParams(queryString); |
There was a problem hiding this comment.
Using req.url.split('?') to split pathname and query works in many cases, but it can leave queryString undefined when there is no ?. Although new URLSearchParams(undefined) produces an empty set, it's clearer and safer to normalize the value (e.g. new URLSearchParams(queryString || '')) so the intent is explicit.
| ? path.slice(2).split('/') | ||
| : path.slice(1).split('/'); | ||
|
|
||
| const normalizeQuery = Object.fromEntries(params.entries()); |
There was a problem hiding this comment.
Object.fromEntries(params.entries()) produces a plain object with string values and if a parameter appears multiple times the later occurrence will overwrite earlier ones. This deterministic choice satisfies the requirement to map keys to string values (not arrays). If this behavior is intended, consider adding a brief comment to make that explicit for maintainers/tests.
| res.statusCode = 200; | ||
| res.statusMessage = 'OK'; | ||
| res.setHeader('Content-Type', 'application/json'); |
There was a problem hiding this comment.
Good: response status and Content-Type are set properly (200 OK and Content-Type: application/json), which meets the requirement that responses be JSON with a 200 status code.
| JSON.stringify({ | ||
| parts: normalizeParts, | ||
| query: normalizeQuery, |
There was a problem hiding this comment.
Good: the response body is a single JSON object with exactly two top-level properties parts and query. Once parts is computed by filtering out empty segments, this will match the specified structure exactly.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Requesting changes: the core requirement to return decoded pathname segments is not met. In src/createServer.js, the parts array is built via path.split('/').filter(Boolean) but each segment is not URL-decoded, so paths like /a%20b yield ['a%20b'] instead of ['a b']. This violates the requirement to provide decoded path segments and is a functional correctness issue.
What to fix (critical):
- Decode each non-empty segment before returning
parts. For example, map segments throughdecodeURIComponentwith a try/catch to avoid crashes on malformed encodings (e.g.,seg => { try { return decodeURIComponent(seg); } catch { return seg; } }).
What you did well:
- Correctly default the query string to empty and use
URLSearchParams, producing decoded, string values and{}for empty queries. The JSON shape{ parts, query }and slash handling are correct.
Minor (non-blocking):
- Add a brief comment noting that
Object.fromEntries(params.entries())keeps the last value for repeated query keys. Consider a small note explainingfilter(Boolean)usage. Nice work overall—just address the decoding to meet the spec.
✨ 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
| /* Write your code here */ | ||
| // Return instance of http.Server class | ||
| const server = http.createServer((req, res) => { | ||
| const [path, queryString = ''] = req.url.split('?'); |
There was a problem hiding this comment.
Good: you default the query string to an empty string when splitting req.url, which avoids passing undefined into URLSearchParams and matches the suggestion in the task notes. This ensures an empty query returns {}. (See requirement: handle empty query string.)
| // Return instance of http.Server class | ||
| const server = http.createServer((req, res) => { | ||
| const [path, queryString = ''] = req.url.split('?'); | ||
| const params = new URLSearchParams(queryString); |
There was a problem hiding this comment.
Good: using new URLSearchParams(queryString) ensures query keys/values are decoded and represented as strings, satisfying the requirement that query values are strings and decoded. (URLSearchParams returns strings for values.)
src/createServer.js
Outdated
| const [path, queryString = ''] = req.url.split('?'); | ||
| const params = new URLSearchParams(queryString); | ||
|
|
||
| const normalizeParts = path.split('/').filter((ch) => ch); |
There was a problem hiding this comment.
Critical: the parts array is produced by path.split('/').filter((ch) => ch) but the segments are not URL-decoded. The task requires decoded path segments (e.g. /a%20b should yield "a b"). Consider mapping the filtered segments through decodeURIComponent and guarding against malformed encodings. For example (conceptual):
const normalizeParts = path
.split('/')
.filter(Boolean)
.map(seg => {
try { return decodeURIComponent(seg); }
catch (e) { return seg; }
});Wrap decoding in try/catch so invalid percent-encodings don't crash the server. This is required before the PR can be accepted.
|
|
||
| const normalizeParts = path.split('/').filter((ch) => ch); | ||
|
|
||
| const normalizeQuery = Object.fromEntries(params.entries()); |
There was a problem hiding this comment.
Minor suggestion — Object.fromEntries(params.entries()) deterministically keeps the last value when a query key appears multiple times. That's acceptable per the task, but please add a short comment in the code to document that behavior (so readers know repeated keys will yield the last occurrence).
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Educational approval to help you progress. You did a solid job on query parsing with URLSearchParams and returning the correct JSON shape with proper headers—nice work. However, there’s a critical issue on lines 13–19: using decodeURIComponent inside a filter callback doesn’t replace segments, so percent-encoded parts (e.g., /a%20b) remain encoded, violating the requirement to return decoded pathname segments. This happens because filter expects a boolean and doesn’t transform items.
- Fix hint (lines 13–19): split the path, remove empty segments, then map to decoded strings:
.split('/').filter(Boolean)to ignore doubled/leading/trailing slashes.map(seg => { try { return decodeURIComponent(seg); } catch { return seg; } })
Minor note: your approach to splitting on ? is acceptable here; consider new URL(req.url, 'http://example') for more robust parsing in the future. Overall, you’re very close—just adjust the pathname decoding and you’re set. Great progress!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| const normalizeParts = path.split('/').filter((ch) => { | ||
| try { | ||
| return decodeURIComponent(ch); | ||
| } catch (error) { | ||
| return ch; | ||
| } | ||
| }); |
There was a problem hiding this comment.
The code building normalizeParts uses filter incorrectly. A filter callback should return a boolean to decide which items to keep; returning the decoded string only influences truthiness but does not replace the original segment with its decoded value. This means parts will contain the original (possibly percent-encoded) segments, not decoded ones — violating the requirement to return decoded pathname parts.
Please change the logic to: split the path, remove empty segments (to ignore doubled/leading/trailing slashes), then map each segment through decodeURIComponent in a try/catch. For example:
const parts = path
.split('/')
.filter(Boolean)
.map(seg => {
try { return decodeURIComponent(seg); } catch { return seg; }
});Update the code on lines 13–19 so parts contains decoded strings and empty/doubled slashes are ignored.
| const params = new URLSearchParams(queryString); | ||
|
|
||
| const normalizeParts = path.split('/').filter((ch) => { | ||
| try { | ||
| return decodeURIComponent(ch); | ||
| } catch (error) { | ||
| return ch; | ||
| } | ||
| }); | ||
|
|
||
| const normalizeQuery = Object.fromEntries(params.entries()); |
There was a problem hiding this comment.
Query parsing is implemented correctly: new URLSearchParams(queryString) together with Object.fromEntries(params.entries()) produces an object whose values are strings (no type coercion), which meets the requirement. Note: Object.fromEntries(params.entries()) will keep the last value when a key appears multiple times; this is fine for the task but worth knowing.
| res.statusCode = 200; | ||
| res.statusMessage = 'OK'; | ||
| res.setHeader('Content-Type', 'application/json'); | ||
|
|
||
| res.end( | ||
| JSON.stringify({ | ||
| parts: normalizeParts, | ||
| query: normalizeQuery, | ||
| }), |
There was a problem hiding this comment.
Response formatting is correct: the server sets status code/message, sets Content-Type: application/json, and returns JSON.stringify({ parts, query }), matching the required JSON shape and property names.
| /* Write your code here */ | ||
| // Return instance of http.Server class | ||
| const server = http.createServer((req, res) => { | ||
| const [path, queryString = ''] = req.url.split('?'); |
There was a problem hiding this comment.
Small note about splitting req.url on ? (line 10): this is acceptable here because you only need pathname and query. If you ever need more robust parsing (e.g. base URL resolution), consider using the WHATWG URL constructor: new URL(req.url, 'http://example'). Not required to change for this task, just a suggestion.
| ); | ||
| }); | ||
|
|
||
| return server; |
There was a problem hiding this comment.
Return of the server instance (line 35) and module export (lines 38–39) are fine — the function returns an http.Server instance as required.
No description provided.