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

@aragon/api-react: useApps() #327

Merged
merged 12 commits into from Sep 2, 2019

Conversation

@bpierre
Copy link
Member

commented Jun 21, 2019

Builds on #326

Receive the complete list of apps installed in the organization.

Example:

import { useApps } from '@aragon/api-react'
function App() {
  const apps = useApps()
  return (
    <div>
      {apps.map(app => (
        <div>{app.appAddress}</div>
      ))}
    </div>
  )
}

@bpierre bpierre requested review from 2color and sohkai Jun 21, 2019

@Schwartz10 Schwartz10 referenced this pull request Jun 25, 2019
1 of 1 task complete

@bpierre bpierre changed the title Apireact apps @aragon/api-react: useApps() Jun 26, 2019

@bpierre bpierre referenced this pull request Jul 3, 2019
1 of 1 task complete
@sohkai

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

@bpierre Pushed 3de8d3b to update this implementation with @aragon/api's. Perhaps it may make sense to rebase the apps changes on master and merge it separately from path?

@stale stale bot added the abandoned label Aug 24, 2019

@aragon aragon deleted a comment from stale bot Aug 24, 2019

@stale stale bot removed the abandoned label Aug 24, 2019

bpierre and others added 6 commits Jun 21, 2019

@sohkai sohkai force-pushed the apireact-apps branch from 3de8d3b to 9f47d9e Sep 1, 2019

@sohkai
sohkai approved these changes Sep 1, 2019
Copy link
Member

left a comment

@bpierre I've pushed a few commits and rebased this on top of master.

It updates the usage of aragonAPI to what's on master, and also includes a currentApp API. The currentApp API is the only one that isn't a subscription (since this detail should never change in-flight), but it's such essential information (e.g. getting the currently running app's own proxy address) that it feels right to make it very easily accessible.


Each object in the array holds the following keys:

- `appAddress`: the app's contract address

This comment has been minimized.

Copy link
@bpierre

bpierre Sep 1, 2019

Author Member

@sohkai Is the idea to progressively move away from proxyAddress / contractAddress? And shouldn’t we pass currentApp through transformAppInformation(app) too?

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 2, 2019

Member

And shouldn’t we pass currentApp through transformAppInformation(app) too?

I think we do this: https://github.com/aragon/aragon.js/blob/master/packages/aragon-wrapper/src/rpc/handlers/get-apps.js#L53

Is the idea to progressively move away from proxyAddress / contractAddress?

I'd like to. proxyAddress has always been a bit of a misnomer, and confusing. contractAddress even more so. The important thing, outside of aragonOS (and some tiny parts in aragon.js), is that each app has a contract address. Maybe it's a proxy, maybe it has a base implementation, but these details shouldn't matter to any developer unless they're building something custom on the contract level (at which point aragon.js probably isn't the library for them).

This comment has been minimized.

Copy link
@bpierre

bpierre Sep 2, 2019

Author Member

I think we do this: https://github.com/aragon/aragon.js/blob/master/packages/aragon-wrapper/src/rpc/handlers/get-apps.js#L53

Oh I missed that 👍

I'd like to. proxyAddress has always been a bit of a misnomer, and confusing. contractAddress even more so. The important thing, outside of aragonOS (and some tiny parts in aragon.js), is that each app has a contract address. Maybe it's a proxy, maybe it has a base implementation, but these details shouldn't matter to any developer unless they're building something custom on the contract level (at which point aragon.js probably isn't the library for them).

That sounds good yes! I wonder if we should also move to this naming in aragon/aragon, just for consistency.

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 2, 2019

Member

Haha absolutely... we can start by mapping the values over to a different schema in aragonjs-wrapper.

It's a bit hard to update @aragon/wrapper because the CLI also depends on it, but I hope with the refactor they're doing they'll be able to accommodate for breaking changes more easily.

sohkai added 5 commits Sep 2, 2019

@sohkai sohkai merged commit 33345a9 into master Sep 2, 2019

11 of 12 checks passed

bootstrap bootstrap
Details
build build
Details
install install
Details
lint lint
Details
size size
Details
test test
Details
License Compliance FOSSA is analyzing this commit
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 54.837%
Details
license/cla Contributor License Agreement is signed.
Details

@sohkai sohkai deleted the apireact-apps branch Sep 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.