Skip to content
This repository has been archived by the owner on Apr 11, 2022. It is now read-only.

refactor(context): Split ApiContext into Api & SystemContext #120

Merged
merged 4 commits into from
Jan 30, 2020
Merged

refactor(context): Split ApiContext into Api & SystemContext #120

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7ac44b
refactor(context): Split ApiContext into Api & SystemContext
amaury1093 Jan 29, 2020
0f0bb36
Fix lint
amaury1093 Jan 29, 2020
4c8cfc0
Update headers
amaury1093 Jan 29, 2020
d33d9a2
Fix lint
amaury1093 Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion front/context/src/util/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { Observable } from 'rxjs';
*
* @param provider - The provider to track
*/
export function providerConnected(provider: ProviderInterface): Observable<boolean> {
export function providerConnected(
provider: ProviderInterface
): Observable<boolean> {
return new Observable(subscriber => {
if (provider.isConnected()) {
subscriber.next(true);
Expand Down
3 changes: 1 addition & 2 deletions front/gatsby/src/ContextGate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
AccountsContextProvider,
AlertsContextProvider,
ApiContextProvider,
StakingContextProvider,
TxQueueContextProvider,
} from '@substrate/context';
import React from 'react';
Expand All @@ -24,7 +23,7 @@ export function ContextGate({
<AccountsContextProvider>
<TxQueueContextProvider>
<ApiContextProvider provider={new WsProvider(ARCHIVE_NODE_ENDPOINT)}>
<StakingContextProvider>{children}</StakingContextProvider>
<>{children}</>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<>{children}</>
{children}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, i get some error like https://github.com/paritytech/substrate-light-ui/pull/743/files#diff-418f37370656dbd0c860458c05c4933cR17

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i guess that makes sense....

</ApiContextProvider>
</TxQueueContextProvider>
</AccountsContextProvider>
Expand Down
4 changes: 2 additions & 2 deletions front/gatsby/src/pages/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ function IndexPage(): React.ReactElement {
<ContextGate>
<ApiContext.Consumer>
{({
isReady,
isApiReady,
}: Partial<ApiContextType>): React.ReactElement | boolean | undefined =>
isReady && (
isApiReady && (
<Layout>
<SEO title='Home' />
<Onboarding />
Expand Down
6 changes: 3 additions & 3 deletions front/ui-components/src/stateful/Balance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ export function Balance(props: BalanceProps): React.ReactElement {
orientation = 'horizontal',
...rest
} = props;
const { api, isReady } = useContext(ApiContext);
const { api, isApiReady } = useContext(ApiContext);
const [allBalances, setAllBalances] = useState();
const [allStaking, setAllStaking] = useState();

useEffect(() => {
if (isReady) {
if (isApiReady) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it weird that we have to do this everywhere, since none of these should load in the first place before the isApiReady is set to true in the ContextGate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since none of these should load in the first place before the isApiReady is set to true in the ContextGate

Hmm, if we put this in ui-components and expose this package to npm, then I don't think we should assume that the end-user will user this component inside a ContextGate when isApiReady is set to true. Just to be sure, I'd leave this check here.

In light-ui, if we use internal components that are behind a Gate, then yeah we can remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: but also, as we talked before, a better way would simply only expose pure/stateless components in ui-components

const balanceSub = combineLatest([
api.derive.balances.all(address),
api.derive.staking.account(address),
Expand All @@ -42,7 +42,7 @@ export function Balance(props: BalanceProps): React.ReactElement {

return (): void => balanceSub.unsubscribe();
}
}, [api, isReady, address]);
}, [api, isApiReady, address]);

const handleRedeem = (address: string): void => {
// FIXME We're not unsubscring here, we should
Expand Down
3 changes: 0 additions & 3 deletions front/ui-components/stories/customDecorators/withApi.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ import { WsProvider } from '@polkadot/api';
import { ApiContextProvider } from '@substrate/context';
import React from 'react';

import { Loading } from '../../src/index';

export const withApi = (
storyFn: () => React.ReactElement
): React.ReactElement => {
return (
<ApiContextProvider
loading={<Loading active>Connecting to the node...</Loading>}
provider={new WsProvider('wss://kusama-rpc.polkadot.io/')}
>
{storyFn()}
Expand Down