[Ember Data 4.7] Using peekAll after destroyRecord returns invalid length and records without stable Identifier

[eodb]

Reproduction

I couldn't use Twiddle with ED 4.7, so I included following basic qunit test that will help to quickly reproduce the issue.

import { run } from '@ember/runloop';
import Model, { attr } from '@ember-data/model';
import { setupTest } from 'ember-qunit';
import { module, test } from 'qunit';

let store;
let Company;

module('Unit | Model | Destroy model', function (hooks) {
  setupTest(hooks);

  hooks.beforeEach(function () {
    Company = Model.extend({
      name: attr('string'),
    });

    this.owner.register('model:company', Company);
    store = this.owner.lookup('service:store');

    Company = store.modelFor('company');
  });

  test('destroyRecord twice with a peekAll in between', function (assert) {
    let company;

    run(() => {
      store.createRecord('company', { id: 'c1', name: 'IPC' });
      company = store.peekRecord('company', 'c1');
    });
    run(() => {
      company.destroyRecord();
    });

    // When the next peekAll method is skipped, the test succeeds!
    assert.equal(store.peekAll('company').get('length'), 0, 'no company loaded');

    run(() => {
      store.createRecord('company', { id: 'c1', name: 'IPC' });
      company = store.peekRecord('company', 'c1');
    });
    run(() => {
      company.destroyRecord();
    });

    assert.equal(store.peekAll('company').get('length'), 0, 'no company loaded');
  });
});

Description

Scenario is straight-forward: perform a store.createRecord and destroyRecord for the same record and do this twice, with a store.peekAll to retrieve the existing models after each destroyRecord call.

One would expect the same result after each store.peekAllcall i.e. to return with 0 records in the store.
However, the 2nd peekAll call, returns with length = 1. An attempt to retrieve this record however, throws following assertion error:
"Error: Assertion Failed: Expected to receive a stable Identifier to subscribe to".

The curious thing is that, if you ommit the first store.peekAllcall after the first destroyRecord call (see comment line in the test and comment out the first call to store.peekAll), both asserts in the test succeed. So the fact of doing a store.peekAll after the first destroyRecord, results in the 2nd store.peekAll to return an invalid length of 1.

I couldn't help thinking about Schrödinger's cat, but this might be my misconception :).

Versions

This scenario did work in ember-data 4.4.0 but not anymore starting from 4.7.x.
Tests were done with:

Ember      : 4.7.0
Ember Data : 4.8.2

[runspired]

Thanks for the fairly clear reproduction and test case! I'll try to slide a patch in for 4.8.4, unsure if Ill backport to 4.7.x since there isn't a ton different between these and 4.8 will become LTS

[eodb]

The 4.8.4 is fine. We will ship with that version anyway. I'll rerun our unit tests and scenarios once the 4.8.4. is available.
Thanks for the quick response!

[paddor]

Is it possible that this is happening again in ED 5.3.0?

[runspired]

@paddor not really. There was a different bug with hasMany due to deduping that we've since fixed, the patch for which was in 5.3.1 (though that release encountered some issues around TS publishing we're working to fix now)