Skip to content

Commit 77f6de5

Browse files
author
Lee Richmond
committed
Fix attribute inheritance bug
Though user-facing behavior was (mostly) unaffected, there was an under-the-hood issue with unrelated models getting each others' attributes. The `Model.attributeList` was being shared across all models - you'd see typescript errors but if you ignored them you would have shared attribute lists across models. In other words: ```ts class Person extends ApplicationRecord { firstName = attr() } class Truck extends ApplicationRecord { wheels = attr() } let p = new Person(); p.wheels // typescript error, but p['wheels'] works // and vice versa for Truck['firstName'] // In a practical sense, here's the real-world bug: let truck = new Truck({ firstName: 'foo' }) expect(truck['firstName']).to.eq(undefined) // but is actually 'foo' ``` To fix this, ensure we clone the `attributeList` within the `inherited` hook, so each class has its own unique attributeList. However, this means subclasses will no longer inherit their parents' attributes. So, after we define all the attribute lists, merge parent and child lists.
1 parent e05e795 commit 77f6de5

File tree

4 files changed

+28
-3
lines changed

4 files changed

+28
-3
lines changed

src/configuration.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
import Model from './model';
44
import Attribute from './attribute';
55
import Logger from './logger';
6+
import * as _cloneDeep from './util/clonedeep';
7+
let cloneDeep: any = (<any>_cloneDeep).default || _cloneDeep;
8+
if (cloneDeep.default) {
9+
cloneDeep = cloneDeep.default;
10+
}
611

712
let ctx = this;
813

@@ -25,6 +30,12 @@ export default class Config {
2530
for (let model of this.models) {
2631
Attribute.applyAll(model);
2732
}
33+
34+
for (let model of this.models) {
35+
let parentAttrList = cloneDeep(model.parentClass.attributeList);
36+
let attrList = cloneDeep(model.attributeList);
37+
model.attributeList = Object.assign(parentAttrList, attrList);
38+
}
2839
}
2940

3041
static reset() : void {

src/model.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ export default class Model {
4242
klass: typeof Model;
4343

4444
static attributeList = {};
45-
static relationList = [];
4645
private static _scope: Scope;
4746

4847
static extend(obj : any) : any {
@@ -53,6 +52,7 @@ export default class Model {
5352
Config.models.push(subclass)
5453
subclass.parentClass = this;
5554
subclass.prototype.klass = subclass;
55+
subclass.attributeList = cloneDeep(subclass.attributeList)
5656
}
5757

5858
static scope(): Scope {

test/integration/finders-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe('Model finders', function() {
1919
id: '1',
2020
type: 'people',
2121
attributes: {
22-
name: 'John'
22+
firstName: 'John'
2323
}
2424
}
2525
});
@@ -35,7 +35,7 @@ describe('Model finders', function() {
3535

3636
it('assigns attributes correctly', function(done) {
3737
resultData(Person.find(1)).then((data) => {
38-
expect(data).to.have.property('name', 'John');
38+
expect(data).to.have.property('firstName', 'John');
3939
done();
4040
});
4141
});

test/unit/attributes-test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,19 @@ describe('Model attributes', function() {
3636
let person = new Person({ foo: 'bar' });
3737
expect(person['foo']).to.eq(undefined);
3838
});
39+
40+
describe('but that attribute exists in an unrelated model', function() {
41+
it('still silently drops', function() {
42+
let person = new Person({ title: 'bar' });
43+
expect(person['title']).to.eq(undefined);
44+
});
45+
});
46+
47+
describe('but that attribute exists in a subclass', function() {
48+
it('still silently drops', function() {
49+
let person = new Person({ extraThing: 'bar' });
50+
expect(person['extraThing']).to.eq(undefined);
51+
});
52+
});
3953
})
4054
});

0 commit comments

Comments
 (0)