Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @aragon/api-react #259

Merged
merged 23 commits into from Mar 22, 2019

Conversation

Projects
None yet
5 participants
@bpierre
Copy link
Member

commented Mar 12, 2019

aragonAPI for React

This module allows to interact with aragonAPI using React Hooks. @aragon/api is used under the hood, so being familiar with it can be useful.

Usage

import { AragonApi, useAragonApi } from '@aragon/api-react'

function App() {
  const { api, appState } = useAragonApi()
  const { count = 0 } = appState

  return (
    <div>
      <div>{count}</div>
      <button onClick={() => api.increment(1)}>
        Increment
      </button>
    </div>
  )
}

ReactDOM.render(
  <AragonApi>
    <App />
  </AragonApi>,
  document.getElementById('root')
)

This is a simple example demonstrating how we can use aragonAPI for React to connect the app to its contract, fetch some data from its state (using appState), and trigger an action on it (with api.increment(1)). The full API is detailed below.

Documentation

<AragonApi />

Before using any Hook provided, you need to declare this component to connect the app. It is generally a good idea to do it near the top level of your React tree. It should only be declared once.

It has an optional reducer prop, which lets you process the state coming from the background script. If not provided, the state is passed as is.

Example

import { AragonApi, useAppState } from  '@aragon/api-react'
import BN from 'bn.js'

function App() {
  const { balance } = useAppState()
  return (
    <div>{balance.toString(10)}</div>
  )
}

function reducer(state) {
  if (state === null) { // initial sync
    return { balance: new BN(0) }
  }
  return { balance: new BN(state.balance) }
}

ReactDOM.render(
  <AragonApi reducer={reducer}>
    <App />
  </AragonApi>,
  document.getElementById('root')
)

useAragonApi()

A React Hook that returns the data needed to interact with the app contract.

As with any React Hook, please ensure that you follow the Rules of Hooks.

It returns an object containing the following entries:

api

This is the current AragonApp instance. Use it to call methods on the contract.

Example:

function App() {
  const { api } = useAragonApi()
  return (
    <button onClick={() => api.vote(true)}>Vote</button>
  )
}

appState

The app state, after having passed the background script state through the reducer prop of AragonApi.

Example:

import { useAragonApi } from  '@aragon/api-react'

function App() {
  const { appState } = useAragonApi()
  return (
    <div>{appState.count}</div>
  )
}

connectedAccount

The connected Ethereum account. Its value is "" (empty string) when there is no account connected.

Example:

function App() {
  const { connectedAccount } = useAragonApi()
  return (
    <div>Account: {connectedAccount? connectedAccount : 'Not connected'}</div>
  )
}

network

An object representing the current network using its id and type entries. Its value is null until it gets loaded.

Example:

function App() {
  const { network } = useAragonApi()
  return (
    <div>Current network: {network.type}</div>
  )
}

displayMenuButton

Whether or not to display the menu button (Boolean), depending on it being automatically hidden or not in the client.

requestMenu()

Call this function to display the Aragon menu, when hidden automatically. This should be called when the user clicks on the menu button.

useApi()

This Hook returns the same data than the api entry from the useAragonApi() hook.

useAppState()

This Hook returns the same data than the appState entry from the useAppState() hook.

useConnectedAccount()

This Hook returns the same data than the connectedAccount entry from the useAragonApi() hook.

useMenuButton()

This Hook returns an array containing the displayMenuButton and the requestMenu entries from the useAragonApi() hook, in that order.

useNetwork()

This Hook returns the same data than the network entry from the useAragonApi() hook.

License

AGPL-3.0

bpierre added some commits Mar 12, 2019

@bpierre bpierre requested review from 2color, sohkai and 0x6431346e Mar 12, 2019

@bpierre bpierre referenced this pull request Mar 12, 2019

Merged

Use @aragon/api-react #38

bpierre added some commits Mar 12, 2019

Menu management: displayMenuButton + requestMenu()
Also add _sendMessage() and _onMessage(), as a private API for the time
being.
@0x6431346e
Copy link
Member

left a comment

Awesome stuff!!

