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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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:

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 32 to 49 in f515113

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:

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 51 to 66 in f515113

// 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:

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 68 to 94 in f515113

// 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):

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 96 to 117 in f515113

// 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:

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 119 to 149 in f515113

// 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:

web/packages/react/src/components/penumbra-context-provider.tsx

Lines 151 to 196 in f515113

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.
@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra web Jul 22, 2024
@grod220 grod220 mentioned this issue Jul 22, 2024
Merged
@turbocrime
Copy link
Contributor

turbocrime commented Jul 25, 2024 •
edited
Loading

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 this to Labs web Jul 25, 2024
@grod220 grod220 moved this to 📝 Todo in Labs web Jul 25, 2024
@turbocrime turbocrime mentioned this issue Jul 26, 2024
Closed
@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
react hooks #1687
Draft
@grod220 grod220 moved this from 📝 Todo to 🗄️ Backlog in Labs web Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
refactor Improving existing system with new design
Projects
Penumbra web
Status: 🗄️ Backlog
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

react hooks penumbra-zone/web
2 participants
@grod220 @turbocrime