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

Cleanup record array manager #4591

Merged
merged 18 commits into from
Oct 19, 2016
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
36 changes: 12 additions & 24 deletions addon/-private/system/promise-proxies.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import Ember from 'ember';
import { assert } from "ember-data/-private/debug";

var Promise = Ember.RSVP.Promise;
var get = Ember.get;
const { get , RSVP: { Promise }} = Ember;

/**
A `PromiseArray` is an object that acts like both an `Ember.Array`
Expand Down Expand Up @@ -33,7 +32,7 @@ var get = Ember.get;
@extends Ember.ArrayProxy
@uses Ember.PromiseProxyMixin
*/
var PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin);
export const PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin);

/**
A `PromiseObject` is an object that acts like both an `Ember.Object`
Expand Down Expand Up @@ -64,19 +63,19 @@ var PromiseArray = Ember.ArrayProxy.extend(Ember.PromiseProxyMixin);
@extends Ember.ObjectProxy
@uses Ember.PromiseProxyMixin
*/
var PromiseObject = Ember.ObjectProxy.extend(Ember.PromiseProxyMixin);
export const PromiseObject = Ember.ObjectProxy.extend(Ember.PromiseProxyMixin);

var promiseObject = function(promise, label) {
export function promiseObject(promise, label) {
return PromiseObject.create({
promise: Promise.resolve(promise, label)
});
};
}

var promiseArray = function(promise, label) {
export function promiseArray(promise, label) {
return PromiseArray.create({
promise: Promise.resolve(promise, label)
});
};
}

/**
A PromiseManyArray is a PromiseArray that also proxies certain method calls
Expand All @@ -96,14 +95,13 @@ var promiseArray = function(promise, label) {
@extends Ember.ArrayProxy
*/

function proxyToContent(method) {
export function proxyToContent(method) {
return function() {
var content = get(this, 'content');
return content[method].apply(content, arguments);
return get(this, 'content')[method](...arguments);
};
}

var PromiseManyArray = PromiseArray.extend({
export const PromiseManyArray = PromiseArray.extend({
reload() {
//I don't think this should ever happen right now, but worth guarding if we refactor the async relationships
assert('You are trying to reload an async manyArray before it has been created', get(this, 'content'));
Expand All @@ -125,18 +123,8 @@ var PromiseManyArray = PromiseArray.extend({
has: proxyToContent('has')
});

var promiseManyArray = function(promise, label) {
export function promiseManyArray(promise, label) {
return PromiseManyArray.create({
promise: Promise.resolve(promise, label)
});
};


export {
PromiseArray,
PromiseObject,
PromiseManyArray,
promiseArray,
promiseObject,
promiseManyArray
};
}
4 changes: 2 additions & 2 deletions addon/-private/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export default Ember.Object.extend({
this.changedRecords.forEach(internalModel => {

if (internalModel.isDestroyed ||
internalModel.currentState.stateName === 'root.deleted.saved') {
internalModel.currentState.stateName === 'root.deleted.saved') {
this._recordWasDeleted(internalModel);
} else {
this._recordWasChanged(internalModel);
Expand All @@ -123,7 +123,6 @@ export default Ember.Object.extend({
record._recordArrays = null;
},


_recordWasChanged(record) {
heimdall.increment(_recordWasChanged);
let typeClass = record.type;
Expand Down Expand Up @@ -152,6 +151,7 @@ export default Ember.Object.extend({
this._addRecordToRecordArray(liveRecordArray, record);
}
},

/**
Update an individual filter.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import isEnabled from 'ember-data/-private/features';
@module ember-data
*/

var get = Ember.get;
const { get } = Ember;

/**
Represents an ordered list of records whose order and membership is
Expand All @@ -20,11 +20,15 @@ var get = Ember.get;
@extends DS.RecordArray
*/
export default RecordArray.extend({
query: null,
init() {
this._super(...arguments);
this.query = this.query || null;
Copy link
Member

Choose a reason for hiding this comment

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

Why the guard against existing on query and not on links

Copy link
Member Author

Choose a reason for hiding this comment

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

this is due to links is state provided by internal loadRecords, and not via the constructor

this.links = null;
},

replace() {
var type = get(this, 'type').toString();
throw new Error("The result of a server query (on " + type + ") is immutable.");
let type = get(this, 'type').toString();
throw new Error(`The result of a server query (on ${type}) is immutable.`);
},

_update() {
Expand All @@ -44,7 +48,7 @@ export default RecordArray.extend({
loadRecords(records, payload) {
let token = heimdall.start('AdapterPopulatedRecordArray.loadRecords');
//TODO Optimize
var internalModels = Ember.A(records).mapBy('_internalModel');
let internalModels = records.map(record => get(record, '_internalModel'));
Copy link
Member

@igorT igorT Oct 19, 2016

Choose a reason for hiding this comment

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

is mapBy not that great?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Ember.A() is heavy weight, for merely doing something map + => can do.

this.setProperties({
content: Ember.A(internalModels),
isLoaded: true,
Expand All @@ -56,7 +60,7 @@ export default RecordArray.extend({
this.set('links', cloneNull(payload.links));
}

internalModels.forEach((record) => {
internalModels.forEach(record => {
this.manager.recordArraysForRecord(record).add(this);
});

Expand Down
20 changes: 13 additions & 7 deletions addon/-private/system/record-arrays/filtered-record-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import RecordArray from "ember-data/-private/system/record-arrays/record-array";
@module ember-data
*/

var get = Ember.get;
const { get } = Ember;

/**
Represents a list of records whose membership is determined by the
Expand All @@ -18,6 +18,12 @@ var get = Ember.get;
@extends DS.RecordArray
*/
export default RecordArray.extend({
init() {
this._super(...arguments);

this.set('filterFunction', this.get('filterFunction') || null);
this.isLoaded = true;
},
/**
The filterFunction is a function used to test records from the store to
determine if they should be part of the record array.
Expand All @@ -44,21 +50,21 @@ export default RecordArray.extend({
@param {DS.Model} record
@return {Boolean} `true` if the record should be in the array
*/
filterFunction: null,
isLoaded: true,

replace() {
var type = get(this, 'type').toString();
throw new Error("The result of a client-side filter (on " + type + ") is immutable.");
let type = get(this, 'type').toString();
throw new Error(`The result of a client-side filter (on ${type}) is immutable.`);
},

/**
@method updateFilter
@private
*/
_updateFilter() {
var manager = get(this, 'manager');
manager.updateFilter(this, get(this, 'type'), get(this, 'filterFunction'));
if (get(this, 'isDestroying') || get(this, 'isDestroyed')) {
return;
}
get(this, 'manager').updateFilter(this, get(this, 'type'), get(this, 'filterFunction'));
},

updateFilter: Ember.observer('filterFunction', function() {
Expand Down
Loading