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]: No way to add new methods to octokit.rest.repos in v7 #668

Closed
1 task done
mheap opened this issue Aug 12, 2023 · 6 comments
Closed
1 task done

[BUG]: No way to add new methods to octokit.rest.repos in v7 #668

mheap opened this issue Aug 12, 2023 · 6 comments
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs

Comments

@mheap
Copy link

mheap commented Aug 12, 2023

What happened?

I maintain octokit-commit-multiple-files, which has stopped working with plugin-rest-endpoint-methods.js 7.0. (Bug report: mheap/octokit-commit-multiple-files#110)

It seems to be related to the refactoring in #622

I previously attached a new method to octokit.rest.repos like so:

module.exports = function (octokit) {
  octokit.rest.repos.createOrUpdateFiles = plugin.bind(null, octokit);
};

Is there a way to register new methods after this refactoring?

Versions

@octokit/rest 19.0.7
Node.js v18.15.0

Relevant log output

/private/tmp/octokit-test/node_modules/@octokit/plugin-rest-endpoint-methods/dist-node/index.js:1929
    const { decorations, endpointDefaults } = endpointMethodsMap.get(scope).get(methodName);
            ^

TypeError: Cannot destructure property 'decorations' of 'endpointMethodsMap.get(...).get(...)' as it is undefined.
    at Object.get (/private/tmp/octokit-test/node_modules/@octokit/plugin-rest-endpoint-methods/dist-node/index.js:1929:13)
    at /private/tmp/octokit-test/index.js:14:44
    at Object.<anonymous> (/private/tmp/octokit-test/index.js:32:3)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.15.0

Code of Conduct

  • I agree to follow this project's Code of Conduct
@mheap mheap added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Aug 12, 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! 🚀

@gr2m
Copy link
Contributor

gr2m commented Aug 16, 2023

the correct way to define methods is to export an object where the keys are the methods that should be added to octokit:
https://github.com/octokit/core.js/#plugins

You cannot rely on octokit.rest.repos. to exist, it would make your plugin incompatible with @octokit/core

@mheap
Copy link
Author

mheap commented Aug 17, 2023

If I want to add a method to octokit.rest.repos explicitly, how can I do so?

module.exports = function (octokit) {
  return {
    rest: {
      repos: {
        ...octokit.rest.repos,
        createOrUpdateFiles: plugin.bind(null, octokit),
      },
    },
  };
};

This has the same error:

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

       5 |     rest: {
       6 |       repos: {
    >  7 |         ...octokit.rest.repos,
         |                         ^
       8 |         createOrUpdateFiles: plugin.bind(null, octokit),
       9 |       },
      10 |     },

Or is the recommendation not to attach at that point, and to make octokit.createOrUpdateFiles the method to call?

@gr2m
Copy link
Contributor

gr2m commented Aug 17, 2023

If I want to add a method to octokit.rest.repos explicitly, how can I do so?

I'm sorry but the best answer I have is you shouldn't, as I explained above. Plugins should declare top level methods, in your case it would be octokit.createOrUpdateFiles(). The octokit.rest.* methods have other APIs that yours doesn't have, like octokit.rest.repos.get.endpoint(). Please avoid mingling with the .rest namespace

@nickfloyd nickfloyd added Type: Support Any questions, information, or general needs around the SDK or GitHub APIs and removed Status: Triage This is being looked at and prioritized labels Aug 18, 2023
@nickfloyd nickfloyd moved this from 🆕 Triage to 🛑 Blocked/Awaiting Response in 🧰 Octokit Active Aug 18, 2023
@nickfloyd nickfloyd removed the Type: Bug Something isn't working as documented label Aug 18, 2023
@mheap
Copy link
Author

mheap commented Aug 19, 2023

Thanks for the info. I've moved the method to the object root in the plugin

@mheap mheap closed this as completed Aug 19, 2023
@github-project-automation github-project-automation bot moved this from 🛑 Blocked/Awaiting Response to ✅ Done in 🧰 Octokit Active Aug 19, 2023
@gr2m
Copy link
Contributor

gr2m commented Aug 19, 2023

Thank you! I'll make sure to add note to plugin docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs
Projects
Archived in project
Development

No branches or pull requests

3 participants