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
Island rhythms/sync schema options for discriminators #12928
Conversation
lib/helpers/model/discriminator.js
Outdated
const toJSON = schema.options.toJSON; | ||
const toObject = schema.options.toObject; | ||
const _id = schema.options._id; | ||
const id = schema.options.id; | ||
|
||
const keys = Object.keys(schema.options); | ||
schema.options.discriminatorKey = baseSchema.options.discriminatorKey; | ||
|
||
const hasUserProvidedTypeKey = schema._userProvidedOptions.typeKey != null && baseSchema._userProvidedOptions.typeKey != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a step in the right direction, but I'd like for you to generalize this check for typeKey
to all schema options. So if user didn't specify other options in discriminator schema (capped, etc.) then use the base schema's options by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making the changes you requested causes the test mentioned earlier in the thread to fail.
… schema options Fix #12135
So moving this line above the for loop causes this test to fail. It also doesn't solve the issue. The if statement that was added cannot be appended to the check in the for loop for deep equality or it would also cause that same test to fail.