datx-jsonapi `model.id` is not set after save / create about datx HOT 6 CLOSED

Hi @re5et,

The direct access to the model id doesn't work, because the DatX only exposes attributes and relationships directly on the model, and all other data can be accessed in other ways (e.g. model.meta.id in this case).
If you want to use the id directly, you can do it with this change:

class Foo extends jsonapi(Model) {
  @computed get id() {
    return this.meta.id;
  }
}

This example is with either TypeScript or JavaScript with decorator support. If you don't have decorator support, you should be able to do the same thing following the instructions in the MobX docs.

from datx.

I tried something along those lines before posting this, but if I add:

(i assume the this.model.id is supposed to be this.meta.id in your example)

  @computed get id() {
    return this.meta.id
  }

To my model, I end up with the following error:

Model ID can't be updated directly. Use the `updateModelId` helper function instead.

I have tried a few things, but seems like I can't set any getter for id without running into this error.

from datx.

I don't understand something, why is it that when I construct with existing record data that includes an id, like:

const foo = new Foo({ id: 123, .... })
isEqual(foo.id, foo.meta.id) // true, id and meta id are both 123

but that isn't the case when I do something like this:

const foo = new Foo({})
updateModel(foo, { id: 123, ...})
updateModelId(foo, 123)
isEqual(foo.id, foo.meta.id) // false, foo.id is not defined as above, meta id is correct

Shouldn't those behave in the same way where id is set?

from datx.

Is it possible you have an id property in the attributes section? Could you provide the example API response?
I tried to reproduce the issue, but everything was working:

  it('should handle id changes correctly', async () => {
    const store = new TestStore();
    const image1 = store.add({}, Image);
    const image2 = new Image({});

    mockApi({
      method: 'POST',
      name: 'image-1',
      url: 'image',
    });
    expect(image1.id).toBeLessThan(0); // Temporary id, negative autoincrement
    await image1.save(); // Load the image-1.json data
    expect(image1.id).toBe(1);
    expect(image1.id).toBe(image1.meta.id);

    mockApi({
      method: 'POST',
      name: 'image-1',
      url: 'image',
    });
    expect(image2.id).toBeLessThan(0); // Temporary id, negative autoincrement
    await image2.save(); // Load the image-1.json data
    expect(image2.id).toBe(1);
    expect(image2.id).toBe(image2.meta.id);
  });

image-1.json

{
  "data": {
    "id": 1,
    "type": "image",
    "attributes": {
      "url": "http://example.com/1.jpg"
    }
  }
}

from datx.

Shouldn't those behave in the same way where id is set?

That's a good question. I don't think this is a bug, but it might be an behavior that is not expected.

In constructor, the id property will be used as identifier (unless configured otherwise with @prop.identifier), but when using the update method it won't touch the internal identifier, but it might update the defined property (and override the computed property which could cause unexpected things).

The logic behind this is that it should be easy to set the initial id (quite often use case), but it should be hard to change the id (I would say this is an antipattern in most cases that are not API related) and that's why the lib is forcing you to use updateModelId to make mistakes harder to make.

from datx.

We did have id in our attributes. After removing that, I can now add an id setter without error, and everything works the way we were looking for 👍

from datx.