-
-
Notifications
You must be signed in to change notification settings - Fork 697
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
Refactor .ownProperty
to leverage .property
#795
Comments
I totally agree! Awesome idea! Doing this would make the code more modular and, as you said, would cause these two assertions to never diverge again since they basically do the same thing but have different assertion subjects. I just think we need to have a good and self explanatory error message when Great suggestion 👍 |
@lucasfcosta It really seems to me that using I see two options:
If we choose the second option I have a suggestion:
Yes, this is totally stolen from here |
I'm strongly in favor of throwing an error when I should have a chance to work on this tomorrow! |
There's a divergence between the behavior of the
.property
and.ownProperty
assertions due to refactoring of.property
in the master branch and.ownProperty
in the 4.x.x branch.I think the thing to do here is:
own
flag..property
to restrict checks to own properties when theown
flag is set..ownProperty
and.haveOwnProperty
into simple assertions that set theown
flag and then call.property
to do the rest.That way, the code paths for
.property
and.ownProperty
are unified and will never diverge again. It also enables chaining with.deep
. For example:The only breaking change to
.ownProperty
would be the same breaking change that happened to.property
during the refactor in #744.However, I don't think the
nested
andown
flags mix conceptually, since the intent ofnested
is to test a property that's multiple levels deep, whereas the purpose ofown
is to test a property that is set directly on an object as opposed to being inherited. Therefore, combining these two flags would cause.property
to throw an error.I already started playing around with this on the plane. I'll go ahead and finish it if there's consensus.
@lucasfcosta @keithamus?
The text was updated successfully, but these errors were encountered: