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: not to break on expect matcher extension overwrite #11978

Merged
merged 4 commits into from Oct 19, 2021

Conversation

blaky
Copy link
Contributor

@blaky blaky commented Oct 18, 2021

Summary

Before #11949 it was possible to overwrite expect extensions. With this change I recover that functionality by adding configurable: true to the Object.defineProperty call which adds the extensions to the expect object.

Please find more the details about the issue and a reproduction repo at #11977

Test plan

I added a unit test which ensure that this issue doesn't get reintroduced again to jest.

@blaky blaky changed the title fix: not to break on expect extension override fix: not to break on expect matcher extension overwrite Oct 18, 2021
@mrazauskas
Copy link
Contributor

@mrazauskas mrazauskas commented Oct 19, 2021

Looks good. I am the one who broke this, so... thanks for fixing!

@SimenB Would it make sense to add enumerable: true and writable: true as well? If I get it right, this was the previous behaviour and the refactoring to Object.defineProperty broke all of these (not just configurable).

@SimenB SimenB linked an issue Oct 19, 2021 that may be closed by this pull request
@SimenB
Copy link
Collaborator

@SimenB SimenB commented Oct 19, 2021

Good call @mrazauskas, added that 🙂

SimenB
SimenB approved these changes Oct 19, 2021
Copy link
Collaborator

@SimenB SimenB left a comment

Thanks @blaky!

@SimenB SimenB changed the title fix: not to break on expect matcher extension overwrite fix: not to break on expect matcher extension overwrite Oct 19, 2021
@SimenB SimenB merged commit 2e2b17a into facebook:main Oct 19, 2021
29 of 30 checks passed
@blaky blaky deleted the expect-extension-override-fix branch Oct 19, 2021
@blaky
Copy link
Contributor Author

@blaky blaky commented Oct 19, 2021

@SimenB thank you for the improvements and merging this PR. Do you have any ETA on when this could be released?

@SimenB
Copy link
Collaborator

@SimenB SimenB commented Oct 19, 2021

@blaky
Copy link
Contributor Author

@blaky blaky commented Oct 19, 2021

Thanks for the quick response 👍🏻 😆

@github-actions
Copy link

@github-actions github-actions bot commented Nov 19, 2021

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants