fix: use identifierAttribute to get id name when possible (#215)

[dmytro-shpak]

[dmytro-shpak]

it still does not works correctly because of MSTGQLRef implementation. it uses the id property in setter. It is possible to get the proper 'identifierAttribute' with targetType.getSubType().identifierAttribute, but i can not find the getSubType method in the mst spec

[dmytro-shpak]

The workaround is to add the *id view in your model

export const MyModel = MyModelBase
  .actions(self => ({
    // This is an auto-generated example action.
    log() {
      console.log(JSON.stringify(self))
    }
  })).views(self => ({
    get id () {
      return self.myCustomId
    }
  }))

[jesse-savary]

Will look at this soon.

[mrvinceit]

The workaround is to add the *id view in your model

export const MyModel = MyModelBase
  .actions(self => ({
    // This is an auto-generated example action.
    log() {
      console.log(JSON.stringify(self))
    }
  })).views(self => ({
    get id () {
      return self.myCustomId
    }
  }))

@dmytro-shpak , I've tried the workaround you mentioned it doesn't seem to avoid the issue in https://github.com/mobxjs/mst-gql/issues/215. In my case, my custom id is "_id".

Specifically, I added the view to the ModelBase object, so that all entities can provide the "id" alias for "_id". I've also tried setting the view on individual model and no luck. Any thoughts? Thanks!

[dmytro-shpak]

Specifically, I added the view to the ModelBase object, so that all entities can provide the "id" alias for "_id". I've also tried setting the view on individual model and no luck. Any thoughts? Thanks!

Hi @mrvinceit have you also add https://github.com/mobxjs/mst-gql/pull/379/files ?

[mrvinceit]

Hey @dmytro-shpak, yes I created a patch that included the changes in https://github.com/mobxjs/mst-gql/pull/379. In my case, I only patched the dist files (via patch-package). Because the view approach wasn't working I also tried the following:

      let id = data['_id']
      if (!id)
        id = data[typeDef.identifierAttribute ?? 'id']

It worked to load the object partially but things are still a bit wonky.

I also noticed the deflate function might need patching as well?

[mrvinceit]

@dmytro-shpak, yes typeDef.identifierAttribute is correctly defined as _id. Though I still couldn't get the id() view to work from me. The earlier issue I was having after adopting https://github.com/mobxjs/mst-gql/pull/379, was that reloading objects from the store that were previously loaded successfully would throw the following:

 Error: [mobx-state-tree] Error while converting `{"__typename":"Bot","id":"62b9ed1c18d8867eeb464409"}` to `Bot`:
    at path "/_id" value `undefined` is not assignable to type: `identifier` (Value is not a valid identifier, expected a string).

I was able to resolve this by modifying the deflate function to use the typeDef.identifierAttribute based id ('_id') directly as seen below:

function deflateHelper(store, data) {
  function deflate(data) {
    if (!data || typeof data !== "object") return data;
    if (Array.isArray(data)) return data.map(deflate);
    const { __typename } = data;

    if (__typename && store.isRootType(__typename)) {
      const typeDef = store.getTypeDef(__typename);

      // Infer id attribute from type definition, with default of 'id'
      const identifierAttribute = typeDef.identifierAttribute ?? 'id'

      const id = data[identifierAttribute]

      // GQL object with root type, keep only __typename & id
      return {
        __typename,        
        [identifierAttribute]: id,
      };
    } else {
      // GQL object with non-root type, return object with all props deflated
      const snapshot = {};

      for (const key in data) {
        snapshot[key] = deflate(data[key]);
      }

      return snapshot;
    }
  }

  return deflate(data);
}

So there is a need to also pass the custom id in the deflate function for it to reload objects from the store which addresses this comment https://github.com/mobxjs/mst-gql/issues/215#issuecomment-617250784

[dmytro-shpak]

@mrvinceit, confirm. The deflare function is used to make a record to cache and then cached data could go to merge. I think this change also need some more tests :-D

[mrvinceit]

@dmytro-shpak, sure what specifically do you want me to look out for, in addition to what I posted in https://github.com/mobxjs/mst-gql/pull/379#issuecomment-1196973314?