Skip to content

feat: read and write hooks for invite-over-internet - #211

Open
achou11 wants to merge 36 commits into
mainfrom
experiments/invite-over-internet
Open

achou11 wants to merge 36 commits into
mainfrom
experiments/invite-over-internet

Conversation

@achou11

@achou11 achou11 commented Oct 26, 2026

Copy link
Copy Markdown
Member

Closes #206

  • Introduces the following hooks:

    • Read: useManyInviteLinks, useManyJoinRequests, useSingleJoinRequest
    • Write: useCreateInviteLink, useCancelInviteLink, useAcceptInviteLinkRequest, useDenyInviteLinkRequest, useCreateJoinRequest, useCancelJoinRequest
  • Bumps minimum version of @tanstack/react-query peer dep due to internal implementation details that are using added + changed APIs.

  • Where appropriate, the new write hooks are implemented such that the relevant project ID becomes a parameter of the write hook mutate call instead of the hook itself. This allows more flexibility for where the write hooks can be used within an application. There are plans to port other write hooks in this library to use this approach as well (see Update write hooks that apply to a project to accept the ID as a mutation param #195).

  • Tests are set up similarly to core, using hyperdht to create a testnet for internet-based discovery.

@awana-lockfile-bot

Copy link
Copy Markdown

package-lock.json changes

Click to toggle table visibility
Name Status Previous Current
@comapeo/core UPDATED 7.5.2 7.7.0
adaptive-timeout ADDED - 1.0.1
bits-to-bytes ADDED - 1.3.0
blind-relay ADDED - 1.6.1
compact-encoding-bitfield ADDED - 1.1.0
crockford-base32 ADDED - 2.1.0
dht-rpc ADDED - 6.27.0
generate-object-property ADDED - 2.0.0
generate-string ADDED - 1.0.1
hyperdht-address ADDED - 1.1.1
hyperdht ADDED - 6.34.1
hyperschema ADDED - 1.26.2
hyperswarm ADDED - 4.17.2
is-property ADDED - 1.0.2
kademlia-routing-table ADDED - 1.0.6
nat-sampler ADDED - 1.0.1
record-cache ADDED - 1.2.0
shuffled-priority-queue ADDED - 2.1.0
signal-promise ADDED - 1.0.3
time-ordered-set ADDED - 2.0.1
udx-native ADDED - 1.21.3
unordered-set ADDED - 2.0.1

@socket-security

socket-security Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​comapeo/​core@​7.5.2 ⏵ 7.7.07710084 +198100
Addedhyperdht@​6.34.19810010094100

View full report

@achou11
achou11 marked this pull request as draft October 26, 2026 21:00
@achou11
achou11 marked this pull request as ready for review October 29, 2026 15:18
Comment thread src/contexts/ClientApi.ts
Comment on lines +50 to +67
queryClient.invalidateQueries({
queryKey: getJoinRequestsQueryKey(),
// NOTE: Only want to invalidate the query for getting all join requests (no cascade)
exact: true,
})

// NOTE: Directly update query for getting specific join request
queryClient.setQueryData<InviteLinkJoiner.JoinRequest>(
getJoinRequestsByIdQueryKey({ inviteId: update.inviteId }),
(prev) => {
if (!prev) {
return prev
}

return { ...prev, ...update }
},
)
}

@achou11 achou11 Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Part of me is a little bit hesitant on the implementation here. We invalidate the query used to get all of the join requests but we manually update the data for the query for affected join request. Usually I prefer to just call invalidateQueries with the appropriate cascade and call it a day to avoid issues with client vs server state misalignment, but I think it's a bit more complicated in this case...

Granular updating of the single join request query data accounts for core removing access to the join request once it's in a "completed" state (e.g. status is completed, failed, etc). Without granular updating, what happens is something like the following:

  1. Call useSingleJoinRequest()
  2. completed event emits and invalidate all queries related to join request
  3. Query from (1) refetches from core, which throws a not found error, and its data still reflects an "in-progress" status e.g. status is connected. basically, the data from (1) will never be able to represent a "completed" join request in this case.

From an end-user perspective, this sequence requires more complicated workarounds to support UI where a join request's completed state needs to be represented. Sometimes this is manageable enough to let the consuming app to handle it via the event listener + useState, but based on discussions with Cindy I think it makes more sense do that work here instead and rely on the facilities provided by react query.

The potentially confusing bit with this implementation is that we don't attempt to keep a reference to the completed join request for the query that gets all join requests, and still just rely on what core returns in this case (which is a list of only pending ones). Based on our UI requirements, I don't think this is an issue, but the inconsistency between this vs a specific join request might cause confusion. I've also made sure that the query for the single join request is configured to not rely too heavily on the query cache and do refetches when appropriate, which should mitigate the misalignment of client vs server state for this hook.

One can argue that we should take this approach for the hooks related to the local invites API as well, which have similar limitations as the invite links API. Something to consider for follow-up based on how it works out here...

Comment thread src/hooks/invites.ts
Comment on lines +268 to +271
// NOTE: Ensure garbage collection occurs immediately when all observers are unmounted
// so that newly active instances only use data from refetch.
// https://tanstack.com/query/latest/docs/framework/react/guides/caching
gcTime: 0,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Related to my other comment about updating the query cache manually when relevant join request events are received.

We don't want to hold on to cached values once all relevant queries are no longer active, such that any new queries that are introduced will rely on the result of the fetch result instead of the cache (i.e. the true server state from core).

Comment thread src/hooks/projects.ts

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Worth noting the new approach for write hooks being used in the newly introduced write hooks in this file. Basically:

  1. Make projectId a mutation parameter
  2. Get the active project client instance using queryClient.query() within the mutation function. The query options used ensure that we only ever attempt refetch the instance if there isn't already one in the cache, which should be aligned with how we configure the query that populates the cache with the project client instance to begin with.

Comment thread src/hooks/invites.ts
> {
const clientApi = useClientApi()

const { data, error, isRefetching } = useSuspenseQuery({

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

looks like our custom errors in core are not being preserved when stored in data.error, meaning doing something like this doesn't currently work as intended:

import { getErrorCode } from '@comapeo/core/errors.js'

function App() {
  const { data } = useSingleJoinRequest({ inviteId })

  if (getErrorCode(data.error) === ...) {
    // Not reaching this
  }
}

Based on Mauve's initial guess, seems to be an issue with nested errors in an event listener payload. Probably an issue in ipc or rpc-reflector. Mauve mentioned maybe adjusting the shape of the event listener callback to avoid the nesting to side step the issue as an alternative approach.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, this will be the reason. I don't think the fix belongs in IPC or rpc-reflector: it's just not possible to send anything that can't be serialized to JSON between processes. rpc-reflector serializes then deserialize errors that are thrown by any function, and does maybe a bit of magic to serialize an error emitted by a listener, but these are built into the rpc protocol shape (the serialized error is sent separately). Auto-detecting any errors in a nested shape is beyond what rpc-reflector should be doing, and there is no way to neatly detect them on the client without doing a lot of magic.

I think we should either emit an error, or emit a JSON-serializable object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for invite-over-internet

2 participants