Interface for getting Github PRs commenters using Github API

By Posted on

Problem

I build this silly app to get the commenters’ information on some PR, mostly for learning how to use API’s and for improving my JS dom skills.

Although it is working, it’s mostly spaghetti code and very disorganized, so I am sure there is a lot of room for improvement.

I would like to get some feedback on how to better organize my code and structure it so it can easily grow and can be more maintainable.

This is the JS file (probably need to split into modules/classes, but don’t know where to start):

function createElement(type, options = {}) {
  const element = document.createElement(type)
  Object.keys(options).forEach(option => {
    element[option] = options[option]
  })
  return element
}

function createUser(user) {
  const userHtml = {
    avatarUrl: createElement('img', {
      src: user.avatar_url,
    }),
    login: createElement('a', {
      href: user.html_url,
      textContent: user.login,
    }),
  }
  if (user.name) {
    const name = createElement('div', {
      textContent: user.name,
    })
    userHtml.name = name
    userHtml.name.classList.add('name')
  }
  userHtml.avatarUrl.classList.add('avatar')
  userHtml.login.classList.add('login')
  return userHtml
}

function renderPullRequestInfo(pullRequest) {
  const { login } = createUser(pullRequest.user)
  const title = createElement('a', {
    id: 'title',
    href: pullRequest.html_url,
    textContent: pullRequest.title,
  })
  document.querySelector('#pull-request-info').append(title, login)
}

function renderUsers(users) {
  const fragment = document.createDocumentFragment()
  const title = document.createElement('h3')
  if (users.length === 0) {
    title.textContent = 'No reviewers found :(.'
  } else {
    title.textContent = 'Reviewers:'
    users.forEach(user => {
      if (user.name) {
        const li = createElement('li')
        const { avatarUrl, login, name } = createUser(user)
        li.append(avatarUrl, login, name)
        fragment.appendChild(li)
      }
    })
    const ul = document.querySelector('#user-list')
    ul.before(title)
    ul.appendChild(fragment)
  }
}

async function fetchGithubApi(endpoint) {
  const githubApiUrl = 'https://api.github.com'
  const response = await fetch(
    endpoint.startsWith(githubApiUrl) ? endpoint : `${githubApiUrl}${endpoint}`,
    {
      headers: { Accept: 'application/vnd.github.v3+json' },
    }
  )
  const data = await response.json()
  return data
}

async function getPullRequestData(event) {
  event.preventDefault()
  document.querySelector('#pull-request-url-messages').textContent = ''
  document.querySelector('#user-list').innerHTML = ''
  document.querySelector('#pull-request-info').innerHTML = ''
  try {
    const pullRequestUrl = new URL(
      document.querySelector('#pull-request-url').value
    )
    const pullRequestPath = `/repos${pullRequestUrl.pathname.replace(
      //pull//,
      '/pulls/'
    )}`
    const pullRequest = await fetchGithubApi(pullRequestPath)
    renderPullRequestInfo(pullRequest)
    if (pullRequest.state === 'open') {
      const allComments = await Promise.all([
        fetchGithubApi(pullRequest.review_comments_url),
        fetchGithubApi(pullRequest.comments_url),
      ])

      const uniqueUsersUrls = [
        ...new Set(
          allComments
            .flat()
            .filter(comment => comment.user.login !== pullRequest.user.login)
            .map(comment => comment.user.url)
        ),
      ]
      const users = await Promise.all(
        uniqueUsersUrls.map(uniqueUserUrl => fetchGithubApi(uniqueUserUrl))
      )
      renderUsers(users)
    } else {
      document.querySelector('#pull-request-url-messages').textContent =
        'The pull request is already closed!'
    }
  } catch (e) {
    document.querySelector('#pull-request-url-messages').textContent = e
  }
}

document
  .querySelector('#pull-request-form')
  .addEventListener('submit', getPullRequestData)

And this is the index.html:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
    <title>Pull Request Reviewers</title>
  </head>

  <body>
    <header class="container">
      <h1>Pull Request Reviewers</h1>
    </header>
    <main class="container">
      <form id="pull-request-form">
        <input
          id="pull-request-url"
          type="text"
          placeholder="Enter the full pull request url here"
        />
        <button>Go!</button>
        <div id="pull-request-url-messages"></div>
      </form>
      <div id="pull-request-info"></div>
      <ul id="user-list"></ul>
    </main>
    <script src="index.js"></script>
  </body>
</html>

Solution

