Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update schema.js #9185

Merged
merged 1 commit into from Jul 1, 2020
Merged

Update schema.js #9185

merged 1 commit into from Jul 1, 2020

Conversation

joaoritter
Copy link
Contributor

@joaoritter joaoritter commented Jun 29, 2020

Summary

I've been trying to debug the following problem: when a custom typeKey and typePojoToMixed: false are passed as options to a schema, schema properties under the new typeKey that are a POJO get set as Mixed in the schema. This is incorrect, the type should be dynamically converted to a subschema, as per the line I have changed in this PR.
My change preserves the typeKey during the Object.assign, instead of using the default key of type.

Examples

const schema = mongoose.Schema({ a: { $type: { b: String, c: String } }}, { typeKey: '$type', typePojoToMixed: false });

// current buggy behavior: 
console.log({ schema }) // Schema { ..., paths: {  'a': [Mixed] } }   

// expected behavior: 
console.log({ schema }) // Schema { ..., paths: {  'a.b': [SchemaString], 'a.c': [SchemaString] } }   

@joaoritter
Copy link
Contributor Author

I also noticed there are no test cases for setting a typeKey to a POJO. It's probably worth adding one if you intend to support it, or add to documentation if not.

@joaoritter
Copy link
Contributor Author

This is blocking me right now, I'd appreciate a quick acknowledgement of this PR. I understand if it takes a bit to merge, but I just want to make sure im not missing something simple. I could attempt other work arounds if this is in fact a bug and it'll take a while to merge a fix.

@joaoritter
Copy link
Contributor Author

I dug up this commit where this line was last changed. It would be useful to add a test with a typeKey here.
b4b61d3

Thanks!

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@vkarpov15 vkarpov15 added this to the 5.9.21 milestone Jul 1, 2020
@vkarpov15 vkarpov15 merged commit 6fe937f into Automattic:master Jul 1, 2020
@joaoritter
Copy link
Contributor Author

100%, thank YOU for all your work on this project.

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

Successfully merging this pull request may close these issues.

None yet

2 participants