Get all connections for a given key

[taion]

There's already a TODO for this (https://github.com/facebook/relay/blob/v1.0.0/packages/relay-runtime/handlers/connection/RelayConnectionHandler.js#L230-L233), but I wanted to open an issue for public visibility.

It would be really nice to have something like a getConnections or getAllConnections as described. In many cases, it's nice to not have to explicitly enumerate the connection variables I care about. When the connection filters aren't based on e.g. an enum, it may in fact be quite difficult to pass through the correct filter to my mutation to update the currently displayed connection, without jumping through hoops.

For example, in TodoMVC with routing, per https://github.com/taion/relay-todomvc/blob/98f473abb07fcd1c4340ebcd9b48d350b7c0c818/src/mutations/RemoveTodoMutation.js#L19-L26, I end up having to write:

function sharedUpdater(store, user, deletedId) {
  const userProxy = store.get(user.id);

  ['any', 'active', 'completed'].forEach((status) => {
    const connection = ConnectionHandler.getConnection(
      userProxy, 'TodoList_todos', { status },
    );
    if (connection) {
      ConnectionHandler.deleteNode(connection, deletedId);
    }
  });
}

I can't use filters: [] because in the "add" mutation, I don't want to append to the connection if it's filtered to only completed todos, per https://github.com/taion/relay-todomvc/blob/98f473abb07fcd1c4340ebcd9b48d350b7c0c818/src/mutations/AddTodoMutation.js#L26-L33:

function sharedUpdater(store, user, todoEdge) {
  const userProxy = store.get(user.id);

  ['any', 'active'].forEach((status) => {
    const connection = ConnectionHandler.getConnection(
      userProxy, 'TodoList_todos', { status },
    );
    if (connection) {
      ConnectionHandler.insertEdgeAfter(connection, todoEdge);
    }
  });
}

Unless I'm missing something, this isn't ideal.

[josephsavona]

Our idea here was that ConnectionHandler would write - in a client field (e.g. __connections) - a list of all the connections (key and filters) that have been fetched for a given schema field. Then getConnections() would basically iterate over that and return the corresponding connection records.

The use of a client field is similar to the way the next edge id is stored today.

[taion]

So this would be tied to the schema field, not the key in @connection?

[josephsavona]

Probably store all connections for a given schema field, and have getConnections() (all) and getConnectionsForKey(key).

[taion]

I think naming-wise the better parallel might be:

ConnectionHandler.getConnection(record, key, filters) -> connection
ConnectionHandler.getConnections(record, key) -> [{ filters, connection }]
ConnectionHandler.getAllConnections(record, fieldName) -> [{ key, filters, connection }]

This looks a bit complicated code-wise, though. Right now it looks like HandleFieldPayload doesn't really give access to the raw selection field name, the raw selection key, or the actual filters.

[josephsavona]

@taion Ah, forgot about that. It's probably reasonable to ignore the schema field name and just go based on the @connection key name. filters isn't well integrated into the HandleField system; the filters list should probably become part of the payload itself.

[taion]

Hmm, the RecordProxy interface doesn't really expose a way to record the filters at all, as far as I can tell.

The best I can think of is maybe to invent a local type like ConnectionAndFilters, and store the filters as a JSON-serialized string? Seems clunky. Am I missing something?

[JenniferWang]

@taion We actually considered about ConnectionHandler.getConnections(record, key) -> [{ filters, connection }] when we first introduced filters. We didn't added at that time since by supporting that, we need somehow either regrex the key in the record source or add additional meta data for every connection, which is expensive.

[taion]

What's the right path forward, then? I think in most cases when you would want to use getConnections as above, you do actually want to get the filters.

[josephsavona]

Storing a small amount of metadata per connection isn't that expensive, I don't see how that would be a blocker for supporting this. @jenniferwang can you clarify?

[JenniferWang]

@josephsavona What I was thinking is to have a special kind of record in the store of which the key is connectionKey and the value is a bag of the actual key of the connection record. (pointers) My concern is that we need to iterate over this bag during GC as well. Not sure if it is expensive or not. Another thing is that we tried to make the Relay implementation align with native GraphQL client implementation and we don't support this on native side. Maybe trying it out on Relay is a good start?

[josephsavona]

Ah, yeah storing this as a linked record gets complicated and introduces overhead for GC. What I was thinking is storing the data as a scalar field on the parent object, where the scalar field value is actually an object. We need to relax the restriction of RecordProxy#setValue() to allow writing "scalar" (non-record) objects anyway, to support things like the JSON scalar type that some schemas use.

[taion]

