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

Add the parent expect to the prototype chain of a child expect #785

Merged
merged 3 commits into from
Apr 26, 2022

Conversation

papandreou
Copy link
Member

Extracted from #784 because it seems like a reasonable idea no matter what.

Strictly speaking this is semver-major, but I very much doubt that anyone is relying on it.

@papandreou papandreou self-assigned this Jan 4, 2021
@papandreou
Copy link
Member Author

papandreou commented Sep 9, 2021

@sunesimonsen @alexjeffburke, any objections to landing this in a minor given that it doesn't break anything, including the 🐦 build?

@alexjeffburke
Copy link
Member

@papandreou would you be able to drop a refresher on what the effect of this is? I vaguely remember the change but have no memory of what it achieves lol.

@papandreou
Copy link
Member Author

papandreou commented Sep 9, 2021

The effect is that the expect that we hand into assertions will now have the "top level" expect in its prototype chain, so it's effectively a subclass of it. Before it was detached from it, but was still calling into it in a bunch of ways. If we make this change, it'll allow us to share more functionality between the two by just calling through to the method/property in the top level expect without installing a specific method on the child that closes over the top level expect.

This seems like a sane direction to go in no matter what, but IIRC the specific occasion was that it would be easier to make the expect.it camel case equivalent work on child expects if that's the case, eg.

expect.addAssertion('<object> to have foo property that bars', (expect, subject, value) => {
  expect(object, 'to satisfy', {
    foo: expect.toBar()
  });
});

... where the alternative would be to attach every expanded camelCase assertion to child expects also, which would be expensive.

@papandreou
Copy link
Member Author

@alexjeffburke, did that make sense? :)

@papandreou papandreou force-pushed the feature/childInheritsFromParentExpect branch from fd6231b to 23105f0 Compare April 25, 2022 19:42
... so that the expect.it equivalent of subsequently added assertions are
available as childExpect.toWhatever

(cherry picked from commit 194ac56)
@papandreou papandreou force-pushed the feature/childInheritsFromParentExpect branch from 23105f0 to 16e4b28 Compare April 25, 2022 19:46
Copy link
Member

@sunesimonsen sunesimonsen left a comment

Choose a reason for hiding this comment

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

Other than my comment is looks fine :-)

@papandreou papandreou merged commit 7299a38 into master Apr 26, 2022
@papandreou papandreou deleted the feature/childInheritsFromParentExpect branch April 26, 2022 18:45
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.

3 participants