For a framework-less vanilla JS/DOM it’s pretty much fine as is.
I can only suggest a few minor/cosmetic points to consider:

  • extract things from the big DOM event handler so it contains only the major flow and rename it from getPullRequestData to something else because your function also renders the data but getXXX implies only the action of getting – I would use a generic name like onsubmit since the function isn’t reusable and this name tells exactly what it actually is.
  • shorten some names because verbosity can make simple things look complicated
  • sometimes then + catch may convey the flow better than await + try/catch
function onsubmit(event) {
  event.preventDefault()
  byId('user-list').textContent = ''
  byId('pull-request-info').textContent = ''
  renderStatus('')
  fetchPR()
    .then(async pr => {
      renderPrInfo(pr)
      renderPrBody(pr.state === 'open' && await fetchUsers(pr))
    })
    .catch(renderStatus)
}
  • since you’re not writing a low-level library, there should be no need to use createDocumentFragment explicitly, instead just accumulate the elements in an array and pass it to append, which will allow us to express the intent and flow more explicitly:
function renderUsers(users) {
  const ul = byId('user-list')
  const title = createElement('h3', {
    textContent: users.length
      ? 'Reviewers:'
      : 'No reviewers found :(.',
  })
  ul.before(title)
  ul.append(...users.map(renderUser).filter(Boolean))
}
  • use createElement‘s second props parameter more in createUser
  • add a third parameter children to createElement
  • use Object.assign in createElement
  • make createUser return an array so it can be used directly in append
  • shorten .map(item => singleArgFunction(item)) to .map(singleArgFunction)
  • add JSDoc comments describing what the function does and its params/returned value, some IDE provide convenient plugins or built-in functionality to simplify the task
  • see if you like declaring functions in the order that loosely follows the execution flow from more generalized to more specific
const apiUrl = 'https://api.github.com'
const apiOptions = {
  headers: {
    Accept: 'application/vnd.github.v3+json',
  },
}

byId('pull-request-form').addEventListener('submit', onsubmit)

function onsubmit(event) {
  event.preventDefault()
  byId('user-list').textContent = ''
  byId('pull-request-info').textContent = ''
  renderStatus('')
  fetchPR()
    .then(async pr => {
      renderPrInfo(pr)
      renderPrBody(pr.state === 'open' && await fetchUsers(pr))
    })
    .catch(renderStatus)
}

function fetchPR() {
  const { pathname } = new URL(byId('pull-request-url').value)
  const url = `/repos${pathname.replace('/pull/', '/pulls/')}`
  return fetchGithubApi(url)
}

async function fetchUsers(pr) {
  const allComments = await Promise.all([
    fetchGithubApi(pr.review_comments_url),
    fetchGithubApi(pr.comments_url),
  ])
  const userUrls = allComments
    .flat()
    .filter(({ user }) => user.login !== pr.user.login)
    .map(({ user }) => user.url)
  const uniqUrls = [...new Set(userUrls)]
  return Promise.all(uniqUrls.map(fetchGithubApi))
}

async function fetchGithubApi(endpoint) {
  const url = endpoint.startsWith(apiUrl) ? endpoint : `${apiUrl}${endpoint}`
  return (await fetch(url, apiOptions)).json()
}

function renderStatus(status) {
  byId('pull-request-url-messages').textContent = status
}

function renderPrInfo(pr) {
  const title = createElement('a', {
    id: 'title',
    href: pr.html_url,
    textContent: pr.title,
  })
  byId('pull-request-info').append(title, createAvatar(pr.user))
}

function renderPrBody(users) {
  if (users) {
    renderUsers(users)
  } else {
    renderStatus('The pull request is already closed!')
  }
}

function renderUsers(users) {
  const ul = byId('user-list')
  const title = createElement('h3', {
    textContent: users.length
      ? 'Reviewers:'
      : 'No reviewers found :(.',
  })
  ul.before(title)
  ul.append(...users.map(renderUser).filter(Boolean))
}

function renderUser(user) {
  return user.name && createElement('li', {}, createUser(user))
}

function createUser(user) {
  return [
    createAvatar(user),
    createElement('a', {
      href: user.html_url,
      className: 'login',
      textContent: user.login,
    }),
    user.name &&
    createElement('div', {
      className: 'name',
      textContent: user.name,
    }),
  ].filter(Boolean)
}

function createAvatar(user) {
  return createElement('img', {
    src: user.avatar_url,
    className: 'avatar',
  })
}

function createElement(type, options = {}, children) {
  const element = document.createElement(type)
  Object.assign(element, options)
  if (children) element.append(...children)
  return element
}

function byId(elementId) {
  return document.getElementById(elementId)
}

Leave a Reply

Your email address will not be published. Required fields are marked *