Would it be possible to just store connectionKey as the value?

[wincent]

Thanks for the input here. We're currently going through old issues that appear to have gone stale (ie. not updated in about the last 3 months) because the volume of material in the issue tracker is becoming hard to manage. If this is still important to you please comment and we'll re-open this.

Thanks once again!

[taion]

This is still a problem.

[johanobergman]

Yep, it really is. It was already adressed once in relay classic (making rangeBehaviors a function), and it would be great to have a solution in relay modern.

[wincent]

Re-opening. Thanks for chiming in.

[taion]

We're moving one of our larger apps to Relay Modern very belatedly now.

How does this API look?

function getConnections(
  record: RecordProxy,
  key: string,
  filter?: (variables: Variables) => boolean,
)

The idea here is that we want to pass in a filter function, analogous to rangeBehaviors as a function.

Implementation-wise, this would look like doing a prefix search on all linked records, hydrating the filters in the storage, then running the match.

Does that sound reasonable?

[ntelkedzhiev]

Any updates on this?

[sibelius]

@ntelkedzhiev what is your use case?

[ntelkedzhiev]

I want to add an edge to all relevant connections while respecting the applied filters.

So let's say I have a connection called Tasks_tasks with filters: [{dateRange: [start: DD.MM.YYYY, end: DD.MM.YYYY]}], and I have an incoming edge with a date prop, I want to be able to filter the connections and only add the edge if its date in within the range of the connection filter.

[sibelius]

I think for your use case is better to create another handler to handle this behaviour

[taion]

Oops misclicked.

This is pretty generic functionality though. Relay Classic supported this with passing in functions for range behaviors.

[taion]

This turns out to be not-too-bad in userspace. It looks like there's already an internal ticket to track this, but here's a short, only-slightly-hacky implementation:

import {
  ConnectionHandler,
  RecordProxy,
  Variables,
  getRelayHandleKey,
} from 'relay-runtime';
import {
  HandleFieldPayload,
  ReadOnlyRecordProxy,
  RecordSourceProxy,
} from 'relay-runtime/lib/store/RelayStoreTypes';
import { getStableStorageKey } from 'relay-runtime/lib/store/RelayStoreUtils';

const CONNECTION_HANDLE_KEYS = '__connectionHandleKeys';

function update(store: RecordSourceProxy, payload: HandleFieldPayload) {
  ConnectionHandler.update(store, payload);

  // Hackishly get the handle key minus the args.
  const [handleName] = payload.handleKey.split('(');

  const record = store.get(payload.dataID)!;

  const prevHandleKeys: {} | undefined = record.getValue(
    CONNECTION_HANDLE_KEYS,
    {
      handleName,
    },
  ) as any;
  const nextHandleKeys = {
    ...prevHandleKeys,
    [payload.handleKey]: payload.args,
  };

  // FIXME: The RecordProxy API doesn't let us set objects as values. We bypass
  //  this validation by reaching into internals.
  // eslint-disable-next-line no-underscore-dangle
  const mutator: any = (record as any)._mutator;

  // It's slightly janky to store this as a value rather than as linked
  //  records, but this will prevent the value from getting GCed so long as the
  //  parent record stays live, which is what we want. These data are light
  //  enough that this shouldn't take too much memory.
  mutator.setValue(
    record.getDataID(),
    getStableStorageKey(CONNECTION_HANDLE_KEYS, { handleName }),
    nextHandleKeys,
  );
}

function connectionExists(
  connection: RecordProxy | null | undefined,
): connection is RecordProxy {
  return !!connection;
}

function getConnections(
  record: ReadOnlyRecordProxy,
  key: string,
  filter?: (variables: Variables) => boolean,
): Array<RecordProxy> {
  const handleName = getRelayHandleKey('connection', key, null);

  const handleKeys: Record<string, Variables> | undefined = record.getValue(
    CONNECTION_HANDLE_KEYS,
    {
      handleName,
    },
  ) as any;
  if (!handleKeys) {
    return [];
  }

  return Object.entries(handleKeys)
    .filter(([_handleKey, args]) => !filter || filter(args))
    .map(([handleKey]) =>
      // XXX: This takes advantage of how a full storageKey with args can be
      //  used in place of name and args separately.
      record.getLinkedRecord(handleKey),
    )
    .filter(connectionExists);
}

export default {
  ...ConnectionHandler,
  update,
  getConnections,
};

I'm going to open source this shortly (really once I figure out a name for it).

[taion]

Okay, I've open-sourced this as https://github.com/relay-tools/relay-connection-handler-plus. I'm going to close this issue out for now.