Skip to content

Commit

Permalink
fix: RecordState cleanup, drop requireESM and node12 (#8042)
Browse files Browse the repository at this point in the history
* fix: minor tweaks to convert tests to testing RecordState instead of the StateMachine

* node 16, drop node 12

* bump mocha now that node-12 dropped

* remove esm

* chore: eliminate requireESM

* fix asset size check
  • Loading branch information
runspired authored Jul 23, 2022
1 parent 243c896 commit b115541
Show file tree
Hide file tree
Showing 46 changed files with 204 additions and 192 deletions.
4 changes: 2 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ module.exports = {
],
parserOptions: {
sourceType: 'script',
ecmaVersion: 2015,
ecmaVersion: 2018,
},
env: {
browser: false,
Expand Down Expand Up @@ -385,7 +385,7 @@ module.exports = {
},
parserOptions: {
sourceType: 'script',
ecmaVersion: 2015,
ecmaVersion: 2018,
},
},

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/alpha-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
fi
- uses: actions/setup-node@v3
with:
node-version: 14.x
node-version: 16.x
cache: 'yarn'
- name: Install dependencies for master
run: yarn install --frozen-lockfile --non-interactive
Expand All @@ -37,7 +37,7 @@ jobs:
- uses: actions/setup-node@v3
with:
registry-url: 'https://registry.npmjs.org'
node-version: 14.x
node-version: 16.x
cache: 'yarn'
- name: Install dependencies for master
run: yarn install --frozen-lockfile --non-interactive
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/asset-size-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- run: git fetch origin master --depth=1
- uses: actions/setup-node@v3
with:
node-version: 14.x
node-version: 16.x
cache: 'yarn'
- name: Check SHA
run: |
Expand All @@ -36,7 +36,7 @@ jobs:
- name: Install dependencies for master
run: yarn install
- name: Build Production master
run: EMBER_DATA_FULL_COMPAT=true yarn workspace ember-data ember build -e production --output-path dists/control
run: IS_ASSET_SIZE_CHECK=true EMBER_DATA_FULL_COMPAT=true yarn workspace ember-data ember build -e production --output-path dists/control
- name: Build Production master (no rollup)
run: EMBER_DATA_FULL_COMPAT=true EMBER_DATA_ROLLUP_PRIVATE=false yarn workspace ember-data ember build -e production --output-path dists/control-no-rollup
- name: Checkout ${{github.ref}}
Expand All @@ -46,7 +46,7 @@ jobs:
- name: Install dependencies for ${{github.ref}}
run: yarn install
- name: Build Production ${{github.ref}}
run: EMBER_DATA_FULL_COMPAT=true yarn workspace ember-data ember build -e production --output-path dists/experiment
run: IS_ASSET_SIZE_CHECK=true EMBER_DATA_FULL_COMPAT=true yarn workspace ember-data ember build -e production --output-path dists/experiment
- name: Build Production ${{github.ref}} (no rollup)
run: EMBER_DATA_FULL_COMPAT=true EMBER_DATA_ROLLUP_PRIVATE=false yarn workspace ember-data ember build -e production --output-path dists/experiment-no-rollup
- name: Analyze Master Assets
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [12.x, 14.x, 18.x]
node-version: [14.x, 18.x]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 14.x
node-version: 16.x
cache: 'yarn'
- name: Install dependencies for master
run: yarn install
Expand All @@ -36,7 +36,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 14.x
node-version: 16.x
cache: 'yarn'
- name: Install dependencies for ${{ matrix.scenario }}
run: yarn install
Expand All @@ -45,4 +45,3 @@ jobs:
CI: true
ASSERT_ALL_DEPRECATIONS: true
run: yarn test:try-one ${{ matrix.scenario }}

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"eslint-plugin-ember": "^11.0.1",
"eslint-plugin-ember-data": "link:./packages/unpublished-eslint-rules",
"eslint-plugin-import": "^2.26.0",
"eslint-plugin-mocha": "^9.0.0",
"eslint-plugin-mocha": "^10.1.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-qunit": "^7.3.1",
Expand All @@ -119,7 +119,7 @@
"lerna": "^4.0.0",
"lerna-changelog": "^2.2.0",
"loader.js": "^4.7.0",
"mocha": "^9.2.2",
"mocha": "^10.0.0",
"npm-git-info": "^1.0.3",
"pre-commit": "^1.2.2",
"pretender": "^3.4.7",
Expand All @@ -137,7 +137,7 @@
},
"dependencies": {},
"engines": {
"node": "12.* || >= 14.*",
"node": "^14.8.0 || 16.* || >= 18.*",
"yarn": "1.22.19"
},
"volta": {
Expand Down
4 changes: 2 additions & 2 deletions packages/-ember-data/ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ module.exports = function (defaults) {
defaults: true,
arguments: true,
keep_fargs: false,
toplevel: true,
toplevel: process.env.IS_ASSET_SIZE_CHECK ? false : true,
unsafe: true,
unsafe_comps: true,
unsafe_math: true,
unsafe_symbols: true,
unsafe_proto: true,
unsafe_undefined: true,
},
toplevel: true,
toplevel: process.env.IS_ASSET_SIZE_CHECK ? false : true,
sourceMap: false,
ecma: 2016,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/-ember-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"webpack": "^5.73.0"
},
"engines": {
"node": "12.* || >= 14.*",
"node": "^14.8.0 || 16.* || >= 18.*",
"yarn": "1.22.19"
},
"keywords": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {
assert.strictEqual(passedVerb, null, 'There is no ajax call to delete a record that has never been saved.');
assert.strictEqual(passedHash, null, 'There is no ajax call to delete a record that has never been saved.');

assert.true(internalModel.currentState.isEmpty, 'the post is now deleted');
assert.true(internalModel.isEmpty, 'the post is now deleted');
});

