Address review comments.

Author: rosalyntan

package-lock.json

@@ -505,7 +505,6 @@
     "@types/express": "^4.17.21",
     "cors": "^2.8.5",
     "express": "^4.21.0",
-    "graphql": "^16.12.0",
     "protobufjs": "^7.2.2"
   },
   "devDependencies": {
@@ -537,6 +536,7 @@
     "eslint-plugin-prettier": "^4.2.1",
     "firebase-admin": "^13.0.0",
     "genkit": "^1.0.0-rc.4",
+    "graphql": "^16.12.0",
     "jsdom": "^16.2.1",
     "jsonwebtoken": "^9.0.0",
     "jwk-to-pem": "^2.0.5",

package.json

@@ -618,7 +618,7 @@ describe("dataconnect", () => {
       const func = dataconnect.onGraphRequest(opts);
       expect(func.__endpoint).to.deep.equal({
         ...expectedEndpointBase,
-        dataConnectHttpsTrigger: {},
+        dataConnectGraphqlTrigger: {},
       });
     });
     it("returns callable function with request opts with Data Connect trigger", () => {
@@ -636,7 +636,7 @@ describe("dataconnect", () => {
       const func = dataconnect.onGraphRequest(opts);
       expect(func.__endpoint).to.deep.equal({
         ...expectedEndpointBase,
-        dataConnectHttpsTrigger: {
+        dataConnectGraphqlTrigger: {
           invoker: ["[email protected]"],
         },
         region: ["us-east4"],

spec/v2/providers/dataconnect.spec.ts

@@ -71,7 +71,7 @@ export interface ManifestEndpoint {
     genkitAction?: string;
   };
 
-  dataConnectHttpsTrigger?: {
+  dataConnectGraphqlTrigger?: {
     invoker?: string[];
   };
 

src/runtime/manifest.ts

@@ -20,10 +20,9 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-import cors from "cors";
 import express from "express";
 import fs from "fs";
-import { GraphQLResolveInfo } from "graphql";
+import type { GraphQLResolveInfo } from "graphql";
 import { HttpsFunction, HttpsOptions } from "./https";
 import { CloudEvent, CloudFunction } from "../core";
 import { ParamsOf, VarName } from "../../common/params";
@@ -42,7 +41,6 @@ import { Expression } from "../../params";
 import * as options from "../options";
 import { ResetValue } from "../../common/options";
 import { withErrorHandler, Request } from "../../common/providers/https";
-import { isDebugFeatureEnabled } from "../../common/debug";
 import { convertIfPresent, convertInvoker } from "../../common/encoding";
 
 /** @internal */
@@ -389,7 +387,7 @@ export async function initGraphqlServer(opts: GraphqlServerOptions): Promise<exp
   if (!opts.resolvers.query && !opts.resolvers.mutation) {
     throw new Error("At least one query or mutation resolver must be provided.");
   }
-  const apolloResolvers: { [key: string]: any } = {};
+  const apolloResolvers: Record<string, any> = {};
   if (opts.resolvers.query) {
     apolloResolvers.Query = opts.resolvers.query;
   }
@@ -406,7 +404,7 @@ export async function initGraphqlServer(opts: GraphqlServerOptions): Promise<exp
         resolvers: apolloResolvers,
       });
       await server.start();
-      app.use(`/${opts.path ?? "graphql"}`, cors(), express.json(), expressMiddleware(server));
+      app.use(`/${opts.path ?? "graphql"}`, express.json(), expressMiddleware(server));
       return app;
     })();
     return serverPromise;
@@ -427,41 +425,16 @@ let serverPromise: Promise<express.Express> | null = null;
  * @returns {HttpsFunction} A function you can export and deploy.
  */
 export function onGraphRequest(opts: GraphqlServerOptions): HttpsFunction {
-  let handler: (req: Request, res: express.Response) => void | Promise<void> = withErrorHandler(
-    async (req: Request, res: express.Response) => {
-      serverPromise = serverPromise ?? initGraphqlServer(opts);
-      const app = await serverPromise;
-      app(req, res);
-    }
+  const handler: (req: Request, res: express.Response) => void | Promise<void> = wrapTraceContext(
+    withInit(
+      withErrorHandler(async (req: Request, res: express.Response) => {
+        serverPromise = serverPromise ?? initGraphqlServer(opts);
+        const app = await serverPromise;
+        app(req, res);
+      })
+    )
   );
 
-  if (isDebugFeatureEnabled("enableCors") || "cors" in opts) {
-    let origin = opts.cors instanceof Expression ? opts.cors.value() : opts.cors;
-    if (isDebugFeatureEnabled("enableCors")) {
-      // Respect `cors: false` to turn off cors even if debug feature is enabled.
-      origin = opts.cors === false ? false : true;
-    }
-    // Arrays cause the access-control-allow-origin header to be dynamic based
-    // on the origin header of the request. If there is only one element in the
-    // array, this is unnecessary.
-    if (Array.isArray(origin) && origin.length === 1) {
-      origin = origin[0];
-    }
-    const middleware = cors({ origin });
-
-    const userProvidedHandler = handler;
-    handler = (req: Request, res: express.Response): void | Promise<void> => {
-      return new Promise((resolve) => {
-        res.on("finish", resolve);
-        middleware(req, res, () => {
-          resolve(userProvidedHandler(req, res));
-        });
-      });
-    };
-  }
-
-  handler = wrapTraceContext(withInit(handler));
-
   const globalOpts = options.getGlobalOptions();
   const baseOpts = options.optionsToEndpoint(globalOpts);
   // global options calls region a scalar and https allows it to be an array,
@@ -476,16 +449,16 @@ export function onGraphRequest(opts: GraphqlServerOptions): HttpsFunction {
       ...baseOpts?.labels,
       ...specificOpts?.labels,
     },
-    dataConnectHttpsTrigger: {},
+    dataConnectGraphqlTrigger: {},
   };
   convertIfPresent(
-    endpoint.dataConnectHttpsTrigger,
+    endpoint.dataConnectGraphqlTrigger,
     globalOpts,
     "invoker",
     "invoker",
     convertInvoker
   );
-  convertIfPresent(endpoint.dataConnectHttpsTrigger, opts, "invoker", "invoker", convertInvoker);
+  convertIfPresent(endpoint.dataConnectGraphqlTrigger, opts, "invoker", "invoker", convertInvoker);
   (handler as HttpsFunction).__endpoint = endpoint;
 
   return handler as HttpsFunction;