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 to 3.1 and fix ObjectProxy extends validations bug #586

Merged
merged 2 commits into from
Apr 23, 2018

Conversation

danielspaniel
Copy link
Contributor

Resolves an issue where you get this error:

Assertion Failed: You attempted to access the __VALIDATIONS_MIXIN_COUNT__ property (of <(unknown mixin):ember391>).↵Since Ember 3.1, this is usually fine as you no longer need to use .get()↵to access computed properties. However, in this case, the object in question↵is a special kind of Ember object (a proxy). Therefore, it is still necessary↵to use .get('__VALIDATIONS_MIXIN_COUNT__')

when you try to extend ObjectProxy with validations and the content you are proxying is an ember-data model

Changes proposed:

  • minor cleanup/update in ValidationsMixin within buildValidations to account for issues with extending ObjectProxy with Validations
  • does not need documentation because it is merely fixing internal implementation

Tasks:

  • [+ ] Added test case(s)

@@ -101,8 +101,8 @@ export default function buildValidations(validations = {}, globalOptions = {}) {
this._super(...arguments);

// Count number of mixins to bypass super check if there is more than 1
this[VALIDATIONS_MIXIN_COUNT] = this[VALIDATIONS_MIXIN_COUNT] || 0;
validationMixinCount = ++this[VALIDATIONS_MIXIN_COUNT];
validationMixinCount = (this.get(VALIDATIONS_MIXIN_COUNT) || 0) + 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

get(this, VALIDATIONS_MIXIN_COUNT)

this[VALIDATIONS_MIXIN_COUNT] = this[VALIDATIONS_MIXIN_COUNT] || 0;
validationMixinCount = ++this[VALIDATIONS_MIXIN_COUNT];
validationMixinCount = (this.get(VALIDATIONS_MIXIN_COUNT) || 0) + 1;
this.set(VALIDATIONS_MIXIN_COUNT, validationMixinCount);
Copy link
Collaborator

Choose a reason for hiding this comment

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

set(this, VALIDATIONS_MIXIN_COUNT, validationMixinCount)

@offirgolan
Copy link
Collaborator

@danielspaniel thank you for this PR 😸. Added 2 small comments but other than that, lgtm!

@danielspaniel
Copy link
Contributor Author

All set @offirgolan

@offirgolan
Copy link
Collaborator

Thanks again @danielspaniel!

@offirgolan offirgolan merged commit d4e720d into adopted-ember-addons:master Apr 23, 2018
@danielspaniel
Copy link
Contributor Author

your welcome @offirgolan

@Turbo87
Copy link
Contributor

Turbo87 commented Apr 24, 2018

@offirgolan would be open to backporting this fix to the v3.x branch?

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

3 participants