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

[Test]: fix for DataWrapper refactor in emberjs #7435

Merged
merged 25 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7707e3a
[Test]: fix for RecordData refactor in emberjs
snewcomer Feb 20, 2021
8ce9e6d
settled after set
snewcomer Feb 21, 2021
3be4841
fix to handle new API for removed records
snewcomer Feb 21, 2021
f67e48f
fix lint
snewcomer Feb 21, 2021
852dc57
note API change
snewcomer Feb 21, 2021
de68a6d
use compat helpers
snewcomer Feb 22, 2021
03a2594
improve test for 3.26 to refer to actual removed record
snewcomer Feb 24, 2021
c5e20ee
use attributes property to compare to avoid accessing properties afte…
snewcomer Feb 26, 2021
73e2f89
Allow direct comparison
snewcomer Mar 19, 2021
6cfb36f
Another test fix with externalDependenciesForPrivateModule in canary
snewcomer Mar 19, 2021
809c2b6
Another another test fix with externalDependenciesForPrivateModule in…
snewcomer Mar 19, 2021
1efd480
fix test by registering the class instead of the factory manager. th…
snewcomer Mar 23, 2021
6a97cd9
rm ember-cli-shims and ember-internal-test-helpers
snewcomer Mar 23, 2021
f7e9e77
terser instead of uglify
snewcomer Mar 23, 2021
1b75f2e
add explicit @ember/test-helpers so that legacy-0.6.x is not installed
snewcomer Mar 23, 2021
1437f6b
update lockfile
snewcomer Mar 23, 2021
051b1a7
update ember-cli-babel
snewcomer Mar 23, 2021
b38e71b
@ember/test-helpers 1.7.3
snewcomer Mar 24, 2021
186df14
yarn resolution 1.7.3
snewcomer Mar 24, 2021
633fbe2
fix lint
snewcomer Mar 24, 2021
bd93053
Fix deprecations for accessing run.* (e.g. `run.next(...)`)
rwjblue Mar 24, 2021
2981e7f
fix lint
snewcomer Mar 25, 2021
4b74236
lets try this
snewcomer Mar 26, 2021
24c1a96
Revert "lets try this"
snewcomer Mar 26, 2021
ee8efc4
update to ember-cli-babel 7.26.3
snewcomer Mar 26, 2021
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: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,13 @@
"debug": "^4.1.1",
"ember-cli": "~3.24.0",
"ember-cli-app-version": "^3.2.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-blueprint-test-helpers": "^0.19.1",
"ember-cli-dependency-checker": "^3.2.0",
"ember-cli-htmlbars": "^5.1.2",
"ember-cli-inject-live-reload": "^2.0.2",
"ember-cli-internal-test-helpers": "^0.9.1",
"ember-cli-path-utils": "^1.0.0",
"ember-cli-pretender": "^3.2.0",
"ember-cli-shims": "^1.2.0",
"ember-cli-sri": "^2.1.1",
"ember-cli-string-utils": "^1.1.0",
"ember-cli-template-lint": "^2.0.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/-ember-data/ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = function(defaults) {
'ember-cli-babel': {
throwUnlessParallelizable: true,
},
'ember-cli-uglify': {
'ember-cli-terser': {
exclude: ['assets/dummy.js', 'assets/tests.js', 'assets/test-support.js', 'dist/docs/*', 'docs/*'],
},
});
Expand Down
2 changes: 2 additions & 0 deletions packages/-ember-data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ module.exports = Object.assign({}, addonBaseConfig, {
shouldRollupPrivate: true,
externalDependenciesForPrivateModule() {
return [
'ember',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we have to add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2021-03-26 at 3 28 54 PM

'@ember/application/namespace',
'@ember-data/record-data/-private',
'ember-data/version',
'@ember-data/store/-private',
Expand Down
4 changes: 1 addition & 3 deletions packages/-ember-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@ember/string": "^1.0.0",
"@glimmer/env": "^0.1.7",
"broccoli-merge-trees": "^4.2.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-typescript": "^4.0.0",
"ember-inflector": "^4.0.1"
},
Expand All @@ -61,9 +61,7 @@
"ember-cli-dependency-checker": "^3.2.0",
"ember-cli-htmlbars": "^5.1.2",
"ember-cli-inject-live-reload": "^2.0.2",
"ember-cli-internal-test-helpers": "^0.9.1",
"ember-cli-pretender": "^3.2.0",
"ember-cli-shims": "^1.2.0",
"ember-cli-sri": "^2.1.1",
"ember-cli-terser": "~4.0.1",
"ember-cli-test-loader": "^3.0.0",
Expand Down
29 changes: 19 additions & 10 deletions packages/-ember-data/tests/integration/debug-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { settled } from '@ember/test-helpers';

import { module, test } from 'qunit';

import { gte } from 'ember-compatibility-helpers';
import { setupTest } from 'ember-qunit';

import Adapter from '@ember-data/adapter';
Expand Down Expand Up @@ -65,7 +66,7 @@ module('integration/debug-adapter - DS.DebugAdapter', function(hooks) {
test('Watching Records', async function(assert) {
let { owner } = this;
let debugAdapter = owner.lookup('data-adapter:main');
let addedRecords, updatedRecords, removedIndex, removedCount;
let addedRecords, updatedRecords, removedRecords;

this.owner.register(
'adapter:application',
Expand All @@ -86,15 +87,17 @@ module('integration/debug-adapter - DS.DebugAdapter', function(hooks) {
},
});

var recordsAdded = function(wrappedRecords) {
let recordsAdded = function(wrappedRecords) {
addedRecords = wrappedRecords;
};
var recordsUpdated = function(wrappedRecords) {
let recordsUpdated = function(wrappedRecords) {
updatedRecords = wrappedRecords;
};
var recordsRemoved = function(index, count) {
removedIndex = index;
removedCount = count;
let recordsRemoved = function(...args) {
// in 3.26 there is only 1 argument - the record removed
// below 3.26, it is 2 arguments - the index and count removed
// https://github.com/emberjs/ember.js/pull/19379
removedRecords = args;
};

debugAdapter.watchRecords('post', recordsAdded, recordsUpdated, recordsRemoved);
Expand All @@ -114,6 +117,9 @@ module('integration/debug-adapter - DS.DebugAdapter', function(hooks) {

post.set('title', 'Modified Post');

// await updated callback
await settled();

assert.equal(get(updatedRecords, 'length'), 1, 'We updated 1 post');
record = updatedRecords[0];
assert.deepEqual(
Expand All @@ -131,7 +137,6 @@ module('integration/debug-adapter - DS.DebugAdapter', function(hooks) {

// reset
addedRecords = updatedRecords = [];
removedCount = removedIndex = null;

post = store.createRecord('post', { id: '2', title: 'New Post' });

Expand Down Expand Up @@ -159,14 +164,18 @@ module('integration/debug-adapter - DS.DebugAdapter', function(hooks) {

// reset
addedRecords = updatedRecords = [];
removedCount = removedIndex = null;

post.unloadRecord();

await settled();

assert.equal(removedIndex, 1, 'We are notified of the start index of a removal when we remove posts');
assert.equal(removedCount, 1, 'We are notified of the total posts removed');
if (gte('3.26.0')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably add a comment about why we expect these to differ for 3.26+

assert.equal(removedRecords.length, 1, 'We are notified of the total posts removed');
assert.equal(removedRecords[0][0].object, post, 'The removed post is correct');
} else {
assert.equal(removedRecords[0], 1, 'We are notified of the start index of a removal when we remove posts');
assert.equal(removedRecords[1], 1, 'We are notified of the total posts removed');
}
});

test('Column names', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module('integration/store/creation-recursion', function(hooks) {
let storeFactory = this.owner.factoryFor('service:store');

this.owner.unregister('service:store');
this.owner.register('service:store', storeFactory);
this.owner.register('service:store', storeFactory.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What caused this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ember asserts if you pass the factoryFor result directly to register now


let test = this;
test.dateTransformCreated = false;
Expand Down
4 changes: 2 additions & 2 deletions packages/-ember-data/tests/unit/model/merge-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { run } from '@ember/runloop';
import { next, run } from '@ember/runloop';

import { module, test } from 'qunit';
import { Promise as EmberPromise, reject, resolve } from 'rsvp';
Expand Down Expand Up @@ -103,7 +103,7 @@ module('unit/model/merge - Merging', function(hooks) {
updateRecord(store, type, snapshot) {
// Make sure saving isn't resolved synchronously
return new EmberPromise(resolve => {
run.next(null, resolve, {
next(null, resolve, {
data: {
id: 1,
type: 'person',
Expand Down
10 changes: 5 additions & 5 deletions packages/-ember-data/tests/unit/store/adapter-interop-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { A } from '@ember/array';
import { get, set } from '@ember/object';
import { run } from '@ember/runloop';
import { later, run } from '@ember/runloop';

import { module, test } from 'qunit';
import { all, Promise as EmberPromise, resolve } from 'rsvp';
Expand Down Expand Up @@ -693,15 +693,15 @@ module('unit/store/adapter-interop - Store working with a Adapter', function(hoo
let record = { id, type: type.modelName };

return new EmberPromise(resolve => {
run.later(() => resolve({ data: record }), 5);
later(() => resolve({ data: record }), 5);
});
},

findMany(store, type, ids, snapshots) {
let records = ids.map(id => ({ id, type: type.modelName }));

return new EmberPromise(resolve => {
run.later(() => {
later(() => {
resolve({ data: records });
}, 15);
});
Expand Down Expand Up @@ -791,7 +791,7 @@ module('unit/store/adapter-interop - Store working with a Adapter', function(hoo
if (id === 'igor') {
resolve({ data: record });
} else {
run.later(function() {
later(function() {
davidResolved = true;
resolve({ data: record });
}, 5);
Expand Down Expand Up @@ -844,7 +844,7 @@ module('unit/store/adapter-interop - Store working with a Adapter', function(hoo
if (id === 'igor') {
reject({ data: record });
} else {
run.later(() => {
later(() => {
davidResolved = true;
resolve({ data: record });
}, 5);
Expand Down
6 changes: 3 additions & 3 deletions packages/adapter/addon/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getOwner } from '@ember/application';
import { deprecate, warn } from '@ember/debug';
import { computed } from '@ember/object';
import { assign } from '@ember/polyfills';
import { run } from '@ember/runloop';
import { join } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';

import { has } from 'require';
Expand Down Expand Up @@ -1068,12 +1068,12 @@ class RESTAdapter extends Adapter.extend(BuildURLMixin) {
return new RSVPPromise(function(resolve, reject) {
hash.success = function(payload, textStatus, jqXHR) {
let response = ajaxSuccessHandler(adapter, payload, jqXHR, requestData);
run.join(null, resolve, response);
join(null, resolve, response);
};

hash.error = function(jqXHR, textStatus, errorThrown) {
let error = ajaxErrorHandler(adapter, jqXHR, errorThrown, requestData);
run.join(null, reject, error);
join(null, reject, error);
};

adapter._ajax(hash);
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"@ember-data/store": "3.27.0-alpha.0",
"@ember/edition-utils": "^1.2.0",
"@ember/string": "^1.0.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^4.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/canary-features/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
},
"scripts": {},
"dependencies": {
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-typescript": "^4.0.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/debug/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@ember-data/private-build-infra": "3.27.0-alpha.0",
"@ember/edition-utils": "^1.2.0",
"@ember/string": "^1.0.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^4.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/model/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"@ember-data/store": "3.27.0-alpha.0",
"@ember/edition-utils": "^1.2.0",
"@ember/string": "^1.0.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-string-utils": "^1.1.0",
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^4.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/private-build-infra/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"broccoli-rollup": "^4.1.1",
"calculate-cache-key-for-tree": "^2.0.0",
"chalk": "^4.0.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-path-utils": "^1.0.0",
"ember-cli-string-utils": "^1.1.0",
"ember-cli-typescript": "^3.1.3",
Expand Down
4 changes: 2 additions & 2 deletions packages/record-data/addon/-private/record-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { assert, inspect, warn } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { run } from '@ember/runloop';
import { _backburner as emberBackburner } from '@ember/runloop';
import { isEqual } from '@ember/utils';
import { DEBUG } from '@glimmer/env';

Expand Down Expand Up @@ -437,7 +437,7 @@ export default class RecordDataDefault implements RelationshipRecordData {
this._destroyRelationships();
this.reset();
if (!this._scheduledDestroy) {
this._scheduledDestroy = run.backburner.schedule('destroy', this, '_cleanupOrphanedRecordDatas');
this._scheduledDestroy = emberBackburner.schedule('destroy', this, '_cleanupOrphanedRecordDatas');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does run complain now when you access backburner on it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/record-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"@ember-data/store": "3.27.0-alpha.0",
"@ember/edition-utils": "^1.2.0",
"@ember/ordered-set": "^4.0.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^4.0.0"
},
Expand Down
1 change: 1 addition & 0 deletions packages/record-data/types/@ember/runloop/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
// which we use to avoid autorun triggering for Ember <= 3.4
// we can drop this and use run directly ~11/1/2019
export const run: any;
export const _backburner: any;
2 changes: 1 addition & 1 deletion packages/serializer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"dependencies": {
"@ember-data/private-build-infra": "3.27.0-alpha.0",
"@ember-data/store": "3.27.0-alpha.0",
"ember-cli-babel": "^7.18.0",
"ember-cli-babel": "^7.26.3",
"ember-cli-test-info": "^1.0.0",
"ember-cli-typescript": "^4.0.0"
},
Expand Down
1 change: 1 addition & 0 deletions packages/store/addon/-private/system/backburner.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { registerWaiter } from '@ember/test';
import { DEBUG } from '@glimmer/env';
import Ember from 'ember';

// TODO: expose Ember._Backburner as `import { _Backburner } from '@ember/runloop'` in ember-rfc176-data + emberjs/ember.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please clarify this a bit, not quite sure what the future work required is

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ember doesn't expose the constructor for Backburner in a modules API

const backburner = new Ember._Backburner(['normalizeRelationships', 'syncRelationships', 'finished']);

if (DEBUG) {
Expand Down
11 changes: 5 additions & 6 deletions packages/store/addon/-private/system/core-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { A } from '@ember/array';
import { assert, deprecate, inspect, warn } from '@ember/debug';
import { computed, defineProperty, get, set } from '@ember/object';
import { assign } from '@ember/polyfills';
import { run as emberRunLoop } from '@ember/runloop';
import { _backburner as emberBackburner } from '@ember/runloop';
import Service from '@ember/service';
import { registerWaiter, unregisterWaiter } from '@ember/test';
import { isNone, isPresent, typeOf } from '@ember/utils';
Expand Down Expand Up @@ -95,7 +95,6 @@ type Relationship = import('@ember-data/record-data/-private').Relationship;
type RecordDataClass = typeof import('@ember-data/record-data/-private').RecordData;

let _RecordData: RecordDataClass | undefined;
const emberRun = emberRunLoop.backburner;

const { ENV } = Ember;
type AsyncTrackingToken = Readonly<{ label: string; trace: Error | string }>;
Expand Down Expand Up @@ -603,7 +602,7 @@ abstract class CoreStore extends Service {
// of record-arrays via ember's run loop, not our own.
//
// to remove this, we would need to move to a new `async` API.
return emberRun.join(() => {
return emberBackburner.join(() => {
return this._backburner.join(() => {
let normalizedModelName = normalizeModelName(modelName);
let properties = assign({}, inputProperties);
Expand Down Expand Up @@ -1261,7 +1260,7 @@ abstract class CoreStore extends Service {

internalModel.loadingData(promise);
if (this._pendingFetch.size === 0) {
emberRun.schedule('actions', this, this.flushAllPendingFetches);
emberBackburner.schedule('actions', this, this.flushAllPendingFetches);
}

let fetches = this._pendingFetch;
Expand Down Expand Up @@ -2518,7 +2517,7 @@ abstract class CoreStore extends Service {
resolver: resolver,
});

emberRun.scheduleOnce('actions', this, this.flushPendingSave);
emberBackburner.scheduleOnce('actions', this, this.flushPendingSave);
}

/**
Expand Down Expand Up @@ -3618,7 +3617,7 @@ abstract class CoreStore extends Service {
return;
}

emberRun.schedule('actions', this, this._flushUpdatedInternalModels);
emberBackburner.schedule('actions', this, this._flushUpdatedInternalModels);
}

_flushUpdatedInternalModels() {
Expand Down
Loading