meteor-astronomy
meteor-astronomy copied to clipboard
Model validation always fails when having duplicate classes
Astronomy doesn't handle the attempt to Class.create() with the name of already existing class. Example: we have the next three files in project:
// file: MyModel.js
export const MyModel = Class.create({
name: 'MyModel',
fields: {
foo: String
}
});
// file: OtherModel.js
const MyModel = Class.create({
name: 'MyModel',
fields: {
bar: String
}
});
export const OtherModel = Class.create({
name: 'MyModel',
fields: {
myModel: MyModel,
default: function() { return new MyModel(); }
}
});
// file: app.js
import { OtherModel } from 'OtherModel';
let otherModel = new OtherModel();
otherModel.myModel.bar = "123";
myModel.validate();
The validate() call on the last line will throw Error saying "myModel.bar has to be MyModel".
Looking at the Astronomy source /lib/core/class.js:140
let Class = this.classes[className] = class Class extends this {};
it's pretty clear that in such case the second declared class will simply overwrite the previous one.
Could you please take a look?
Thanks
One thing first, always check your examples before posting. You have several errors and first I had to figure out what you wanted to achieve, to actually start thinking about the answer to your problem. Don't give me extra work and always prepare reproduction repository. Preparing such a reproduction is a matter of a few minutes.
Why would anyone want to override class? If you do so, then you're probably doing something wrong. In the real-life application, no one would risk introducing two classes with the same name, because it's causing more confusion. So your problem can probably be solved just by renaming class.
However, after fixing your example everything works as expected so it's not a problem with Astronomy but with your code. Here is fixed code:
// MyModel.js
import { Class } from "meteor/jagi:astronomy";
export const MyModel = Class.create({
name: "MyModel",
fields: {
foo: String
}
});
// OtherModel.js
import { Class } from "meteor/jagi:astronomy";
const MyModel = Class.create({
name: "MyModel",
fields: {
bar: String
}
});
export const OtherModel = Class.create({
name: "OtherModel",
fields: {
myModel: {
type: MyModel,
default() {
return new MyModel();
}
}
}
});
// app.js
import { MyModel } from "./MyModel"; // Just to demonstrate that it's being imported
import { OtherModel } from "./OtherModel";
const otherModel = new OtherModel();
otherModel.myModel.bar = "123";
otherModel.validate(); // No error thrown
You can also made this ugly check to make sure that indeed "otherModel.myModel.bar" requires string:
otherModel.constructor.schema.fields.myModel.type.class.schema.fields.bar.type.name // Returns "String"
@lukejagodzinski I also have such issue. Of course I didn't have intent to override class with another class with same name. It was copy-paste mistake but your framework doesn't have check to prevent such kind of mistakes. It will be really good if you add it in next versions.
@DarthRumata indeed such a check would be a good idea to do just to prevent accidental overrides. I will fix that in the next release.