problem-report.md

Problem Report

We hope that future developers will find it easy to get up to speed with our app, as we have followed the guidelines for good practice set out by Dan Abramov in his Redux videos and exemplified by the minimal repo here.

There are nonetheless various corners and complexities that may not be immediately obvious. To aid in understanding these, we are building a record of the technical problems that we encountered on this project and the solutions that we found.

At the moment the report is very much in progress...

Deployd Boolean Fields

Problem

Submitting a form to the database was returning a generic error even though form data was good.

Solution

Deployd debug logs (turn them on with export DEBUG="*") revealed validation on the backend was failing on certain boolean fields. The fields were set to isRequired and Deployd was failing to distinguish between values of false and null or undefined. The workaround was to remove isRequired for these fields--if they are missing Deployd sets them to false anyway.

Deployd Dashboard Key

Problem

This is easy to get from the local command line but what about when the app is hosted remotely?

Solution

Run dpd keygen && dpd showkey && npm start from the Procfile and get the key from Heroku log.

Deployd with Webpack Dev Server

Problem

Webpack Dev Server will try to respond to all requests with static assets, so how do we make calls to our Deployd server?

Solution

We can pass an option to the dev server that will set it up to proxy requests to and from Deployd (hosted on port 2403): proxy: { '*': 'http://localhost:2403' }. Note, * is equivalent to ** in this case. We can simply proxy requests to all paths (but the root) because Deployd itself can be trusted to serve static assets from /public. Neat!

Browser History Module

Problem

For purely cosmetic reasons we would have preferred to use browserHistory with React-router. Browser history allows you to navigate to different resource locations (rather than simply different hashes at the same location) without reloading the index.html every time. In theory, this just requires some simple server configuration to ensure that all requests except for static assets are fielded with index.html, as below:

app.use(express.static(__dirname + '/public'));
app.get('*', function (request, response){
  response.sendFile(path.resolve(__dirname, 'public', 'index.html'));
})

If only...

Actually, some of our requests need to go to Deployd. So we would have to also set up Deployd as Express middleware and distinguish between requests to Deployd and requests for assets. We'd then need to proxy this server with webpack dev server or, better, set up the latter as another middleware. All together, this is not so simple and we were pressed for time.

Solution

Just use hashHistory and serve everything from the same location! We do not use server-side rendering or need our routes accessible to Google's crawler (the main practical benefits of serving from different locations), and for a minor cosmetic benefit, persevering was not worth the time.

If anyone wants to switch to browserHistory in the future, here are some tips:

• Dpd.js has an undocumented setBaseUrl method that should enable you to separate API calls from the rest of the application by prefixing them with /api or suchlike.

• You should just be able to mount the Deployd middleware at the right path prefix. If necessary, you can programatically access the API routes generated by Deployd with process.server.resources and make a filter, as described here.

Hash History Querystring

Problem

hashHistory puts a querystring starting _k= after every url. This is ugly and causes issues with navigation using the address bar; for example, hitting refresh will incorporate the querystring into the path recognised by React-router.

Solution

It is possible to manually create a hash history object without a query string and pass it to React-router. This only works if you use the same version of history as React-router:

import { Router, useRouterHistory } from 'react-router';
import { createHashHistory } from 'history';

const appHistory = useRouterHistory(createHashHistory)({ queryKey: false });

<Router history={appHistory}>

Be aware that the querystring is used by hashHistory to access any state you set for your locations like this:

router.push('/home', { some: 'state' })

We do not use location state, so removing the querystring is fine.

Tasks Form List

Problem

Tasks would appear to be randomly missing from the list of editable tasks beneath the case form. This was noticeable when switching between 'all' and 'pending' or 'to do' on the filter.

Solution

When multiple components are generated by a loop as an array, React uses a key prop to keep track of which components inside the array get added, removed or changed. We were setting this prop by the index of the element in the mapped array, a practice that React docs suggest is acceptable, if non-optimal, since without a 1:1 correspondence between data and key React cannot optimise the diff.

{tasks.map((task, idx) =>
      <ConnectedEditTaskForm key={idx} task={task} />)}

In fact, setting the key from the index can break the rerender entirely. Here's why (I think): if we apply a filter that selects items 3 and 4 from the list, their keys will be changed to 0 and 1. React will not rerender the components that already have those keys -- i.e. items 1 and 2 on the list. But we have filtered out their data on the props so we will get empty components...

The solution was to use the unique id from the database:

{tasks.map((task) =>
      <ConnectedEditTaskForm key={task.id} task={task} />)}

Storing Dates

Problem

Deployd does not offer a date data type.

Solution

We convert dates to UTC milliseconds for storage in the database. Conversion is done with moment. Currently these conversions are defined independently in various components (case list, task tabs and task forms) -- it would be better to extract date conversion functions and import them where needed.

Abstracting 'Create' Forms and 'Edit' Forms

Problem

The forms that are used to create a case or task and edit a case or task are almost identical. But what is the best way to extract the common functionality?

Solution

