Skip to content

Commit

Permalink
fix: prevent public class fields from shadowing association values (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ephys committed Sep 12, 2023
1 parent 8acefff commit b66db0b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
13 changes: 10 additions & 3 deletions packages/core/src/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,14 @@ ${associationOwner._getAssociationDebugList()}`);
delete instance[attributeName];
}

// If there are associations in the instance, we assign them as properties on the instance
// so that they can be accessed directly, instead of having to call `get` and `set`.
// class properties re-assign them to whatever value was set on the class property (or undefined if none)
// so this workaround re-assigns the association after the instance was created.
for (const associationName of Object.keys(this.modelDefinition.associations)) {
instance[associationName] = instance.getDataValue(associationName);
}

return instance;
}

Expand Down Expand Up @@ -3527,7 +3535,6 @@ Instead of specifying a Model, either:

const include = this._options.includeMap[key];
const association = include.association;
const accessor = key;
const primaryKeyAttribute = include.model.primaryKeyAttribute;
const childOptions = {
isNewRecord: this.isNewRecord,
Expand All @@ -3548,10 +3555,10 @@ Instead of specifying a Model, either:
}

isEmpty = value && value[primaryKeyAttribute] === null || value === null;
this[accessor] = this.dataValues[accessor] = isEmpty ? null : include.model.build(value, childOptions);
this[key] = this.dataValues[key] = isEmpty ? null : include.model.build(value, childOptions);
} else {
isEmpty = value[0] && value[0][primaryKeyAttribute] === null;
this[accessor] = this.dataValues[accessor] = isEmpty ? [] : include.model.bulkBuild(value, childOptions);
this[key] = this.dataValues[key] = isEmpty ? [] : include.model.bulkBuild(value, childOptions);
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions packages/core/test/unit/model/overwriting-builtins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,24 @@ describe('Model', () => {
expect(user.firstName).to.eq('Zoe');
expect(user.nonAttribute).to.eq('def');
});

it('makes associations take priority over class properties defined on the model', () => {
// This is the same issue as above, but for associations
class User extends Model<InferAttributes<User>, InferCreationAttributes<User>> {
profile?: Profile;
}

class Profile extends Model<InferAttributes<Profile>, InferCreationAttributes<Profile>> {
userId: number = 10;
}

User.init({}, { sequelize });
Profile.init({}, { sequelize });

User.hasOne(Profile, { as: 'profile', foreignKey: 'userId' });

// @ts-expect-error -- TODO: add typing for creating associations in build/create
const user = User.build({ profile: {} }, { include: ['profile'] });
expect(user.profile).to.be.instanceof(Profile);
});
});

0 comments on commit b66db0b

Please sign in to comment.