I'm not sure I understand the use case of the second reducer though (the one passed to useAragonApi)🤔.
Is this hook expected to be used only once, in the 'App'/top-level component and the api to be passed down to children or would each child use the hook?

@bpierre

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2019

@0x6431346e Thanks!

I'm not sure I understand the use case of the second reducer though (the one passed to useAragonApi)🤔.

This is the reducer that transforms the data coming from the RxJS observer into a data structure ready to be used by the React app. By keeping this part outside the rendering function, it allows to process emitted values only once. It is entirely optional. You can see an example of this kind of reducer in the Voting app or the Token Manager app.

Is this hook expected to be used only once, in the 'App'/top-level component and the api to be passed down to children or would each child use the hook?

For the initial version, we decided that it would be better to make it a hook for the top level component only, to avoid any confusion around the passed reducer… but I agree this might be a bit confusing.

A solution could be to have useAragonApi() work as a read-only hook when it gets used by a child:

import { useAragonApi } from '@aragon/api-react'

const appStateReducer = state => ({ ...state, /**/ })

// The initial call would initiate the connection,
// and be the only call allowed to update the reducer.
const App = () => {
  useAragonApi(appStateReducer)
  return <Child />
}

// This would work fine…
const Child = () => {
  const { appState } = useAragonApi()
  return <p>{appState.value}</p>
}

// …but this would trigger an error.
const Child = () => {
  const { appState } = useAragonApi(appStateReducer)
  return <p>{appState.value}</p>
}

It should be feasible using useEffect() (to ensure only one component is using the initial version), but I don’t like having a function with variable parameters depending on its context, it might feel even more confusing.

Another solution could be to provide another hook, or a set of hooks, to retrieve this data:

import { useAragonApi, useAragonApiContext } from '@aragon/api-react'

const appStateReducer = state => ({ ...state, /**/ })

// This doesn’t change.
const App = () => {
  useAragonApi(appStateReducer)
  return <Child />
}

// But the children would have to use another hook:
const Child = () => {
  const { api, appState } = useAragonApiContext()
  return <p>{appState.value}</p>
}
// We could also export several hooks:

import {
  useApi,
  useAppState,
  useNetwork,
  useConnectedAccount,
  useMenu,
} from '@aragon/api-react'

/**/

const Child = () => {
  const api = useApi()
  const appState = useAppState()
  const network = useNetwork()
  const { displayMenuButton, requestMenu } = useMenu()
  return <p>{appState.value}</p>
}

We could also decide to separate the initialization from the data retrieving, perhaps by renaming useAragonApi into something like useConnectApp (or anything more elegant than having two verbs next to each other 😆):

import {
  useApi,
  useAppState,
  useConnectApp,
} from '@aragon/api-react'

const appStateReducer = state => ({ ...state, /**/ })

const App = () => {

  // required to make the other hooks return something
  useConnectApp(appStateReducer)

  // these can be called from anywhere in the tree
  const api = useApi()
  const appState = useAppState()

  return <p>{appState.value}</p>
}

@sohkai @0x6431346e thoughts?

@@ -57,16 +73,21 @@ function useAragonApi(
}, [api])

