-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
core/providers/api/provider.tsx
Outdated
}); | ||
}; | ||
|
||
const addSmoldotProvider = async (chain: Chain) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only set one provider at a time, not both the light client and a WsProvider instance.
I think it makes sense to add a config in the ConfigProvider so a user or the developer can choose if they want one or the other globally or per chain. The default should be WsProvider.
- Maybe something like this?
// in config/model.ts
export type ConnectionType = 'trusted' | 'light-client';
export type ConfigProps = {
// ... omitted
api?: {
default?: ConnectionType;
} & Partial<Record<ChainId, ConnectionType>>;
};
Then inside of ApiProvider you use useConfig()
to get the values and create the instances accordingly. This will update automatically when the config is changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also add a setter function to ConfigProvider
so a user can change the connection type for all chains or for a specific one.
e.g.
const { setConnectionType } = useConfig();
// set for default chain
setConnectionType('light-client');
// set for a specific chain
setConnectionType('light-client', 'aleph');
// set for all chains
setConnectionType('light-client', 'all');
Maybe setConnectionType
is defined like this:
type Chain = ChainId | 'all';
const setConnectionType = (type: ConnectionType, chain?: Chain) => void;
- If
chain
is omitted as an argument, it uses the default chain. - If
chain
has the valueall
, setconfig.api
to{ default: '<type>' }
- If
chain
is a ChainId then setconfig.api[chainId] = '<type>'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx, made some changes, will continue with ApiProvider
in the upcoming commits
if (!c) { | ||
config.api = defaultChainId; | ||
return; | ||
} | ||
if (c === "all") { | ||
config.api = { default: type }; | ||
return; | ||
} | ||
config.api[c.id] = type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used early returns to keep it flat and avoid if/ else
useEffect(() => { | ||
if (api?.default === "light-client") { | ||
const provider = new ScProvider(Sc, "all"); | ||
|
||
provider.connect(); | ||
} | ||
}, [api]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DoubleOTheven in this comment you mentioned
Then inside of ApiProvider you use useConfig() to get the values and create the instances accordingly. This will update automatically when the config is changed
I tried to use useEffect
to update automatically, but not entirely sure if this is the right approach and/ or if the other useEffect
becomes obsolete.
Let me know how do you envision APIProvider
to behave.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think there is a better approach you can suggest it. I was only giving some ideas.
useEffect
can be used to update the state of ApiProvider if some other state changes.
import { APIContext } from './context.ts' | ||
import { apiProvidersReducer } from './reducer.ts' | ||
import { useConfig } from '../../mod.ts' | ||
import React, { useEffect, useReducer } from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it looks like your IDE formatter is set to something other than Deno. Deno should be the formatter. I think this will fix the issue where '
is converted to "
Resolves #40
Description