Skip to content
This repository has been archived by the owner on Jun 29, 2021. It is now read-only.

Error with required nested elements #8

Closed
JoseLuisGarciaOtt opened this issue Jul 19, 2017 · 6 comments
Closed

Error with required nested elements #8

JoseLuisGarciaOtt opened this issue Jul 19, 2017 · 6 comments
Labels

Comments

@JoseLuisGarciaOtt
Copy link
Contributor

If you put { required: true } in the prop annotation over a nested element you get this error:

TypeError: Undefined type undefined at job.required
Did you try nesting Schemas? You can only nest using refs or arrays.

You can try it with the same code of your md file example:

class Job extends Typegoose {
    @prop()
    title: string;

    @prop()
    position: string;
}

class Contact extends Typegoose {

    @prop({ required: true }) // Added the required constraint
    job: Job;


}

The work around is... not using "required: true" over nested elements (yay!).

I'm using:
Hapi 16.4.3,
typegoose 3.0.0,
typescript 2.4.1,
ts-node 3.1.0,

Once again, let me know if you can reproduce this.
Thanks in advance!

@JoseLuisGarciaOtt
Copy link
Contributor Author

This is happening with @arrayProp() too. Can't use required: true unless the items type is a primitive one.

@szokodiakos
Copy link
Owner

I am able to reproduce this, and will investigate this asap. Thanks for your report!

@JoseLuisGarciaOtt
Copy link
Contributor Author

JoseLuisGarciaOtt commented Jul 21, 2017

I have been digging a little in your code so I could give you more light on this...

How is your lib working right now:

You can't mark required to any nested element. And If any nested element has any of its attributes marked as required, then the parent is also required automatically. This is bad... since you cant make a nested element optional if one of his attributes is marked as required.

I tried with the following change in props.ts for the not-primitive not-array flow:


return schema[name][key] = {
    ...schema[name][key],
    ...options,
    type: {...subSchema},  // changes here!
  };

// this will transpile to { type: subSchema }

With this change I can use @prop({required: true}) over nested elements and they validate their existence just fine. But this generates a problem: there is now no effect in marking require on any of the attributes of the nested element. In Brief: We can now test if the element exists, but not their inner elements.

After investigating a little, I see this is a problem regarding with mongoose, since if you have this..:

const UserSchema = new Schema({
	child: { type: {attr1: { type: String, required: true}, attr2: String}, required: true}
});

..the behaviour is the same: it ignores completely the attr1's required flag.

This seems to be an old issue (or bad design): Automattic/mongoose#1860

Possible solution:

This problem is only related with explicit embedded documents. If you use sub-documents instead...:


const childSchema = new Schema({
    attr1: { type: String, required: true },
    attr2: String
});

const UserSchema = new Schema({
    firstName: { type: String, required: true },
    child: { type: childSchema, required: true}
    });

..the behaviour is the expected one: you have control over the whole sub-doc and the sub-attributes separately.

So... it seems that your implementation mimics the embedded documents way when dealing with nested documents. I think the solution would be to implement it as the sub-documents way. I tried to do it but I think I'm not following your code right, since I'm getting..:

TypeError: Undefined type Job at job

..where Job is the child prop, you know, like in your example.

So well, I hope all this help you debugging this bug/feature.

note: The only posibble downside of implementing this, is that using sub-documents you get an _id field for the child sub document... Really not a big concern here.

note2: I have not tested the "type: {...subSchema}" solution for the case of Arrays... I think the solution there should be similar, but not quite sure. Anyway, as I already said, with this solution you will still get a mimic for the embedded documents style (but fully functional, without the actual crashes). And I think that the final solution should not be that... since it would be great having granular control just like when using sub-documents.

Cheers!

@szokodiakos
Copy link
Owner

Hello, thanks for your investigation. I was going down the same path as you did however I couldn't craft a good solution at the time of writing. I will try to get back to it and figure this out.

@joakimahlen
Copy link

joakimahlen commented Sep 22, 2017

I am having this issue. Any news regarding fixing it?

I also cannot make "unique: true" to work at all - duplicates can be inserted into the database even with this property set. The index on the actual column is created, but not marked as unique. Any ideas?

My package.json says I am using the following version:

"typegoose": "^3.4.5"

@richarddd
Copy link
Contributor

@szokodiakos take a look at my PR. The hints from @JoseLuisGarciaOtt got it working. Please check it out :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants