Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: RecordState cleanup, drop requireESM and node12 #8042

Merged
merged 6 commits into from
Jul 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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