test('findAll - returning an array populates the array', async function (assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ module('integration/deletedRecord - Deleting Records', function (hooks) {

record.deleteRecord();

assert.strictEqual(internalModel.currentState.stateName, 'root.empty', 'new person state is empty');
assert.true(internalModel.isEmpty, 'new person state is empty');
assert.strictEqual(get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store');
});

Expand Down Expand Up @@ -248,7 +248,7 @@ module('integration/deletedRecord - Deleting Records', function (hooks) {

await record.destroyRecord();

assert.strictEqual(internalModel.currentState.stateName, 'root.empty', 'new person state is empty');
assert.true(internalModel.isEmpty, 'new person state is empty');
assert.strictEqual(get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store');
});

Expand Down Expand Up @@ -305,11 +305,7 @@ module('integration/deletedRecord - Deleting Records', function (hooks) {

// it is uncertain that `root.empty` vs `root.deleted.saved` afterwards is correct
// but this is the expected result of `unloadRecord`. We may want a `root.deleted.saved.unloaded` state?
assert.strictEqual(
internalModel.currentState.stateName,
'root.empty',
'We reached the correct persisted saved state'
);
assert.true(internalModel.isEmpty, 'We reached the correct persisted saved state');
assert.strictEqual(get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store');

// let cache = store._identityMap._map.person._models;
Expand Down Expand Up @@ -345,11 +341,7 @@ module('integration/deletedRecord - Deleting Records', function (hooks) {

// it is uncertain that `root.empty` vs `root.deleted.saved` afterwards is correct
// but this is the expected result of `unloadRecord`. We may want a `root.deleted.saved.unloaded` state?
assert.strictEqual(
internalModel.currentState.stateName,
'root.empty',
'We reached the correct persisted saved state'
);
assert.true(internalModel.isEmpty, 'We reached the correct persisted saved state');
assert.strictEqual(get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store');

// let cache = store._identityMap._map.person._models;
Expand Down
18 changes: 9 additions & 9 deletions packages/-ember-data/tests/integration/records/error-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ module('integration/records/error', function (hooks) {
});

assert.strictEqual(
person._internalModel.currentState.stateName,
person.currentState.stateName,
'root.loaded.updated.uncommitted',
'Model state is root.loaded.updated.uncommitted'
);

person.errors.add('firstName', 'is invalid');

assert.strictEqual(
person._internalModel.currentState.stateName,
person.currentState.stateName,
'root.loaded.updated.invalid',
'Model state is updated to root.loaded.updated.invalid after an error is manually added'
);
Expand Down Expand Up @@ -92,15 +92,15 @@ module('integration/records/error', function (hooks) {
});

assert.strictEqual(
person._internalModel.currentState.stateName,
person.currentState.stateName,
'root.loaded.created.uncommitted',
'Model state is root.loaded.updated.uncommitted'
);

person.errors.add('firstName', 'is invalid');

assert.strictEqual(
person._internalModel.currentState.stateName,
person.currentState.stateName,
'root.loaded.created.invalid',
'Model state is updated to root.loaded.updated.invalid after an error is manually added'
);
Expand Down Expand Up @@ -136,11 +136,11 @@ module('integration/records/error', function (hooks) {

person.set('firstName', null);

assert.strictEqual(person._internalModel.currentState.stateName, 'root.loaded.created.uncommitted');
assert.strictEqual(person.currentState.stateName, 'root.loaded.created.uncommitted');

person.errors.add('firstName', 'is invalid');

assert.strictEqual(person._internalModel.currentState.stateName, 'root.loaded.created.invalid');
assert.strictEqual(person.currentState.stateName, 'root.loaded.created.invalid');

person.errors.remove('firstName');

Expand Down Expand Up @@ -170,16 +170,16 @@ module('integration/records/error', function (hooks) {

person.set('firstName', null);

assert.strictEqual(person._internalModel.currentState.stateName, 'root.loaded.created.uncommitted');
assert.strictEqual(person.currentState.stateName, 'root.loaded.created.uncommitted');

person.errors.add('firstName', 'is invalid');

assert.strictEqual(person._internalModel.currentState.stateName, 'root.loaded.created.invalid');
assert.strictEqual(person.currentState.stateName, 'root.loaded.created.invalid');

person.errors.remove('firstName');
person.errors.add('firstName', 'is invalid');

assert.strictEqual(person._internalModel.currentState.stateName, 'root.loaded.created.invalid');
assert.strictEqual(person.currentState.stateName, 'root.loaded.created.invalid');

assert.deepEqual(person.errors.toArray(), [{ attribute: 'firstName', message: 'is invalid' }]);
});
Expand Down
55 changes: 25 additions & 30 deletions packages/-ember-data/tests/integration/records/load-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import Model, { attr, belongsTo } from '@ember-data/model';
import JSONAPISerializer from '@ember-data/serializer/json-api';
import Store from '@ember-data/store';
import testInDebug from '@ember-data/unpublished-test-infra/test-support/test-in-debug';
import todo from '@ember-data/unpublished-test-infra/test-support/todo';

class Person extends Model {
@attr()
Expand Down Expand Up @@ -69,7 +68,7 @@ module('integration/load - Loading Records', function (hooks) {
}
});

todo('Empty records remain in the empty state while data is being fetched', async function (assert) {
test('Empty records remain in the empty state while data is being fetched', async function (assert) {
let payloads = [
{
data: {
Expand Down Expand Up @@ -147,25 +146,21 @@ module('integration/load - Loading Records', function (hooks) {
let internalModel = store._internalModelForId('person', '1');

// test that our initial state is correct
assert.strictEqual(internalModel.currentState.isEmpty, true, 'We begin in the empty state');
assert.strictEqual(internalModel.currentState.isLoading, false, 'We have not triggered a load');
assert.true(internalModel.isEmpty, 'We begin in the empty state');
assert.false(internalModel.isLoading, 'We have not triggered a load');

let recordPromise = store.findRecord('person', '1');

// test that during the initial load our state is correct
assert.todo.equal(internalModel.currentState.isEmpty, true, 'awaiting first fetch: We remain in the empty state');
assert.strictEqual(
internalModel.currentState.isLoading,
true,
'awaiting first fetch: We have now triggered a load'
);
assert.true(internalModel.isEmpty, 'awaiting first fetch: We remain in the empty state');
assert.true(internalModel.isLoading, 'awaiting first fetch: We have now triggered a load');

let record = await recordPromise;

// test that after the initial load our state is correct
assert.strictEqual(internalModel.currentState.isEmpty, false, 'after first fetch: We are no longer empty');
assert.strictEqual(internalModel.currentState.isLoading, false, 'after first fetch: We have loaded');
assert.strictEqual(record.isReloading, false, 'after first fetch: We are not reloading');
assert.false(internalModel.isEmpty, 'after first fetch: We are no longer empty');
assert.false(internalModel.isLoading, 'after first fetch: We have loaded');
assert.false(record.isReloading, 'after first fetch: We are not reloading');

let bestFriend = await record.get('bestFriend');
let trueBestFriend = await bestFriend.get('bestFriend');
Expand All @@ -175,43 +170,43 @@ module('integration/load - Loading Records', function (hooks) {
// discard the internalModel
let shen = store.peekRecord('person', '2');

assert.ok(bestFriend === shen, 'Precond: bestFriend is correct');
assert.ok(trueBestFriend === record, 'Precond: bestFriend of bestFriend is correct');
assert.strictEqual(bestFriend, shen, 'Precond: bestFriend is correct');
assert.strictEqual(trueBestFriend, record, 'Precond: bestFriend of bestFriend is correct');

recordPromise = record.reload();

// test that during a reload our state is correct
assert.strictEqual(internalModel.currentState.isEmpty, false, 'awaiting reload: We remain non-empty');
assert.strictEqual(internalModel.currentState.isLoading, false, 'awaiting reload: We are not loading again');
assert.strictEqual(record.isReloading, true, 'awaiting reload: We are reloading');
assert.false(internalModel.isEmpty, 'awaiting reload: We remain non-empty');
assert.false(internalModel.isLoading, 'awaiting reload: We are not loading again');
assert.true(record.isReloading, 'awaiting reload: We are reloading');

await recordPromise;

// test that after a reload our state is correct
assert.strictEqual(internalModel.currentState.isEmpty, false, 'after reload: We remain non-empty');
assert.strictEqual(internalModel.currentState.isLoading, false, 'after reload: We have loaded');
assert.strictEqual(record.isReloading, false, 'after reload:: We are not reloading');
assert.false(internalModel.isEmpty, 'after reload: We remain non-empty');
assert.false(internalModel.isLoading, 'after reload: We have loaded');
assert.false(record.isReloading, 'after reload:: We are not reloading');

run(() => record.unloadRecord());

// test that after an unload our state is correct
assert.strictEqual(internalModel.currentState.isEmpty, true, 'after unload: We are empty again');
assert.strictEqual(internalModel.currentState.isLoading, false, 'after unload: We are not loading');
assert.strictEqual(record.isReloading, false, 'after unload:: We are not reloading');
assert.true(internalModel.isEmpty, 'after unload: We are empty again');
assert.false(internalModel.isLoading, 'after unload: We are not loading');
assert.false(record.isReloading, 'after unload:: We are not reloading');

recordPromise = store.findRecord('person', '1');

// test that during a reload-due-to-unload our state is correct
// This requires a retainer (the async bestFriend relationship)
assert.todo.equal(internalModel.currentState.isEmpty, true, 'awaiting second find: We remain empty');
assert.strictEqual(internalModel.currentState.isLoading, true, 'awaiting second find: We are loading again');
assert.strictEqual(record.isReloading, false, 'awaiting second find: We are not reloading');
assert.true(internalModel.isEmpty, 'awaiting second find: We remain empty');
assert.true(internalModel.isLoading, 'awaiting second find: We are loading again');
assert.false(record.isReloading, 'awaiting second find: We are not reloading');

await recordPromise;

// test that after the reload-due-to-unload our state is correct
assert.strictEqual(internalModel.currentState.isEmpty, false, 'after second find: We are no longer empty');
assert.strictEqual(internalModel.currentState.isLoading, false, 'after second find: We have loaded');
assert.strictEqual(record.isReloading, false, 'after second find: We are not reloading');
assert.false(internalModel.isEmpty, 'after second find: We are no longer empty');
assert.false(internalModel.isLoading, 'after second find: We have loaded');
assert.false(record.isReloading, 'after second find: We are not reloading');
});
});
Loading

0 comments on commit b115541

Please sign in to comment.