return {
// appStateReducer(null) is called to get the initial state
appState: appState === null ? appStateReducer(null) : appState,

This comment has been minimized.

Copy link
@sohkai

sohkai Mar 19, 2019

Member

What do you think about putting this closer to _sendMessage, so that all the "shorthand" object properties (e.g. api) are grouped at the top?

@sohkai

sohkai approved these changes Mar 19, 2019

Copy link
Member

left a comment

😍

@sohkai

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

For the initial version, we decided that it would be better to make it a hook for the top level component only, to avoid any confusion around the passed reducer… but I agree this might be a bit confusing.

My original thinking was to force users to only use this once, and they could handle any sharing of the state internally. However, I'm starting to think this could be confusing since hooks seem like they should be reusable anywhere in the render tree...

We could make it possible for any call to useAragonApi() to connect a reducer, but this would break most of the current hook. I don't want to be creating a new Aragon object every time a component uses the hook, for example; it'd be much nicer to share the both the Aragon object, as well as its observables' state.

It then sounds like either useAragonApiContext() or useConnectApp() would work, to create a provider-consumer pairing. But this would also mean more boilerplate code, and asks the question of where we would use the provider hook.

@2color

2color approved these changes Mar 19, 2019

@2color

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

It then sounds like either useAragonApiContext() or useConnectApp() would work, to create a provider-consumer pairing. But this would also mean more boilerplate code, and asks the question of where we would use the provider hook.

Not sure I understood.

@0x6431346e

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

This is the reducer that transforms the data coming from the RxJS observer into a data structure ready to be used by the React app. By keeping this part outside the rendering function, it allows to process emitted values only once. It is entirely optional. You can see an example of this kind of reducer in the Voting app or the Token Manager app.

Ah, I see, makes sense.

For the initial version, we decided that it would be better to make it a hook for the top level component only, to avoid any confusion around the passed reducer… but I agree this might be a bit confusing.

My original thinking was to force users to only use this once, and they could handle any sharing of the state internally.

Would be awesome to give a warning/error if there's an easy way to detect this.

We could also decide to separate the initialization from the data retrieving, perhaps by renaming useAragonApi into something like useConnectApp (or anything more elegant than having two verbs next to each other 😆):

Sounds like a great v2.

// This would work fine…
const Child = () => {
  const { appState } = useAragonApi()
  return <p>{appState.value}</p>
}

In this case the Child would render on any state change, right?
Maybe we could do something like this: https://github.com/facebookincubator/redux-react-hook#usemappedstatemapstate

@sohkai

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@2color

It then sounds like either useAragonApiContext() or useConnectApp() would work, to create a provider-consumer pairing. But this would also mean more boilerplate code, and asks the question of where we would use the provider hook.

Not sure I understood.

If we are to share the top level state (the Aragon object as well as a number of observables), we'll need a way to provide that into the other hooks (used by children), hence the provider-consumer pairing. useConnectApi() would be the provider, and the other hooks would be consumers.

If we split this up, then we'll have to use the provider hook separately, and figure out which component it belongs in. In the example given for useConnectApp() though, it seems like we could actually do this in the same component, so maybe it's not a problem.

@2color

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

If we are to share the top level state (the Aragon object as well as a number of observables), we'll need a way to provide that into the other hooks (used by children), hence the provider-consumer pairing. useConnectApi() would be the provider, and the other hooks would be consumers.

I undertand.

I don't want to be creating a new Aragon object every time a component uses the hook

👍

We could also decide to separate the initialization from the data retrieving, perhaps by renaming useAragonApi into something like useConnectApp

Separating/splitting seems logical. We certainly want to ensure initialisation happens only once.

hence the provider-consumer pairing. useConnectApi() would be the provider, and the other hooks would be consumers

Implementation question, would this rely on react context that'd be used in our hook implementation. Basically just abstracting the details of the context provider/consumer pattern?

bpierre added some commits Mar 20, 2019

API update
- There is now a <ConnectAragonApp /> component that need to be declared
  at the top level of the app. This component is responsible of connecting
  the app and takes an optional reducer prop. It can only be declared once
  in an app. If any hook is used without having this component declared at
  an upper level, an error is thrown. The reducer prop can be updated if
  needed.

- The useAragonApi() hook is now only a consumer: it can be used
  anywhere, as many time as we want, and doesn’t take any parameter.

- The render props API has been removed for clarity, and because it is
  now very simple to implement if needed: ({ children }) =>
  children(useAragonApi()).

- Add alias hooks for convenience: useNetwork(), useApi(), etc. (see
  README.md)
@bpierre

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2019

Thank you all for the feedback, it’s been super useful!

@0x6431346e

// This would work fine…
const Child = () => {
  const { appState } = useAragonApi()
  return <p>{appState.value}</p>
}

In this case the Child would render on any state change, right?

Yes 👍

Maybe we could do something like this: https://github.com/facebookincubator/redux-react-hook#usemappedstatemapstate

Thanks, this module inspired me to propose a new solution for the API, that I like much better! 👌

Here are the changes:

  • There is now a <ConnectAragonApp /> component that need to be declared at the top level of the app. This component is responsible of connecting the app and takes an optional reducer prop. It can only be declared once in an app. If any hook is used without having this component declared at an upper level, an error is thrown. The reducer prop can be updated if needed.

  • The useAragonApi() hook is now only a consumer: it can be used anywhere, as many time as we want, and doesn’t take any parameter.

  • The render props API has been removed for clarity, and because it is now very simple to implement if needed: ({ children }) => children(useAragonApi()).

  • Add alias hooks for convenience: useNetwork(), useApi(), etc. (see README.md)

I updated the PR description with the new README.

@2color @0x6431346e @sohkai what do you think?

Doc fixes from @2color
Co-Authored-By: bpierre <hello@pierre.world>
@2color

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

Nice! The latest changes make this really clear and elegant.

Regarding @AquiGorka's suggestion (export default ConnectAragonApi(App, reducer)), I'm aware that HOCs (Higher order components) are contending with render props as a solution for some problems.

@bpierre What do you think about this?

@bpierre

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2019

@AquiGorka @2color Regarding having a function: would it be an HOC in this case, since we don’t pass any data to the wrapped component? I see it more as an “enhanced” context provider.

But I understand the term connectSomething() (and withSomething()) is often used in the React world for HOCs (as in, “a function that returns a new component with implicit props passed to it”), so the term might be a bit confusing, even though it refers to “initiate the connection to aragonAPI” here, and not “connect this component to something”. Maybe we could rename it <AragonApiProvider>? 🤔

It would be similar to the Provider component of React Redux, which behaves like the AragonApiConnect component.

I think a good case for HOC functions would be for the useSomething hooks we provide, but hooks were invented to solve this exact problem 😆

What do you guys think?

@2color

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

Having read your last comment, on a second thought, I think it's sensible as you have it right now.

@2color

2color approved these changes Mar 21, 2019

@bpierre

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2019

I start to like AragonApiProvider as it might be a bit clearer… and I was wondering if AragonApi could work to, now that the render prop that was using this name got removed? Or if it’s too weird to also have useAragonApi() doing something different. 🤔

We could also name it Provider like React Redux does, but for named exports I generally prefer less generic names.

Edit: I renamed it AragonApi for now 💥

bpierre added some commits Mar 21, 2019

@AquiGorka
Copy link

left a comment

❤️ ❤️ this is super cool.

Regarding the hoc thing, I agree it is more an "enhancer", my original question was if it would make sense to offer such enhancer as one other way to "connect" mostly because of the similarity to redux's connect.

@bpierre

This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2019

Regarding the hoc thing, I agree it is more an "enhancer", my original question was if it would make sense to offer such enhancer as one other way to "connect" mostly because of the similarity to redux's connect.

I would prefer not to, because its behavior is closer to the Provider of React Redux than its connect() function, even though the reducer is passed to the provider in our case.

Making it possible to declare the provider using a function named connect() could let people think that it works like React Redux, and can be used to connect any component to the store, while it is not the case: it should only be declared once, at the top level.

What is similar to the React Redux connect() is the collection of hooks itself, exactly the same way redux-react-hook removed the connect() function to provide useMappedState(reducer) instead.

We could also add a function like provideAragonApi(), but I’m not sure why that would be needed (compared to declaring a component)? I think we should aim to provide “one obvious way to do something”, when possible. 😆

@sohkai

sohkai approved these changes Mar 22, 2019

Copy link
Member

left a comment

💯 💯 💯

Can't wait for this to be used by others!

Show resolved Hide resolved packages/aragon-api-react/README.md Outdated
Show resolved Hide resolved packages/aragon-api-react/yarn.lock Outdated
Show resolved Hide resolved packages/aragon-api-react/src/index.js Outdated
Show resolved Hide resolved packages/aragon-api-react/src/index.js Outdated
Show resolved Hide resolved packages/aragon-api-react/src/index.js

@bpierre bpierre merged commit f87acb7 into master Mar 22, 2019

3 of 6 checks passed

License Compliance FOSSA is analyzing this commit
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
bootstrap bootstrap
Details
install install
Details
license/cla Contributor License Agreement is signed.
Details

@bpierre bpierre deleted the aragon-api-react branch Mar 22, 2019

@luisivan luisivan referenced this pull request Apr 5, 2019

Closed

Responsive #4

39 of 46 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.