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

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

Closed
1 task done
Zidious opened this issue Oct 10, 2023 · 6 comments · Fixed by #684
Closed
1 task done
Labels
released Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented

Comments

@Zidious
Copy link

Zidious commented Oct 10, 2023

What happened?

Version 5 of @actions/github, I was able to do this within my tests:

import { getOctokit } from '@actions/github'
import type { Endpoints } from '@octokit/types'
import sinon from 'sinon'

const octokit = getOctokit('token')

const stubLabelList = () => {
	return sinon.stub(octokit.rest.issues, 'listLabelsForRepo').resolves({
		data: [MOCK_LIST_LABELS],
		status: 200
		} as Endpoints["GET /repos/{owner}/{repo}/labels"]["response"])
	}
}

const github = {
	getOctokit: () => octokit
}

// pass github  and stubLabelList to other test functions etc 

As of version 6, I've had to do something like:

import { getOctokit } from '@actions/github'
import type { Endpoints } from '@octokit/types'
import sinon from 'sinon'

const octokit = getOctokit('token')

const github = {
	getOctokit: () => {
		...octokit, 
		rest: { 
			issues: {
				listLabelsForRepo: () => {
					return {
						data: [MOCK_LIST_LABELS],
						status: 200
					} as Endpoints["GET /repos/{owner}/{repo}/labels"]["response"])
				}
			}
		} 
	}
}

I noticed this was a change in way the endpoints are generated and is now done via a proxy as of #622, could this of introduced unwanted side effects or is this the intended behaviour?

Versions

@actions/github: v6
NodeJS: 18

Relevant log output

TypeError: Cannot destructure property ‘decorations’ of ‘endpointMethodsMap.get(...).get(...)’ as it is undefined.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Zidious Zidious added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Oct 10, 2023
@github-actions
Copy link
Contributor

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@wolfy1339
Copy link
Member

I'm not entirely sure this is a problem with Octokit, can you open an issue with @actions/github as well

@Zidious
Copy link
Author

Zidious commented Oct 10, 2023

I'm not entirely sure this is a problem with Octokit, can you open an issue with @actions/github as well

actions/toolkit#1555

@wolfy1339
Copy link
Member

@ZauberNerd Can you help us out here since you implemented the Proxy

ZauberNerd added a commit to ZauberNerd/plugin-rest-endpoint-methods.js that referenced this issue Oct 11, 2023
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
@ZauberNerd
Copy link
Contributor

@ZauberNerd Can you help us out here since you implemented the Proxy

I've submitted a PR to address this issue: #684

wolfy1339 pushed a commit that referenced this issue Oct 11, 2023
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
@github-project-automation github-project-automation bot moved this from 🆕 Triage to ✅ Done in 🧰 Octokit Active Oct 11, 2023
@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 10.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants