Skip to content

Commit

Permalink
marmelab#57 Fix for..in bug, add tests to guard against regressions.
Browse files Browse the repository at this point in the history
  • Loading branch information
RichardBradley committed May 24, 2016
1 parent 5fb0e6c commit b821168
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 17 deletions.
2 changes: 2 additions & 0 deletions lib/DataStore/DataStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,15 @@ class DataStore {

fillReferencesValuesFromEntry(entry, referencedValues, fillSimpleReference) {
for (let referenceField in referencedValues) {
if (!referencedValues.hasOwnProperty(referenceField)) continue;
let reference = referencedValues[referenceField],
choices = this.getReferenceChoicesById(reference),
entries = [],
identifier = reference.getMappedValue(entry.values[referenceField], entry.values);

if (reference.type() === 'reference_many') {
for (let i in identifier) {
if (!identifier.hasOwnProperty(i)) continue;
let id = identifier[i];
entries.push(choices[id]);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Field/Field.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ class Field {
}

getMappedValue(value, entry) {
for (let i in this._maps) {
value = this._maps[i](value, entry);
for (let mapFn of this._maps) {
value = mapFn(value, entry);
}

return value;
Expand All @@ -125,8 +125,8 @@ class Field {
}

getTransformedValue(value, entry) {
for (let i in this._transforms) {
value = this._transforms[i](value, entry);
for (let transformFn of this._transforms) {
value = transformFn(value, entry);
}

return value;
Expand Down
4 changes: 2 additions & 2 deletions lib/Field/ReferenceField.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ class ReferenceField extends Field {
}

if (identifier instanceof Array) {
for (let j in identifier) {
results[identifier[j]] = true;
for (let j of identifier) {
results[j] = true;
}
continue;
}
Expand Down
14 changes: 11 additions & 3 deletions lib/Queries/ReadQueries.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,11 @@ class ReadQueries extends Queries {
let data = {};
let name;
for (name in results[0]) {
if (!results[0].hasOwnProperty(name)) continue;
data[name] = results[0][name];
}
for (name in results[1]) {
if (!results[1].hasOwnProperty(name)) continue;
data[name] = results[1][name];
}
return data;
Expand All @@ -152,11 +154,13 @@ class ReadQueries extends Queries {
calls = [];

for (let i in references) {
if (!references.hasOwnProperty(i)) continue;
let reference = references[i],
targetEntity = reference.targetEntity(),
identifiers = reference.getIdentifierValues(rawValues);

for (let k in identifiers) {
if (!identifiers.hasOwnProperty(k)) continue;
calls.push(getOne(targetEntity, 'listView', identifiers[k], reference.name()));
}
}
Expand All @@ -182,6 +186,7 @@ class ReadQueries extends Queries {
calls = [];

for (let i in references) {
if (!references.hasOwnProperty(i)) continue;
let reference = references[i],
targetEntity = reference.targetEntity(),
identifiers = reference.getIdentifierValues(rawValues);
Expand Down Expand Up @@ -210,8 +215,7 @@ class ReadQueries extends Queries {
let calls = [],
getRawValues = this.getRawValues.bind(this);

for (let i in references) {
let reference = references[i];
for (let reference of references) {
let targetEntity = reference.targetEntity();

const permanentFilters = reference.permanentFilters();
Expand Down Expand Up @@ -274,6 +278,7 @@ class ReadQueries extends Queries {
i = 0;

for (let j in references) {
if (!references.hasOwnProperty(j)) continue;
let reference = references[j],
response = responses[i++];

Expand Down Expand Up @@ -310,11 +315,12 @@ class ReadQueries extends Queries {
i = 0;

for (let j in references) {
if (!references.hasOwnProperty(j)) continue;
let data = [],
reference = references[j],
identifiers = reference.getIdentifierValues(rawValues);

for (let k in identifiers) {
for (let k of identifiers) {
response = responses[i++];
if (response.status == 'error') {
// one of the responses failed
Expand Down Expand Up @@ -349,6 +355,7 @@ class ReadQueries extends Queries {
calls = [];

for (let i in referencedLists) {
if (!referencedLists.hasOwnProperty(i)) continue;
let referencedList = referencedLists[i],
targetEntity = referencedList.targetEntity(),
viewName = referencedList.datagridName(),
Expand Down Expand Up @@ -376,6 +383,7 @@ class ReadQueries extends Queries {
entries = {};

for (let i in referencedLists) {
if (!referencedLists.hasOwnProperty(i)) continue;
let response = responses[j++];
if (response.status == 'error') {
// If a response fail, skip it
Expand Down
4 changes: 2 additions & 2 deletions lib/Utils/PromisesResolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class PromisesResolver {
function resolveState(result) {
states[key] = true;
results[key] = result; // result may be an error
for (let i in states) {
if (!states[i]) {
for (let state of states) {
if (!state) {
return;
}
}
Expand Down
7 changes: 3 additions & 4 deletions lib/Utils/orderElement.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
export default {
order: function (input) {
var results = [],
objectKey;
var results = [];

for (objectKey in input) {
results.push(input[objectKey]);
for (let i of input) {
results.push(i);
}

return results.sort((e1, e2) => e1.order() - e2.order());
Expand Down
4 changes: 2 additions & 2 deletions lib/View/View.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ class View {
if (arg.constructor.name === 'Object') {
console.warn('Passing literal of Field to fields method is deprecated use array instead');
let result = [];
for (let fieldName in arg) {
result = result.concat(View.flatten(arg[fieldName]));
for (let field of arg) {
result = result.concat(View.flatten(field));
}
return result;
}
Expand Down
20 changes: 20 additions & 0 deletions tests/before_all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

beforeEach(function() {
if (!Object.prototype.test_prototype_entry) {
Object.prototype.test_prototype_entry =
"Don't use for..in to enumerate Object properties, as users are free to " +
"add entries to the Object prototype, for example for polyfills. " +
"You should instead use\n" +
" for (let i in xs) { if (!xs.hasOwnProperty(i)) continue; var x = xs[i]; ... }";
}

if (!Array.prototype.test_prototype_entry) {
Array.prototype.test_prototype_entry =
"Don't use for..in to enumerate Array properties, as users are free to " +
"add entries to the Array prototype, for example for polyfills. " +
"You should instead use\n" +
" for (let i in xs) { if (!xs.hasOwnProperty(i)) continue; var x = xs[i]; ... }\n" +
"or\n" +
" for (let x of xs) { ... }";
}
});

0 comments on commit b821168

Please sign in to comment.