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: implement setters, descriptors and more Proxy traps #684

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

ZauberNerd
Copy link
Contributor

The implementation of #622 only implemented a get Proxy trap. This worked fine for the simple use-case of invoking the methods, but failed when users tried to mock functions with Jest or Sinon. It also did not list all functions anymore, when pressing tab-tab in a REPL.

This commit implements further Proxy traps which are required for those use-cases and it uses the cache object for mutating operations.

Fixes: #683

Resolves #683


Before the change?

After the change?

  • Added necessary Proxy traps improving Jest, Sinon mocking, and REPL function listing capabilities

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

The implementation of octokit#622 only implemented a `get` Proxy trap.
This worked fine for the simple use-case of invoking the methods, but
failed when users tried to mock functions with Jest or Sinon.
It also did not list all functions anymore, when pressing tab-tab in a
REPL.

This commit implements further Proxy traps which are required for those
use-cases and it uses the cache object for mutating operations.

Fixes: octokit#683
@ghost ghost added this to Inbox in JS Oct 11, 2023
@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented label Oct 11, 2023
@ghost ghost moved this from Inbox to Bugs in JS Oct 11, 2023
@wolfy1339 wolfy1339 merged commit a1dcf83 into octokit:main Oct 11, 2023
8 of 9 checks passed
JS automation moved this from Bugs to Done Oct 11, 2023
@github-actions
Copy link
Contributor

🎉 This PR is included in version 10.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ZauberNerd ZauberNerd deleted the proxy-configurability branch October 11, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Type: Bug Something isn't working as documented
Projects
Archived in project
JS
  
Done
Development

Successfully merging this pull request may close these issues.

[BUG]: Upgrading from @actions/github V5 -> V6 caused TypeErrors within getOcktokit()
2 participants