adopted-ember-addons/ember-data-factory-guy

String vs number ids and ember-data 5.3+ issues

Techn1x opened this issue · 3 comments

I'd like to use string ids across the board, as that's what my JSONAPI does and it's also what ember-data generally expects.

EDFG seems to always use number ids by default when using a factory.

nextId() {
return this.modelId++;
}

That might have worked in the past, but I think ember-data is a bit more strict now. On ember-data 4.12.1, it seems that for a patch/update payload, if ED receives an id from the api (string) that does match match what is cached in its store (number), it fails with this error;

Error: Assertion Failed: Expected the ID received for the primary 'teacher' resource being saved to match the current id '1' but received '1'.
    at assert (index.js:153:1)
    at JSONAPICache.didCommit (index.js:627:103)
    at eval (index.js:432:22)
    at Store._run (index-757cf686.js:5403:5)
    at Store._join (index-757cf686.js:5419:12)
    at eval (index.js:429:11)

This can be replicated with code like the following;

const teach = make('teacher') // pushes a teacher to the store with id 1 (number)
// console.log(teach.id) // record in store has id '1' (string)
mockUpdate(this.teacher)
  .match({ locale: 'fr' })
  .returns({ attrs: { language: 'fr' } })
teach.name = "newname"
await teach.save()

The workaround is to set an id in the model when creating it make('teacher', { id: 'my-string-id' }) but I've got a lot of tests to update if I go that path..

I think the fix is to either force string ids from that nextId function, or give an option to users to set a flag in a factory to use string ids.

Looking deeper, it might be specifically something to do with the mockUpdate(...).match(...).returns(...) because it seems to work without the match/returns chains.

Either way, I still think giving the option to use string ids in factories would be beneficial

Also, I think string ids is be a requirement of ember-data 5 & 6.

It's a deprecation item here until ED6
https://deprecations.emberjs.com/ember-data/v5.x#toc_ember-data-deprecate-non-strict-id

We are deprecating this legacy support for numeric IDs.

But even when using ED with compatWith: '5.3' it will fail with

Error: Assertion Failed: Resource IDs must be a non-empty string or null. Received '1'.

Plus 1