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

fix(virtualtype): order in applyGetters and applySetters #8897

Merged
merged 3 commits into from
May 2, 2020

Conversation

ggurkal
Copy link
Contributor

@ggurkal ggurkal commented Apr 29, 2020

Summary

Virtual's getter and setter functions are called in reverse order. As for the example below, second function is called first, then the first function called next. However, the opposite would be the logical order.

SomeSchema.virtual('something').get(function() {
  return 'first!';
}).get(function() {
  return 'second';
})

Fixes #8892

Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

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

Welcome @ggurkal

Changing the current tests could mean we're introducing breaking changes, and we have to take those very carefully. Please revert the changes to the current tests and introduce a new test(s) demonstrating what the changes are intended to achieve.

@hasezoey
Copy link
Collaborator

@AbdelrahmanHafez from what i see this pr is just extending the test for getters & setters (to also test class inheritance)

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 but we'll have to put this change in 6.0. This could be backwards breaking.

@vkarpov15 vkarpov15 changed the base branch from master to 6.0 May 2, 2020 15:51
@vkarpov15 vkarpov15 added this to the 6.0 milestone May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schema.loadClass overwrite methods from inherited class
4 participants