Fix updating fragment arrays multiple times in the same runloop (#487)

* run CI in production environment

* upgrade ember-try

Fixes the following error:
Error: Cannot copy '../../../browserslist/cli.js' to a subdirectory of itself, '../../../browserslist/cli.js'.
    at /home/runner/work/ember-data-model-fragments/ember-data-model-fragments/node_modules/ember-try/node_modules/fs-extra/lib/copy/copy.js:210:21
    at FSReqCallback.oncomplete (fs.js:180:23)

ember-cli/ember-try#967

* upgrade to node.js 18

* fix store.find deprecation

* replace assert.throws with expectAssertion

* fix updating fragment array multiple times

* revert to node 14 and use `yarn --ignore-engines`

.github/workflows/ci.yml

@@ -77,7 +77,7 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-
 
       - name: Install Dependencies
-        run: yarn install --frozen-lockfile
+        run: yarn install --frozen-lockfile --ignore-engines
         if: |
           steps.cache-dependencies.outputs.cache-hit != 'true'
 
@@ -121,7 +121,7 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-
 
       - name: Install Dependencies
-        run: yarn install --frozen-lockfile
+        run: yarn install --frozen-lockfile --ignore-engines
         if: |
           steps.cache-dependencies.outputs.cache-hit != 'true'
 
@@ -163,7 +163,7 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-
 
       - name: Install Dependencies
-        run: yarn install --no-lockfile --non-interactive
+        run: yarn install --no-lockfile --non-interactive --ignore-engines
 
       - name: Test
         run: yarn test:ember --launch ${{ matrix.browser }}
@@ -217,11 +217,16 @@ jobs:
             ${{ runner.os }}-${{ matrix.node-version }}-
 
       - name: Install Dependencies
-        run: yarn install --frozen-lockfile
+        run: yarn install --frozen-lockfile --ignore-engines
         if: |
           steps.cache-dependencies.outputs.cache-hit != 'true'
 
       - name: Test
         env:
           EMBER_TRY_SCENARIO: ${{ matrix.ember-try-scenario }}
         run: node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO
+
+      - name: Test (Prod)
+        env:
+          EMBER_TRY_SCENARIO: ${{ matrix.ember-try-scenario }}
+        run: node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO --- ember test --environment=production

addon/array/stateful.js

@@ -115,6 +115,9 @@ const StatefulArray = EmberObject.extend(MutableArray, Copyable, {
       // array is unchanged
       return;
     }
+    if (this._isDirty) {
+      this.retrieveLatest();
+    }
     const data = this.currentState.slice();
     data.splice(
       start,

package.json

@@ -67,7 +67,7 @@
     "ember-source": "~4.6.0",
     "ember-source-channel-url": "^3.0.0",
     "ember-template-lint": "^5.3.1",
-    "ember-try": "^2.0.0",
+    "ember-try": "^3.0.0",
     "eslint": "^7.32.0",
     "eslint-config-prettier": "^8.6.0",
     "eslint-plugin-ember": "^11.4.3",

tests/helpers/assertion.js

@@ -0,0 +1,42 @@
+import { getDebugFunction, setDebugFunction } from '@ember/debug';
+import { DEBUG } from '@glimmer/env';
+
+/**
+ * Asserts that `Ember.assert` is called with a falsy condition
+ * @param func function which calls `Ember.assert`
+ * @param expectedMessage the expected assertion text to compare with the first argument to `Ember.assert`
+ */
+function expectAssertion(func, expectedMessage) {
+  if (!DEBUG) {
+    this.ok(true, 'Assertions disabled in production builds');
+    return;
+  }
+  const originalAssertFunc = getDebugFunction('assert');
+  try {
+    let called = false;
+    let failed = false;
+    let actualMessage;
+    setDebugFunction('assert', function assert(desc, test) {
+      called = true;
+      if (!test) {
+        failed = true;
+        actualMessage = desc;
+      }
+    });
+    func();
+    this.true(called, `Expected Ember.assert to be called`);
+    this.true(failed, `Expected Ember.assert to fail its test`);
+    this.strictEqual(
+      actualMessage,
+      expectedMessage,
+      'Expected Ember.assert message to match'
+    );
+  } finally {
+    // restore original assert function
+    setDebugFunction('assert', originalAssertFunc);
+  }
+}
+
+export function setup(assert) {
+  assert.expectAssertion = expectAssertion;
+}

tests/test-helper.js

@@ -2,11 +2,13 @@ import Application from 'dummy/app';
 import config from 'dummy/config/environment';
 import * as QUnit from 'qunit';
 import { setApplication } from '@ember/test-helpers';
-import { setup } from 'qunit-dom';
+import { setup as setupQunitDom } from 'qunit-dom';
+import { setup as setupCustomAssertions } from './helpers/assertion';
 import { start } from 'ember-qunit';
 
 setApplication(Application.create(config.APP));
 
-setup(QUnit.assert);
+setupQunitDom(QUnit.assert);
+setupCustomAssertions(QUnit.assert);
 
 start();

tests/unit/fragment_array_property_test.js

@@ -195,11 +195,11 @@ module('unit - `MF.fragmentArray` property', function (hooks) {
     const person = await store.findRecord('person', 1);
     const addresses = person.addresses;
 
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       const otherPerson = store.createRecord('person');
 
       addresses.addFragment(otherPerson);
-    }, 'error is thrown when adding a DS.Model instance');
+    }, "You can only add 'address' fragments or object literals to this property");
   });
 
   test('adding fragments from other records throws an error', async function (assert) {
@@ -212,9 +212,9 @@ module('unit - `MF.fragmentArray` property', function (hooks) {
     ]);
     const address = people[0].addresses.firstObject;
 
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       people[1].addresses.addFragment(address);
-    }, 'error is thrown when adding a fragment from another record');
+    }, 'Fragments can only belong to one owner, try copying instead');
   });
 
   test('setting to an array of fragments is allowed', async function (assert) {
@@ -422,9 +422,9 @@ module('unit - `MF.fragmentArray` property', function (hooks) {
     pushPerson(1);
 
     const person = await store.findRecord('person', 1);
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       person.set('addresses', ['address']);
-    }, 'error is thrown when setting to an array of non-fragments');
+    }, "You can only add 'address' fragments or object literals to this property");
   });
 
   test('fragments can have default values', function (assert) {

tests/unit/fragment_owner_property_test.js

@@ -52,13 +52,9 @@ module('unit - `MF.fragmentOwner` property', function (hooks) {
 
     const invalid = store.createRecord('invalidModel');
 
-    assert.throws(
-      () => {
-        invalid.owner;
-      },
-      /Fragment owner properties can only be used on fragments/,
-      'getting fragment owner on non-fragment throws an error'
-    );
+    assert.expectAssertion(() => {
+      invalid.owner;
+    }, 'Fragment owner properties can only be used on fragments.');
   });
 
   test("attempting to change a fragment's owner record throws an error", async function (assert) {

tests/unit/fragment_property_test.js

@@ -111,9 +111,9 @@ module('unit - `MF.fragment` property', function (hooks) {
     });
 
     const person = await store.findRecord('person', 1);
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       person.set('name', store.createRecord('person'));
-    }, 'error is thrown when setting non-fragment');
+    }, 'You must pass a fragment or null to set a fragment');
   });
 
   test('setting fragments from other records throws an error', async function (assert) {
@@ -142,9 +142,9 @@ module('unit - `MF.fragment` property', function (hooks) {
       store.findRecord('person', 1),
       store.findRecord('person', 2),
     ]);
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       people[1].set('name', people[0].name);
-    }, 'error is thrown when setting to a fragment of another record');
+    }, 'Fragments can only belong to one owner, try copying instead');
   });
 
   test('null values are allowed', async function (assert) {

tests/unit/polymorphic_test.js

@@ -94,7 +94,7 @@ module('unit - Polymorphism', function (hooks) {
       },
     });
 
-    const record = await store.find('zoo', 1);
+    const record = await store.findRecord('zoo', 1);
     const animals = record.animals;
 
     const newLion = animals.createFragment({
@@ -138,7 +138,7 @@ module('unit - Polymorphism', function (hooks) {
       },
     });
 
-    const component = await store.find('component', 1);
+    const component = await store.findRecord('component', 1);
     const textOptions = component.optionsHistory.createFragment({
       fontFamily: 'Verdana',
       fontSize: 12,

tests/unit/store_test.js

@@ -29,9 +29,9 @@ module('unit - `DS.Store`', function (hooks) {
   });
 
   test('attempting to create a fragment type that does not inherit from `MF.Fragment` throws an error', function (assert) {
-    assert.throws(() => {
+    assert.expectAssertion(() => {
       store.createFragment('person');
-    }, 'an error is thrown when given a bad type');
+    }, "The 'person' model must be a subclass of MF.Fragment");
   });
 
   test('the store has an `isFragment` method', function (assert) {

yarn.lock

@@ -4728,20 +4728,21 @@ ember-try-config@^4.0.0:
     remote-git-tags "^3.0.0"
     semver "^7.3.2"
 
-ember-try@^2.0.0:
-  version "2.0.0"
-  resolved "https://registry.yarnpkg.com/ember-try/-/ember-try-2.0.0.tgz#2671c221f5a0335fa2c189d00db7146e2e72a1dd"
-  integrity sha512-2N7Vic45sbAegA5fhdfDjVbEVS4r+ze+tTQs2/wkY+8fC5yAGHfCM5ocyoTfBN5m7EhznC3AyMsOy4hLPzHFSQ==
+ember-try@^3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/ember-try/-/ember-try-3.0.0.tgz#3b4e8511b64925aff14224b408fb5b5adab69391"
+  integrity sha512-ZYVKYWMnrHSD3vywo7rV76kPCOC9ATIEnGGG/PEKfCcFE0lB26jltRDnOrhORfLKq0JFp62fFxC/4940U+MwRQ==
   dependencies:
     chalk "^4.1.2"
     cli-table3 "^0.6.0"
     core-object "^3.1.5"
     debug "^4.3.2"
     ember-try-config "^4.0.0"
     execa "^4.1.0"
-    fs-extra "^9.0.1"
+    fs-extra "^6.0.1"
     resolve "^1.20.0"
     rimraf "^3.0.2"
+    semver "^7.5.4"
     walk-sync "^2.2.0"
 
 emoji-regex@^7.0.1:
@@ -5777,6 +5778,15 @@ fs-extra@^5.0.0:
     jsonfile "^4.0.0"
     universalify "^0.1.0"
 
+fs-extra@^6.0.1:
+  version "6.0.1"
+  resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-6.0.1.tgz#8abc128f7946e310135ddc93b98bddb410e7a34b"
+  integrity sha512-GnyIkKhhzXZUWFCaJzvyDLEEgDkPfb4/TPvJCJVuS8MWZgoSsErf++QpiAlDnKFcqhRlm+tIOcencCjyJE6ZCA==
+  dependencies:
+    graceful-fs "^4.1.2"
+    jsonfile "^4.0.0"
+    universalify "^0.1.0"
+
 fs-extra@^7.0.0, fs-extra@^7.0.1:
   version "7.0.1"
   resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-7.0.1.tgz#4f189c44aa123b895f722804f55ea23eadc348e9"
@@ -9849,6 +9859,13 @@ semver@^7.0.0, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semve
   dependencies:
     lru-cache "^6.0.0"
 
+semver@^7.5.4:
+  version "7.6.0"
+  resolved "https://registry.yarnpkg.com/semver/-/semver-7.6.0.tgz#1a46a4db4bffcccd97b743b5005c8325f23d4e2d"
+  integrity sha512-EnwXhrlwXMk9gKu5/flx5sv/an57AkRplG3hTK68W7FRDN+k+OWBj65M7719OkA82XLBxrcX0KSHj+X5COhOVg==
+  dependencies:
+    lru-cache "^6.0.0"
+
 [email protected]:
   version "0.18.0"
   resolved "https://registry.yarnpkg.com/send/-/send-0.18.0.tgz#670167cc654b05f5aa4a767f9113bb371bc706be"