Skip to content

Commit

Permalink
fix: JSONAPISerializer should not reify empty records (emberjs#8934)
Browse files Browse the repository at this point in the history
* fix: JSONAPISerializer should not reify empty records

* fix lint
  • Loading branch information
runspired authored and BoussonKarel committed Oct 6, 2023
1 parent 4eb2d6c commit ebe59ae
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/legacy-compat/src/legacy-network-handler/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,13 @@ export default class Snapshot implements Snapshot {
@type {Model}
@public
*/
get record(): RecordInstance {
return this._store._instanceCache.getRecord(this.identifier);
get record(): RecordInstance | null {
const record = this._store.peekRecord(this.identifier);
assert(
`Record ${this.identifier.type} ${this.identifier.id} (${this.identifier.lid}) is not yet loaded and thus cannot be accessed from the Snapshot during serialization`,
record !== null
);
return record;
}

get _attributes(): Record<string, unknown> {
Expand Down
58 changes: 58 additions & 0 deletions tests/main/tests/integration/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,64 @@ module('integration/relationships/has_many - Has-Many Relationships', function (
}, /Assertion Failed: Encountered a relationship identifier without a type for the hasMany relationship 'comments' on <post:2>, expected an identifier with type 'comment' but found/);
});

test('A record with an async hasMany relationship can safely be saved and later access the relationship', async function (assert) {
const store = this.owner.lookup('service:store');
this.owner.register(
'adapter:application',
class extends JSONAPIAdapter {
updateRecord(store, schema, snapshot) {
assert.step('updateRecord');
const data = store.serializerFor(schema.modelName).serialize(snapshot, { includeId: true });

return Promise.resolve(data);
}
findRecord(store, schema, id, snapshot) {
assert.step('findRecord');
return Promise.resolve({
data: { id, type: 'chapter', attributes: { title: `Chapter ${id}` } },
});
}
}
);
this.owner.register(
'serializer:application',
class extends JSONAPISerializer {
serialize(snapshot, options) {
assert.step('serialize');
return super.serialize(snapshot, options);
}
}
);
const book = store.push({
data: {
type: 'book',
id: '1',
relationships: {
chapters: {
data: [
{ type: 'chapter', id: '1' },
{ type: 'chapter', id: '2' },
],
},
},
},
});
book.title = 'The Book of Foo';

await book.save();

assert.verifySteps(['updateRecord', 'serialize']);

const chapters = await book.chapters;

assert.verifySteps(['findRecord', 'findRecord']);
assert.strictEqual(chapters.length, 2);
assert.deepEqual(
chapters.map((v) => v.id),
['1', '2']
);
});

test("When a hasMany relationship is accessed, the adapter's findMany method should not be called if all the records in the relationship are already loaded", async function (assert) {
assert.expect(1);

Expand Down

0 comments on commit ebe59ae

Please sign in to comment.