We implemented the case form before the task form. Operating on the principle that something should work before it is refactored, we have extracted a common form for cases, but did not get onto doing the same for tasks. The way in which we did it -- taking parameters from the location path and passing different props accordingly from the container component -- is somewhat problematic.

Problem

For example, navigating from an edit form to a new form would not remount the component, so the data was not cleared.

Solution

We listen for this transition in componentWillReceiveProps and fire an action clearCaseForm to dispatch Redux Form's destroy action creator and empty the currentCase.caseData state.

I believe a better approach would be to conditionally create different Redux Form components instead of wrapping the same instance and changing the props. Redux Form should handle destoying automatically.

Problem

Another group of problems related to our solution concerned handling routing edgecases. Because two levels of the url were parametrised (path="cases/:view(/:caseRef)") we could not simply add a catch-all route to match nonexistent views. And how to handle a bad caseRef?

Solution

We had to add logic to mapStateToProps and componentWillReceiveProps, as well as a redirect to 'page-not-found' in the action creator dispatched after a failure to retrieve a case (i.e. an erroneous caseRef url parameter). Getting the caseRef through a prop other than the router path would have enabled us to ensure the case existed beforehand. But we wanted to be able to navigate directly to cases through the address bar. The route's onEnter handler might have been a better place to check for a valid caseRef, but we didn't consider it. That leads us into the next issue...

Populating components asynchronously

Problem

There are several places where the data for components is obtained by an asynchronous API call. How do we get the data into a component after it is rendered?

Solution

Two possibilities we rejected were:

1. Dispatching the action inside the onClick handler of the link used to navigate to the component.
2. Dispatching it inside the onEnter handler on React-router's routes.

Since we wanted users to be able to navigate to components from the address bar we discounted the first option. The second option is criticised here on the basis that the route transition will be blocked until the data is received.

We dispatch an action inside componentDidMount. Unfortunately this solution means our component is no longer purely functional. In some cases we have extracted the API call into a class which does nothing else but wrap the component to be populated, which mitigates the problem of side effects. If continuing with this solution, it would probably be a good idea to extract this functionality further into a single higher order component that takes a dispatcher and can be used to wrap anything.

In retrospect, though, I see no problem with dispatching an async action in onEnter. The transition will not be blocked unless the callback is deliberately delayed. The component will render without data and redux will populate it when the data arrives, which is the same effect as firing the action in componentDidMount. Something like this would do the trick:

const load = ({dispatch}) =>
	(nextState, replace /*, callback*/) => { // no need to pass the callback
      dispatch(asyncAction)
    }
<Route path='path' component={comp} onEnter={load(store)} />

Problem

Redux Form uses an initialValues prop to preload a form with data. Unless you pass enableReinitialize: true to the form, this prop can only be passed once -- no good if it is filled with async data that arrives after the form has mounted as it will be empty when first passed.

Solution

Just pass enableReinitialize: true? No! There is reportedly a bug in Redux Form that breaks validation when passing this prop. Instead we dispatch our clearCaseForm after the async data has been received to force the form to remount from scratch. Ugly. It may be that the supposed bug is a phantom, one of many haunting Redux Form's 457 open issues, but we decided not to take the risk.

Promises with Redux-form

Problem

In theory, Redux-form makes it easy to dispatch actions conditional upon the resolution or rejection of a promise returned by onSubmit and other async actions, but there were a few issues in getting this to work:

• To populate its error object after onSubmit, Redux-form needs the returned promise to throw with a special submissionError object. It turns out that onSubmitFail isn’t called with the error thrown in the promise, but with some Redux-form state which is only populated if the promise threw a submissionError.
• The promises returned by dpd have no catch method (they use fail instead).
• Redux-form’s asyncValidate was failing even when returning a resolved promise, and passing even when returning a rejected promise.

Solution

Where it is necessary to throw a submissionError we wrap the promise returned by dpd in a Javascript native promise and pipe the argument of the reject handler through a function to convert it into a submissionError before passing it to the reject handler. Note that it is not possible simply to catch the error and return a submissionError as once the error is caught, the promise resolves. It would be possible to throw the submissionError anew or return Promise.reject(submissionError), but wrapping the whole promise has the added benefit of ensuring that catch can be called on it in the future (rather than needing to remember to use fail). We extracted the wrapper into a function called dpdRun.

What matters for asyncValidate is not whether or not the promise resolves, but whether it resolves or rejects with an object (which becomes the validation error). That's fine as long as you are aware of it! Make sure the promise resolves (or rejects) with an object if you want to throw the error, and resolves (or rejects) with null if you do not.

Error handling

Problem

We make use of the error handling built into Redux-form and have implemented some actions and reducers for handling other types of error (for example, from fetching data), but not all which are required. We also have no generalised way of responding to the errors that do find their way into the Redux store.

Solution

We render an error component defined inside the case form and dependent on the error prop from Redux-form. One or two fetching errors are handled ad-hoc in a similar manner. In the future, it would make sense to implement a single higher order component that would check for various kinds of error and display the appropriate messages.