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

Renamed: Create React package for updated client package code #1523

Open
grod220 opened this issue Jul 22, 2024 · 1 comment · May be fixed by #1687
Open

Renamed: Create React package for updated client package code #1523

grod220 opened this issue Jul 22, 2024 · 1 comment · May be fixed by #1687
Labels
refactor Improving existing system with new design

Comments

@grod220
Copy link
Collaborator

grod220 commented Jul 22, 2024

Update Aug 26th:

  • Determine whether we need this or not (if not unpublish react package)
  • Consider creating queriers that use the connectRPC package (useBalance, useAddress)
  • Simply things by using helper hooks (performing functions we expect people to use, e.g. aggregate and display balances)

====

Now that Prax v11.14 is live and client package is updated, we should re-write the react package to support it.

=====

Was relevant in last React package:

The PenumbraContextProvider is a react component that consumers are required to wrap around their app.

It should be possible to convert this to a hook so consumers aren't required to wrap their app in it.

PenumbraState has a failure state, so this shouldn't be necessary:

const setFailure = useCallback(
(cause: unknown) => {
if (failure) {
console.warn('PenumbraContextProvider not replacing existing failure with new cause', {
failure,
cause,
});
}
setProviderConnected(false);
setProviderPort(undefined);
setProviderState(PenumbraState.Failed);
dispatchFailure(
failure ?? (cause instanceof Error ? cause : new Error('Unknown failure', { cause })),
);
},
[failure],
);

assertGlobalPresent() in the client package performs this:

// fetch manifest to confirm presence of provider
useEffect(() => {
// require provider manifest uri, skip if failure or manifest present
if (!penumbra?.manifest || (failure ?? providerManifest)) {
return;
}
// abortable effect
const ac = new AbortController();
void getPenumbraManifest(new URL(penumbra.manifest).origin, ac.signal)
.then(manifestJson => ac.signal.aborted || setProviderManifest(manifestJson))
.catch(setFailure);
return () => ac.abort();
}, [failure, penumbra?.manifest, providerManifest, setFailure, setProviderManifest]);

This will have to remain, but can live in a hook and attach itself on mount:

// attach state event listener
useEffect(() => {
// require penumbra, manifest. unnecessary if failed
if (!penumbra || !providerManifest || failure) {
return;
}
// abortable listener
const ac = new AbortController();
penumbra.addEventListener(
'penumbrastate',
(evt: Event) => {
if (isPenumbraStateEvent(evt)) {
if (evt.detail.origin !== new URL(penumbra.manifest).origin) {
setFailure(new Error('State change from unexpected origin'));
} else if (evt.detail.state !== penumbra.state()) {
console.warn('State change not verifiable');
} else {
setProviderState(penumbra.state());
setProviderConnected(penumbra.isConnected());
}
}
},
{ signal: ac.signal },
);
return () => ac.abort();
}, [failure, penumbra?.addEventListener, providerManifest, penumbra?.manifest, setFailure]);

Should not exist (#1522):

// request effect
useEffect(() => {
// require penumbra, manifest, no failures
if (penumbra?.request && providerManifest && !failure) {
switch (providerState) {
case PenumbraState.Present:
if (makeApprovalRequest) {
void penumbra.request().catch(setFailure);
}
break;
default:
break;
}
}
}, [
failure,
makeApprovalRequest,
penumbra?.request,
providerManifest,
providerState,
setFailure,
]);

Can be moved to hook:

// connect effect
useEffect(() => {
// require manifest, no failures
if (penumbra && providerManifest && !failure) {
switch (providerState) {
case PenumbraState.Present:
if (!makeApprovalRequest) {
void penumbra
.connect()
.then(p => setProviderPort(p))
.catch(setFailure);
}
break;
case PenumbraState.Requested:
void penumbra
.connect()
.then(p => setProviderPort(p))
.catch(setFailure);
break;
default:
break;
}
}
}, [
failure,
makeApprovalRequest,
penumbra?.connect,
providerManifest,
providerState,
setFailure,
]);

Can be exposed by hook:

const createdContext: PenumbraContext = useMemo(
() => ({
failure,
manifest: providerManifest,
origin: penumbra?.manifest && new URL(penumbra.manifest).origin,
// require manifest to forward state
state: providerManifest && providerState,
transport:
providerConnected && providerPort
? createChannelTransport({
jsonOptions,
...transportOpts,
getPort: () => Promise.resolve(providerPort),
})
: undefined,
transportOpts,
// require penumbra, manifest and no failures to forward injected things
...(penumbra && providerManifest && !failure
? {
port: providerConnected && providerPort,
connect: penumbra.connect,
request: penumbra.request,
disconnect: penumbra.disconnect,
addEventListener: penumbra.addEventListener,
removeEventListener: penumbra.removeEventListener,
}
: {}),
}),
[
failure,
penumbra?.addEventListener,
penumbra?.connect,
penumbra?.disconnect,
penumbra?.manifest,
penumbra?.removeEventListener,
penumbra?.request,
providerConnected,
providerManifest,
providerPort,
providerState,
transportOpts,
],
);

All told, I don't see a reason to require consumers to have a global context for this when a hook would be easier.

@turbocrime
Copy link
Contributor

turbocrime commented Jul 25, 2024

it's a scoped context. a provider doesn't have to be used at the top level of the page.

the provider can be used as a top level component to provide a scope that encompasses the page.

or it can be used in a smaller component, and will then be scoped only part of the page. this would allow a dapp to connect to multiple providers at once.

the context provider and hook model is common among other react wallet packages and APIs.

i've done some work to make a hook executing without a provider use a default global scope. i'll push that branch up soon as a PR.

@grod220 grod220 added the refactor Improving existing system with new design label Aug 9, 2024
@grod220 grod220 assigned turbocrime and unassigned turbocrime Aug 12, 2024
@grod220 grod220 changed the title Remove Provider in favor of hook Renamed: Create React package for updated client package code Aug 12, 2024
@VanishMax VanishMax linked a pull request Sep 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improving existing system with new design
Projects
Status: 🗄️ Backlog
Development

Successfully merging a pull request may close this issue.

2 participants