Cleaned up the server.js file | Issue with If statement. line 43

[Osintedx]

Short Explanation of Changes: the If statement shouldn't be the only statement in the else block on line 43, its bad code practice.

New Code:

import http from 'http';
import fs from 'fs';
import path from 'path';

const root = process.env.ROOT || '.';
const userHeaders = JSON.parse(process.env.HEAD || '{}');

const mimeLookup = new Map([
  ['.css',  'text/css'],
  ['.html', 'text/html'],
  ['.ico',  'image/x-icon'],
  ['.js',   'application/javascript'],
  ['.json', 'application/json'],
  ['.md',   'text/markdown'],
  ['.png',  'image/png'],
  ['.txt',  'text/plain'],
]);

class Server {
  static origin;

  static requestListener(request, response) {
    if (request.method !== 'GET') {
      response.writeHead(405, { 'Content-Type': 'text/plain', ...userHeaders });
      response.write('Method Not Allowed');
      response.end();
      return;
    }

    const url = new URL(request.url, Server.origin);
    const filePath = path.resolve(`${root}${url.pathname}`);
    const fileExt = path.extname(filePath);
    const mimeType = mimeLookup.get(fileExt);

    if (fileExt) {
      if (mimeType) {
        fs.stat(filePath, (error, stats) => {
          if (error || !stats.isFile()) {
            Server.sendFileNotFound(response);
          } else {
            Server.sendFile(response, filePath, mimeType, stats.size);
          }
        });
      } else {
        Server.sendUnknownMimeType(response, fileExt);
      }
    } else if (url.pathname.endsWith('/')) {
      const directoryIndex = path.join(filePath, 'index.html');
      fs.stat(directoryIndex, (error, stats) => {
        if (error || !stats.isFile()) {
          // @TODO Implement pushState functionality for better routing
          Server.sendRootIndex(response);
        } else {
          Server.sendFile(response, directoryIndex, 'text/html', stats.size);
        }
      });
    } else {
      Server.sendRedirect(response, `${url.pathname}/`);
    }
  }

  static sendUnknownMimeType(response, fileExt) {
    const message = `Error 500: Unknown MIME type for file extension: ${fileExt}`;
    response.writeHead(500, { 'Content-Type': 'text/plain', 'Content-Length': message.length, ...userHeaders });
    response.write(message);
    response.end();
  }

  static sendFileNotFound(response) {
    const message = 'Error 404: Resource not found.';
    response.writeHead(404, { 'Content-Type': 'text/plain', 'Content-Length': message.length, ...userHeaders });
    response.write(message);
    response.end();
  }

  static sendRedirect(response, location) {
    response.writeHead(301, { 'Content-Type': 'text/plain', 'Content-Length': 0, Location: location, ...userHeaders });
    response.end();
  }

  static sendFile(response, filePath, mimeType, contentLength) {
    response.writeHead(200, { 'Content-Type': mimeType, 'Content-Length': contentLength, ...userHeaders });
    fs.createReadStream(filePath).pipe(response);
  }

  static sendRootIndex(response) {
    const rootIndex = path.join(root, 'index.html');
    fs.stat(rootIndex, (error, stats) => {
      if (error || !stats.isFile()) {
        Server.sendFileNotFound(response);
      } else {
        Server.sendFile(response, rootIndex, 'text/html', stats.size);
      }
    });
  }
}

const server = http.createServer(Server.requestListener);

server.listen(process.env.PORT || 8080, () => {
  const { address, port } = server.address();
  Server.origin = `http://${address}:${port}`;
  console.log(`Development server running: ${Server.origin}`); // eslint-disable-line no-console
});

[Osintedx]

This was just a small fun fixture for netflix to review, and repair as obviously someone was in a rush.

[klebba]

@Osintedx thanks for your contribution. Can you please open a pull request with your proposed changes?

[Osintedx]

Hey @klebba, I dont have any permissions to do so.

[klebba]

You'll need to create a fork, commit changes on a branch, then make a pull request to the origin from your forked branch:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

Also I took a closer look at your suggestion above; are you sure the change does what you think? From what I can tell it would modify the conditional flow; however a diff would be easier to comment on.

Some curiosities of mine: how did you come across this library? What are you using it for? Could inform how we can help you -- thanks!

[Osintedx]

Hey there @klebba I have gone ahead and done it.

[klebba]

Thanks @Osintedx — regrettably we have not published any contributing guidelines yet, but I have opened #186 to capture the need and include an initial outline. In the interim my feedback is that the changes proposed in #185 do not appear to address an issue with the x-element library. Thank you for your interest, however at this time we would not be able to review an un-scoped refactor like this. If there is a bug please scope your change to address that bug.