Refactoring breaks initial state

Question

React (from create-react-app) with MobX. Using axios for async backend API calls.

This code works. The initial state (array of issues) is populated, and the webpage presenting this component renders with initial content from state.

import { observable, computed, autorun, reaction } from 'mobx'
import axios from 'axios'

class IssuesStore {
  @observable issues = []

  constructor() {
    autorun(() => console.log("Autorun:" + this.buildIssues))

    reaction(
      () => this.issues,
      issues => console.log("Reaction: " + issues.join(", "))
    )
  }

  getIssues(data) {
    return data.map((issue) => ({title: issue.name, url: issue.url, labels: issue.labels}))
  }

  @computed get buildIssues() {
    const authToken = 'token ' + process.env.REACT_APP_GH_OAUTH_TOKEN

    axios.get(`https://api.github.com/repos/${process.env.REACT_APP_GH_USER}/gh-issues-app/issues`,
                  { 'headers': {'Authorization': authToken} })
      .then(response => {
        console.log(response)
        this.issues = this.getIssues(response.data)
        return this.issues
      })
      .catch(function(response) {
        console.log(response)
      })
  }
}

export default IssuesStore

In an attempt to separate API invocation promises from individual components and stores, I pulled out the axios call into a separate js file, as a collection of functions:

import axios from 'axios'

const authToken = 'token ' + process.env.REACT_APP_GH_OAUTH_TOKEN

export function loadIssues() {
  return this.apiPromise(
    `https://api.github.com/repos/${process.env.REACT_APP_GH_USER}/gh-issues-app/issues`,
    { 'headers': {'Authorization': authToken} }
  )
}

export function apiPromise(endpoint, options) {
  return axios.get(endpoint, options)
      .then((response) => {
        // console.log("response: " + JSON.stringify(response, null, 2))
        return response.data.map((issue) => ({title: issue.name, url: issue.url, labels: issue.labels}))
      })
      .catch(function(response) {
        console.log(response)
      })
}

Now, my store looks like this:

import { observable, computed, autorun, reaction } from 'mobx'
import * as github from '../api/github'

class IssuesStore {
  @observable issues = []


  constructor() {
    autorun(() => console.log("Autorun:" + this.buildIssues))

    reaction(
      () => this.issues,
      issues => console.log("Reaction: " + issues.join(", "))
    )
  }

  @computed get buildIssues() {
    this.issues = github.loadIssues().data
    return this.issues
  }
}

export default IssuesStore

Much smaller... but the webpage now throws an error because it now sees the initial state of issues as undefined on first render.

Uncaught TypeError: Cannot read property 'map' of undefined

The promise completes successfully later on (as it should), but by then it's too late. Sure, I can set up a few null checks in my rendering components to not run .map or other such functions on empty or as-yet-undefined variables.

But why does the code work with no initial rendering errors before the refactoring, and not after? I thought the refactoring was effectively maintaining the same logic flow, but I must be missing something?

Answer 1

In your refactored version

github.loadIssues().data

Is always going to be undefined because the data property on that Promise will always be undefined.

In the original version, this.issues was only ever set once data returned from the api, so the only values that it was ever set to were the initial value [] and the filled array from the api response.

In yours, the three states are [] -> undefined -> and the filled array.

buildIssues should look something like this:

@computed get buildIssues() {
    github.loadIssues().then((data) => {
      this.issues = data
    }).catch((err) => {
      // handle err.
    })
}