fix: implement setters, descriptors and more Proxy traps (#684)

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

Author: ZauberNerd

package-lock.json

@@ -32,6 +32,7 @@
     "@types/fetch-mock": "^7.3.1",
     "@types/jest": "^29.0.0",
     "@types/node": "^18.0.0",
+    "@types/sinon": "^10.0.19",
     "esbuild": "^0.19.0",
     "fetch-mock": "npm:@gr2m/fetch-mock@^9.11.0-pull-request-644.1",
     "github-openapi-graphql-query": "^4.0.0",
@@ -44,6 +45,7 @@
     "npm-run-all": "^4.1.5",
     "prettier": "3.0.3",
     "semantic-release-plugin-update-version-in-files": "^1.0.0",
+    "sinon": "^16.1.0",
     "sort-keys": "^5.0.0",
     "string-to-jsdoc-comment": "^1.0.0",
     "ts-jest": "^29.0.0",

package.json

@@ -48,14 +48,46 @@ type ProxyTarget = {
 };
 
 const handler = {
+  has({ scope }: ProxyTarget, methodName: string) {
+    return endpointMethodsMap.get(scope).has(methodName);
+  },
+  getOwnPropertyDescriptor(target: ProxyTarget, methodName: string) {
+    return {
+      value: this.get(target, methodName), // ensures method is in the cache
+      configurable: true,
+      writable: true,
+      enumerable: true,
+    };
+  },
+  defineProperty(
+    target: ProxyTarget,
+    methodName: string,
+    descriptor: PropertyDescriptor,
+  ) {
+    Object.defineProperty(target.cache, methodName, descriptor);
+    return true;
+  },
+  deleteProperty(target: ProxyTarget, methodName: string) {
+    delete target.cache[methodName];
+    return true;
+  },
+  ownKeys({ scope }: ProxyTarget) {
+    return [...endpointMethodsMap.get(scope).keys()];
+  },
+  set(target: ProxyTarget, methodName: string, value: any) {
+    return (target.cache[methodName] = value);
+  },
   get({ octokit, scope, cache }: ProxyTarget, methodName: string) {
     if (cache[methodName]) {
       return cache[methodName];
     }
 
-    const { decorations, endpointDefaults } = endpointMethodsMap
-      .get(scope)
-      .get(methodName);
+    const method = endpointMethodsMap.get(scope).get(methodName);
+    if (!method) {
+      return undefined;
+    }
+
+    const { endpointDefaults, decorations } = method;
 
     if (decorations) {
       cache[methodName] = decorate(

src/endpoints-to-methods.ts

@@ -1,7 +1,9 @@
-import fetchMock from "fetch-mock";
 import { Octokit } from "@octokit/core";
+import fetchMock from "fetch-mock";
 
-import { restEndpointMethods, legacyRestEndpointMethods } from "../src";
+import sinon from "sinon";
+import { legacyRestEndpointMethods, restEndpointMethods } from "../src";
+import { Api } from "../src/types";
 
 describe("REST API endpoint methods", () => {
   it("README example", async () => {
@@ -175,6 +177,100 @@ describe("REST API endpoint methods", () => {
     return octokit.rest.apps.listInstallations();
   });
 
+  describe("mocking", () => {
+    let octokit: Octokit & Api;
+
+    beforeEach(() => {
+      const networkMock = fetchMock
+        .sandbox()
+        .getOnce(
+          "https://api.github.com/repos/octokit/plugin-rest-endpoint-methods/issues/1/labels",
+          [{ name: "mocked from network" }],
+        );
+
+      const MyOctokit = Octokit.plugin(restEndpointMethods);
+      octokit = new MyOctokit({
+        request: {
+          fetch: networkMock,
+        },
+      });
+    });
+
+    afterEach(async () => {
+      const restoredResult = await octokit.rest.issues.listLabelsOnIssue({
+        owner: "octokit",
+        repo: "plugin-rest-endpoint-methods",
+        issue_number: 1,
+      });
+      expect(restoredResult.data[0].name).toBe("mocked from network");
+    });
+
+    it("allows mocking with sinon.stub", async () => {
+      const stub = sinon
+        .stub(octokit.rest.issues, "listLabelsOnIssue")
+        .resolves({ data: [{ name: "mocked from sinon" }] } as Awaited<
+          ReturnType<typeof octokit.rest.issues.listLabelsOnIssue>
+        >);
+
+      const sinonResult = await octokit.rest.issues.listLabelsOnIssue({
+        owner: "octokit",
+        repo: "plugin-rest-endpoint-methods",
+        issue_number: 1,
+      });
+      expect(sinonResult.data[0].name).toBe("mocked from sinon");
+
+      stub.restore();
+    });
+
+    it("allows mocking with jest.spyOn", async () => {
+      jest
+        .spyOn(octokit.rest.issues, "listLabelsOnIssue")
+        .mockResolvedValueOnce({
+          data: [{ name: "mocked from jest" }],
+        } as Awaited<ReturnType<typeof octokit.rest.issues.listLabelsOnIssue>>);
+
+      const jestResult = await octokit.rest.issues.listLabelsOnIssue({
+        owner: "octokit",
+        repo: "plugin-rest-endpoint-methods",
+        issue_number: 1,
+      });
+      expect(jestResult.data[0].name).toBe("mocked from jest");
+
+      jest.restoreAllMocks();
+    });
+
+    it("allows manually replacing a method", async () => {
+      const oldImplementation = octokit.rest.issues.listLabelsOnIssue;
+
+      octokit.rest.issues.listLabelsOnIssue = (async () => {
+        return {
+          data: [{ name: "mocked from custom implementation" }],
+        } as Awaited<ReturnType<typeof octokit.rest.issues.listLabelsOnIssue>>;
+      }) as unknown as typeof oldImplementation;
+
+      const customResult = await octokit.rest.issues.listLabelsOnIssue({
+        owner: "octokit",
+        repo: "plugin-rest-endpoint-methods",
+        issue_number: 1,
+      });
+      expect(customResult.data[0].name).toBe(
+        "mocked from custom implementation",
+      );
+
+      delete (octokit.rest.issues as any).listLabelsOnIssue;
+      octokit.rest.issues.listLabelsOnIssue = oldImplementation;
+    });
+  });
+
+  it("lists all methods (e.g. with tab-tab in a REPL)", async () => {
+    const MyOctokit = Octokit.plugin(restEndpointMethods);
+    const octokit = new MyOctokit();
+
+    const methods = Object.keys(octokit.rest.issues);
+
+    expect(methods).toContain("listLabelsOnIssue");
+  });
+
   // besides setting `octokit.rest.*`, the plugin exports legacyRestEndpointMethods
   // which is also setting the same methods directly on `octokit.*` for legacy reasons.
   // We will deprecate the `octokit.*` methods in future, but for now we just make sure they are set