React API improvements

[bpierre]

fixes #216, builds on #229

This PR contains various improvements for the React API. I suggest to review them commit by commit, every section ends with a link to the related commits (“see …”).

Loading behavior

Since we replaced the GraphQL subscriptions by queries with polling (see #203), the hooks provided by the React API were updating too often, and incorrectly (see #216). This version is now taking care of only triggering updates when a changed happened, and to only use loading states when necessary.

See 0f0206a, a223382, 3589809.

createAppHook(): allow connector configuration to be set

App connectors can receive a source connector and its options, but this wasn’t possible when using createAppHook(). A second parameter has been added, to either pass a specific connector or to change its options:

import connectVoting from '@aragon/connect-voting'
import { createAppHook } from '@aragon/connect-react'

const useVoting = createAppHook(connectVoting, ['thegraph', { subgraphUrl: 'https://…' }])

See f0dbc28.

createAppHook(): add a dependencies array

Hooks created through createAppHook() now accept a dependency array, that can get used to refresh its callback. When no dependency array is passed, the callback never updates.

import connect from '@aragon/connect'
import connectVoting from '@aragon/connect-voting'
import { createAppHook, useApp } from '@aragon/connect-react'

const useVoting = createAppHook(connectVoting)

function App() {
   const [page, setPage] = useState(0)

   const [voting] = useApp('voting')
   const [votes] = useVoting(voting, app => (
      app.votes({ first: 10, skip: 10 * page })
   ), [page])

   return (
      <div>
         <button onClick={() => setPage(page + 1)}>prev</button>
         <button onClick={() => setPage(page - 1)}>next</button>
      </div>
   )
}

See 7f661d3.

Populating status.error rather than throwing errors

In many cases, errors were thrown rather than populating the status.error object in the React API. It is an issue that was also happening with the JS API (see #229). It has now been fixed.

import connect from '@aragon/connect'
import connectVoting from '@aragon/connect-voting'
import { createAppHook, useApp } from '@aragon/connect-react'

const useVoting = createAppHook(connectVoting)

function App() {

   const [voting, votingStatus] = useApp('voting')
   const [votes, votesStatus] = useVoting(voting, app => app.votes())

   if (votingStatus.error) {
      return (
         <h1>Error: {votingStatus.error.message}</h1>
      )
   }

   if (votesStatus.error) {
      return (
         <h1>Error: {votesStatus.error.message}</h1>
      )
   }

   return (
      // …
   )
}

See ddaad5b, 79c5382.

App methods subscriptions

Prior to this version, it was not possible to subscribe to app methods using createAppHook(). The fetched data was returned once, and only updated when the component was mounted again. The feature has been added, and we can now subscribe to any app methods, alongside the core ones (useApp(), usePermissions(), …). Every part of the React API is now listening to changes and reacting accordingly.

To use this feature, the subscription methods (onX) should be called, but without passing a callback. createAppHook() will take care of handling the subscription.

import connect from '@aragon/connect'
import connectVoting from '@aragon/connect-voting'
import { createAppHook, useApp } from '@aragon/connect-react'

const useVoting = createAppHook(connectVoting)

function App() {
   const [voting, votingStatus] = useApp('voting')

   // To enable a subscription, use app.onVotes() rather than app.votes().
   const [votes, votesStatus] = useVoting(voting, app => app.onVotes())

   return (
      // …
   )
}

Of course, it also works with the dependencies array, taking care of unsubscribing and subscribing to the new query:

import connect from '@aragon/connect'
import connectVoting from '@aragon/connect-voting'
import { createAppHook, useApp } from '@aragon/connect-react'

const useVoting = createAppHook(connectVoting)

function App() {
   const [page, setPage] = useState(0)

   const [voting] = useApp('voting')
   const [votes] = useVoting(voting, app => (

      // Note the app.onVotes() rather than app.votes().
      app.onVotes({ first: 10, skip: 10 * page })

   // The subscription will refresh every time `page` changes.
   ), [page])

   return (
      // …
   )
}

Important: this only works with app connectors enabling this syntax. All the core app connectors have been updated for it, but third party app connectors will require to be updated too.

See #229, 918810b, 2d2cc88, 0278280, 65cc59c, 8e32638, 7a173b9, 0e77afb, 2076bc4.

[codecov]

Codecov Report

Merging #233 into master will decrease coverage by 0.33%.
The diff coverage is 13.86%.

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
- Coverage   39.20%   38.87%   -0.34%     
==========================================
  Files         113      114       +1     
  Lines        2043     2089      +46     
  Branches      314      318       +4     
==========================================
+ Hits          801      812      +11     
- Misses       1242     1277      +35     

Flag	Coverage Δ
#unittests	38.87% <13.86%> (-0.34%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/connect-core/src/entities/App.ts	0.00% <0.00%> (ø)
packages/connect-core/src/utils/index.ts	0.00% <0.00%> (ø)
packages/connect-core/src/utils/subscriptions.ts	0.00% <0.00%> (ø)
packages/connect-thegraph/src/parsers/apps.ts	0.00% <0.00%> (ø)
...t-voting-disputable/src/models/DisputableVoting.ts	20.37% <4.76%> (-0.91%)	⬇️
packages/connect-agreement/src/models/Agreement.ts	65.30% <11.76%> (-6.79%)	⬇️
packages/connect-finance/src/models/Finance.ts	9.52% <14.28%> (+3.96%)	⬆️
...kages/connect-voting-disputable/src/models/Vote.ts	81.81% <20.00%> (-3.73%)	⬇️
...nect-agreement/src/models/CollateralRequirement.ts	86.36% <25.00%> (-3.64%)	⬇️
...ages/connect-agreement/src/models/DisputableApp.ts	80.00% <25.00%> (-4.62%)	⬇️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 259cab7...ba35574. Read the comment docs.

[andy-hook]

This is going to be so useful, thanks for turning it around so quickly @bpierre ! I've been doing a little bit of testing and also left a few surface level comments. Something I've noticed is with the unsubs, when I unmount Votes in the react demo both components seem to be unsubscribed and then loading gets stuck on the remount. Is this expected?

[bpierre]

I've noticed is with the unsubs, when I unmount Votes in the react demo both components seem to be unsubscribed and then loading gets stuck on the remount. Is this expected?

Good catch, thanks! It should be fixed since add4da4. Updates should now be reduced to the strict minimum, and the loading issue is now fixed. Could you please try and tell me if it works for you?

[andy-hook]

I've noticed is with the unsubs, when I unmount Votes in the react demo both components seem to be unsubscribed and then loading gets stuck on the remount. Is this expected?

Good catch, thanks! It should be fixed since add4da4. Updates should now be reduced to the strict minimum, and the loading issue is now fixed. Could you please try and tell me if it works for you?

Awesome stuff, the inclusion of the buttons in the demo was nice too 😁 the mounting issue is fixed! but I still saw the same problem with the votesStatus loading flag mentioned here #233 (comment)

[andy-hook]

@bpierre – Loading statuses are consistent and working great! I did spin up the org viewer react demo as well just to double check some things and I'm seeing a useState recursion warning in the console. Best way to repro is to spin up that example and take a look, let me know if you see the same thing!

[bpierre]

@andy-hook oops ba35574

It should all be fine now, I tested all the examples. Let me know if that works for you!

[andy-hook]

@andy-hook oops ba35574

It should all be fine now, I tested all the examples. Let me know if that works for you!

Works great!

Edit: removing happy clappy Morgan Freeman to save my eyes, but he remains in spirit :)

[andy-hook]

This is working great, thank you @bpierre !

@0xGabi you might want to double check the specifics again but it looks good on my end.