Skip to content

Instantly share code, notes, and snippets.

@mattpocock
Last active May 29, 2019 08:33
Show Gist options
  • Select an option

  • Save mattpocock/aa4f4b01c8d1189957fdce89d506d9af to your computer and use it in GitHub Desktop.

Select an option

Save mattpocock/aa4f4b01c8d1189957fdce89d506d9af to your computer and use it in GitHub Desktop.

extends

We should err on the side of caution with 'extends'. It makes the codebase awful to read for a newbie, and goes against lots of React's functional paradigms. The classes feel quite polluted, especially since you're not sure which methods are native to the class or its parent. OOP is definitely not OP.

withQuery

We should gut the services, actions and stores file. They just aren't necessary, and they look slow and cumbersome to use. For some reason, every single API call is in a flux store, meaning the amount of boilerplate required is killer. We can, and should, reduce this to one function - withQuery. We want to migrate Flux to React, but first I think we should get rid of most of our flux stuff.

withLogic

Extracting pieces of logic into a kind of combination of hooks and HOC's sounds really fecking powerful. It means that we can extract out all the dangerous stuff inside the classes into composable chunks, which can then be pulled out wholesale later. Very, very powerful.

It also means that this compositional function can actually be dropped direct into React Native code, since it's all business logic. Perhaps a reusable library of api queries and logical operations? But then this is so easy to write and follow that perhaps that's overkill.

export default compose(
  withQuery(...),
  withLogic(({pageLoadState}) => {
    useWatchForTruthy(pageLoadState.loaded && !pageLoadState.errored, () => {
      console.log('Yay, it succeeded!')
    })
  })
)

withLogic could also be used without composition, simply:

export default withLogic(({pageLoadState}) => {
  useWatchForTruthy(pageLoadState.loaded && !pageLoadState.errored, () => {
    console.log('Yay, it succeeded!')
  })
}))(Home)

Pre-loading data on build

We should consider pre-loading JSON data at build time. This would mean one important thing - our clients would be able to trigger netlify deploys with a hook. That means careful communication, because changes they make would not immediately head to the site.

We could even consider pre-loading data at build time, and making the API call for the data anyway. That way, the user sees no loading gap, but potentially things move around a little. We could manage that with animations and the like. And given our deploy speed now, it's highly unlikely that the data sets would be out of date.

JSON API definitions

The API client is hefty. All of the this.jsonApi.define stuff is really nasty, especially since it's divided up over so many files. Could we have one apiClient.js that handles all of the definitions in one spot? That way, we don't have to re-initialise it all the time.

Also, could we not do the same with Axios, and some TS definitions to spell out exactly what's returned from the API? I'm not sure what this 300-starred repo is giving us that Axios could not. It certainly isn't giving us autocomplete, or telling us when things change on the API.

Amount of API Calls

The number of API calls the app makes is astonishing. Is this a result of the JSON API spec, asking us to drill deeper into each definition?

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