Skip to content

solution#370

Open
tiblazy wants to merge 1 commit intomate-academy:masterfrom
tiblazy:master
Open

solution#370
tiblazy wants to merge 1 commit intomate-academy:masterfrom
tiblazy:master

Conversation

@tiblazy
Copy link
Copy Markdown

@tiblazy tiblazy commented Mar 7, 2025

No description provided.

Copy link
Copy Markdown

@raulriato raulriato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've done a good job. Just need a few changes

@@ -1,9 +1,24 @@
/* eslint-disable no-console */
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not good to disable the eslint rules on your project.

/* Write your code here */
// Return instance of http.Server class
return http.createServer((req, res) => {
const url = new URL(req.url, `http://${req.headers.host}`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shoul use a try/catch to prevent your app from crashing if the new Url() doesn't work as expected


const urlParams = {
parts: url.pathname
? url.pathname.split('/').filter((pathname) => pathname !== '')
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this ternary operator, since the pathname is always truthy in Url class

? url.pathname.split('/').filter((pathname) => pathname !== '')
: [],
query: url.searchParams
? Object.fromEntries(url.searchParams.entries())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this ternary operator, since the searchParams is always truthy in Url class

: {},
};

res.setHeader('Content-Type', 'application/json');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code does not specify the status code for the response. Even though the default status code is 200, you should make it explicit here.

res.writeHead(200, { 'Content-Type': 'application/json' });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants