Relay: "Uncaught TypeError: Cannot read property 'identifier' of undefined" when calling `retain()` in Relay 8.0.0

[ivosabev]

This is in an example 1:1 from the official documentation https://relay.dev/docs/en/local-state-management

commitLocalUpdate(modernEnvironment, store => {
  const dataID = 'client:store';
  const record = store.create(dataID, 'Store');

  modernEnvironment.retain({
    dataID,
    variables: {},
    node: {selections: []},
  });
});

that causes the following error:

Uncaught TypeError: Cannot read property 'identifier' of undefined
    at RelayModernStore.retain (RelayModernStore.js:158)
    at RelayModernEnvironment.retain (RelayModernEnvironment.js:223)

I did a bare bones setup based on relay-examples todo app. Nothing fancy.

Not sure if related: https://github.com/facebook/relay/issues/2971

[sibelius]

I think it is related to this commit https://github.com/facebook/relay/commit/1ce3e474a59095773a9e28bd30ae5e2dc32203f0

[ivosabev]

Why was this not documented as a breaking change? Also for local schema is it still necessary to retain at all, the new way to retain looks excessively complicated for something so simple?

[sibelius]

can you just change like this

environment.retain(operation.root) to evironment.retain(operation)

[ivosabev]

This

modernEnvironment.retain({
    dataID,
    variables: {},
    node: {selections: []},
  });

to this?

modernEnvironment.retain({
    request: {identifier: 0}, // HACK
    root: {
        dataID,
        variables: {},
        node: {selections: []},
    }
});

I've figured adding request: {identifier: 0} as a sibling of root would make it work, but this looks really hacky. What is the proper way of dealing with this and why are the docs not updated 😞

[jstejada]

hey @ivosabev , sorry we totally missed to document this breaking change, I just updated the release notes.

The way to get an operation descriptor to pass to retain is to use the createOperationDescriptor function available from relay-runtime.

[ivosabev]

Could you update the docs to reflect how to use it to initialize a local store as the docs are very unclear on what to do. Thanks!

[jstejada]

@ivosabev what do you mean by initialize a lcoal store? do yu mean how t owrite data locally to the stoore?

Would these docs help? https://relay.dev/docs/en/experimental/a-guided-tour-of-relay#local-data-updates

[sibelius]

I think he wants an example of retain using createOperationDescriptor

[ivosabev]

Based on the Relay < 8 docs, you had to call retain when you create your local data and inject it into the Relay Store. (https://relay.dev/docs/en/local-state-management.html#create).

It is unclear to me what should the correct approach be in Realy 8+, and should we still call retain and how to achieve the same result as before.

Here is my a simplified snippet of my current code to get the idea:

# localSchema.graphql

type Storage {
  currentTeamId: Int
  dispatcherDate: Date
  dispatcherView: String
}

extend type Query {
  storage: Storage!
}

// relay.js

// some other Relay initialization stuff

commitLocalUpdate(environment, store => {
  const dataID = 'client:storage';
  const record = store.create(dataID, 'Storage');

  record.setValue(DEAFULT_TEAM_ID, 'currentTeamId');
  record.setValue(DateTime.local().toFormat('yyyy-MM-dd'), 'dispatcherDate');
  record.setValue('plan', 'dispatcherView');

  store.getRoot().setLinkedRecord(record, 'storage');

  environment.retain({
    root: {
      dataID,
      variables: {},
      node: {selections: []},
    },
  });
});

[enisdenjo]

Say you want to add a selected field through client extensions and retain the selected items state. How would you achieve this with the new retain signature?

This is the way I made it work now:

# remote.graphql

type Document {
  id: ID!
  title: String!
}

```graphql

local.graphql

extend type Document {
selected: Boolean
}

```js
// document.js

function selectDocument(id, selected) {
  environment.commitUpdate((store) => {
    const record = store.get(id);
    if (!record) {
      // if the document does not exist, nothing should be done
      return;
    }

    record.setValue(selected, 'selected');

    // retain selected documents so that the `selected` state is preserved throughout the app
    if (selected && !retainedDocuments[id]) {
      retainedDocuments[id] = environment.retain({
        request: { identifier: `${id}-selected` }, // :(
        root: {
          dataID: id,
          variables: {},
          node: {
            selections: [
              {
                kind: 'ScalarField',
                name: 'selected',
              },
            ],
          },
        },
      });
    } else if (!selected && retainedDocuments[id]) {
      retainedDocuments[id].dispose();
      delete retainedDocuments[id];
    }
  });
}

[enisdenjo]

I am back with revelations. Thanks to help from @josephsavona, I've figured out a generic way to solve my problem in the above comment.

Relay offers missingFieldHandlers which allows it to populate/inject fields which are seemingly missing from the record store.

Empowering this feature, we can write type-safe retention requests for records which don't exist on the root.

Following the Relay GraphQL Server Specification, we know for a fact that when the user requests something like this:

query {
  node(id: "globallyUniqueID") {
    ... on Document {
      title
    }
  }
}

we want a node/record with the globally unique ID behind the id argument. The following missing field handler allows such queries to get resolved:

// environment.js

import { Environment, ROOT_TYPE } from 'relay-runtime';
export const environment = new Environment({
  store,
  network,
  missingFieldHandlers: [
    {
      kind: 'linked',
      handle: function handle(field, record, variables) {
        if (
          // originates from root
          record?.__typename === ROOT_TYPE &&
          // name of the field equals to `node`
          field.name === 'node' &&
          // requests the record by ID
          field.args[0]?.name === 'id'
        ) {
          const arg = field.args[0];
          if (arg.kind === 'Literal') {
            return arg.value;
          }
          return variables[arg.variableName];
        }
        return undefined;
      },
    },
  ],
});

Refactoring the original document.js from the above comment like this:

// document.js

import { graphql, createOperationDescriptor, getRequest } from 'relay-runtime';
function selectDocument(id, selected) {
  let documentExists = false;
  environment.commitUpdate((store) => {
    const record = store.get(id);
    if (record) {
      documentExists = true;
      record.setValue(selected, 'selected');
    }
  });

  // retain selected documents so that the `selected` state is preserved throughout the app
  if (documentExists && selected && !retainedDocuments[id]) {
    const operation = createOperationDescriptor(
      getRequest(graphql`
        query documentRetainSelectedQuery($id: ID!) {
          node(id: $id) {
            ... on Document {
              selected
            }
          }
        }
      `),
      { id },
    );
    // checking the operation triggers the `missingFieldHandlers` and updates the store if necessary
    environment.check(operation);
    retainedDocuments[id] = environment.retain(operation);
  } else if (!selected && retainedDocuments[id]) {
    retainedDocuments[id].dispose();
    delete retainedDocuments[id];
  }
}

cleanly implements the wanted behaviour with all Relay benefits!

[sibelius]

should we add this to the docs?

[jstejada]

Got it, there are a few things going on here:

Regarding @ivosabev's comment, I think the problem here was that those docs are stale. I'm updating them now to show you how to call retain using createOperationDescriptor. Generally retain now always takes an OperationDescriptor`

---

Regarding @enisdenjo's original comment:

"Say you want to add a selected field through client extensions and retain the selected items state. How would you achieve this with the new retain signature?"

I'm not entirely sure I follow the issue you're describing, but just a couple of clarifications:

• Retaining an OperationDescriptor (e.g. representing a query or mutation), will caused the data referenced by that operation to be retained, meaning that Relay won't garbage collect that data as long as it is retained. If you're trying to retain client-defined data, then you will need an operation (e.g. a query) that selects that data and retain it. You probably don't need to retain inside the updater, and can only retain the root query once
• As you've mentioned, missingFieldHandlers are a way to "fill in" data that is missing when we check a query. I'm not entirely sure how this relates to the retaining issue you described, but yes, this is a way to achieve that. There are experimental docs for that here (which probably needs to be updated): https://relay.dev/docs/en/experimental/a-guided-tour-of-relay#filling-in-missing-data-missing-data-handlers

[enisdenjo]

@jstejada thanks for the clarifications. I'll try to describe the issue a bit better.

I have a selected, client extended, boolean field on the Document type. This field indicates if a given document is selected in a list view.

This is all great, until you navigate away from the list view and all React imposed retentions are released because elements (hooks/components) referencing the selected field are unmounted.

To avoid this loss of state, I want to retain the selected field on specific selected documents. This is exactly what I am trying in the code examples.

• When selecting a document, I ask Relay to retain the selected field of that exact document and add the retention lock to the retainedDocuments map
• When deselecting a document, I check the retainedDocuments map and dispose of the retention lock because I don't care about it anymore

I hope this clarifies stuff from my side.

[jstejada]

Got it, thanks for the clarification @enisdenjo.

So in your latest example, it seems like you're already correctly retaining the correct data for the selected document by retaining a query that references the selected document. And you needed the missingFieldHandler to tell Relay to look up any record that has that id when looking up a node, so it would find the correct Document?

Thanks!

[enisdenjo]

Yes exactly, @jstejada.

The issue I have had, with the new retain method signature, is that I wasn't able to write a GraphQL query which would find me a record by its globally unique ID from the store. If, of course, it is not linked from the root node.

I've had a discussion with @josephsavona on Slack about how to use the lookup method to do such a lookup. Ultimately, solving my issue from the comment as well.

If you are interested in further clarifications, I did a small example here:
https://github.com/bhidapa/heltin/blob/5656f6c1712bc850bd9a6d7923afd2b65ddad668/heltin-app/src/core/Example.tsx#L16-L91

What I do is request a Client using the field clientByRowId from the remote server, populating the store; and then looking up that exact Client by its dataID using client(id: ID!) or node(id: ID!). This was not possible until I introduced the missingFieldHandlers:
https://github.com/bhidapa/heltin/blob/5656f6c1712bc850bd9a6d7923afd2b65ddad668/heltin-app/src/relay/environment.ts#L22-L67

[sibelius]

can we close after this change in the docs https://github.com/facebook/relay/commit/2021d5c13f6f2eaddeb7e0a223316ddafff157ec

[cvle]

This is a bigger breaking change for us. Is there no way to not garbage collect a record (retain) by only knowing its DATA_ID anymore like before? So now I'll always have to create a whole query to retain it?

[jstejada]

hey @cvle, unfortunately yes. The previous api was also only meant to be used on operations and this formalized it a bit more. I /think/ (but could be wrong) that because we use Flow internally that function wasn’t being type checked externally, and it’s an internal-ish api that isn’t well documented, so people figured out how to handcraft an input that would allow retaining a single record.

That being said, technically you could also handcraft an operationdescriptor to pass to retain without actually creating a query if you really need to, but that would not be the recommended way to use the api.

In any case, i think retaining individual records is probably a valid use case for managing local data, so we’ll consider adding a variant of this api for that use case.

thanks!

[cvle]

Thank you @jstejada! This works for me for now:

  /**
   * Retain a record with `id`.
   * This is not the recommended way of using the api!
   * See https://github.com/facebook/relay/issues/2997
   */
  const query = {
    kind: "Request",
    operation: {
      kind: "Operation",
      argumentDefinitions: [],
      selections: [],
    },
    params: {
      id,
      operationKind: "query",
    },
  };
  const operation = createOperationDescriptor(getRequest(query), {}, id));
  environment.retain(operation);

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.