fix: dont skip LiveArray remove if pending queue is not empty (#8306)

* fix: dont skip LiveArray remove if pending queue is not empty * fix issue with hardlinks * fix typo * fix release test
emberjs · Nov 14, 2022 · 2a9a25e · 2a9a25e
1 parent 4e4bb25
commit 2a9a25e
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 6 deletions.
diff --git a/.github/workflows/perf-check.yml b/.github/workflows/perf-check.yml
@@ -40,9 +40,9 @@ jobs:
           cache: 'pnpm'
       - uses: tracerbench/tracerbench-compare-action@master
         with:
-          experiment-build-command: npm run --if-present build-v2-addons && pnpm --filter performance-test-app exec ember build -e production --output-path dist-experiment
+          experiment-build-command: npm run --if-present build-v2-addons && pnpm install && pnpm --filter performance-test-app exec ember build -e production --output-path dist-experiment
           experiment-serve-command: pnpm --filter performance-test-app exec ember s --path dist-experiment --port 4201
-          control-build-command: npm run --if-present build-v2-addons && pnpm --filter performance-test-app exec ember build -e production --output-path dist-control
+          control-build-command: npm run --if-present build-v2-addons && pnpm install && pnpm --filter performance-test-app exec ember build -e production --output-path dist-control
           control-serve-command: pnpm --filter performance-test-app exec ember s --path dist-control
           sample-timeout: 60
           use-pnpm: true

diff --git a/.github/workflows/perf-over-release.yml b/.github/workflows/perf-over-release.yml
@@ -40,10 +40,10 @@ jobs:
           cache: 'pnpm'
       - uses: tracerbench/tracerbench-compare-action@master
         with:
-          experiment-build-command: npm run --if-present build-v2-addons && pnpm --filter performance-test-app exec ember build -e production --output-path dist-experiment
+          experiment-build-command: npm run --if-present build-v2-addons && pnpm install && pnpm --filter performance-test-app exec ember build -e production --output-path dist-experiment
           experiment-serve-command: pnpm --filter performance-test-app exec ember s --path dist-experiment --port 4201
-          control-build-command: yarn install && npm run --if-present build-v2-addons && yarn workspace performance-test-app ember build -e production --output-path dist-control
-          control-serve-command: yarn workspace performance-test-app ember s --path dist-control
+          control-build-command: npm run --if-present build-v2-addons && pnpm install && pnpm --filter performance-test-app exec ember build -e production --output-path dist-control
+          control-serve-command: pnpm --filter performance-test-app exec ember s --path dist-control
           sample-timeout: 60
           use-pnpm: true
           scenarios: |

diff --git a/packages/store/addon/-private/managers/record-array-manager.ts b/packages/store/addon/-private/managers/record-array-manager.ts
@@ -211,7 +211,10 @@ class RecordArrayManager {
     // during unloadAll we can ignore removes since we've already
     // cleared the array.
     if (liveArray && liveArray[SOURCE].length === 0 && isRemove) {
-      return pending;
+      const pendingLive = allPending.get(liveArray);
+      if (!pendingLive || pendingLive.size === 0) {
+        return pending;
+      }
     }
 
     if (!liveArray) {

diff --git a/tests/main/tests/integration/peek-all-test.js b/tests/main/tests/integration/peek-all-test.js
@@ -89,4 +89,89 @@ module('integration/peek-all - DS.Store#peekAll()', function (hooks) {
     store.createRecord('person', { name: 'Tomster' });
     assert.strictEqual(get(store.peekAll('person'), 'length'), 1, 'should contain one person');
   });
+
+  test('Newly created records properly cleanup peekAll state when calling destroyRecord (first peek post create)', async function (assert) {
+    this.owner.register(
+      'model:company',
+      class extends Model {
+        @attr name;
+      }
+    );
+    const store = this.owner.lookup('service:store');
+
+    const company1 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    await company1.destroyRecord();
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company2 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+
+    assert.strictEqual(store.peekAll('company').length, 1, 'one company loaded');
+
+    await company2.destroyRecord();
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company3 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    await company3.destroyRecord();
+
+    const peeked = store.peekAll('company');
+    assert.strictEqual(peeked.length, 0, 'no company loaded');
+  });
+
+  test('Newly created records properly cleanup peekAll state when calling destroyRecord (first peek prior to create)', async function (assert) {
+    this.owner.register(
+      'model:company',
+      class extends Model {
+        @attr name;
+      }
+    );
+    const store = this.owner.lookup('service:store');
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company1 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    await company1.destroyRecord();
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company2 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+
+    assert.strictEqual(store.peekAll('company').length, 1, 'one company loaded');
+
+    await company2.destroyRecord();
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company3 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    await company3.destroyRecord();
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+  });
+
+  test('Newly created records properly cleanup peekAll state when calling destroyRecord (always peek prior to destroy)', async function (assert) {
+    this.owner.register(
+      'model:company',
+      class extends Model {
+        @attr name;
+      }
+    );
+    const store = this.owner.lookup('service:store');
+
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company1 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    assert.strictEqual(store.peekAll('company').length, 1, 'one company loaded');
+    await company1.destroyRecord();
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company2 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    assert.strictEqual(store.peekAll('company').length, 1, 'one company loaded');
+    await company2.destroyRecord();
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+
+    const company3 = store.createRecord('company', { id: 'c1', name: 'IPC' });
+    assert.strictEqual(store.peekAll('company').length, 1, 'one company loaded');
+    await company3.destroyRecord();
+    assert.strictEqual(store.peekAll('company').length, 0, 'no company loaded');
+  });
 });