fix: tighten up V2 API detection mechanism (#1530)

src/runtimes/node/in_source_config/index.ts

@@ -97,7 +97,14 @@ export const findISCDeclarations = (
   }
 
   const iscExports = handlerExports
-    .map(({ args, local: exportName }) => {
+    .map((node) => {
+      // We're only interested in exports with call expressions, since that's
+      // the pattern we use for the wrapper functions.
+      if (node.type !== 'call-expression') {
+        return null
+      }
+
+      const { args, local: exportName } = node
       const matchingImport = imports.find(({ local: importName }) => importName === exportName)
 
       if (matchingImport === undefined) {
@@ -141,7 +148,10 @@ export const findISCDeclarations = (
 
 export type ISCHandlerArg = ArgumentPlaceholder | Expression | SpreadElement | JSXNamespacedName
 
-export interface ISCExport {
-  local: string
+export type ISCExportWithCallExpression = {
+  type: 'call-expression'
   args: ISCHandlerArg[]
+  local: string
 }
+export type ISCExportOther = { type: 'other' }
+export type ISCExport = ISCExportWithCallExpression | ISCExportOther

src/runtimes/node/parser/exports.ts

@@ -182,20 +182,24 @@ const getExportsFromBindings = (specifiers: ExportNamedDeclaration['specifiers']
   return exports
 }
 
-const getExportsFromExpression = (node: Expression | undefined | null) => {
-  // We're only interested in expressions representing function calls, because
-  // the ISC patterns we implement at the moment are all helper functions.
-  if (node?.type !== 'CallExpression') {
-    return []
-  }
+const getExportsFromExpression = (node: Expression | undefined | null): ISCExport[] => {
+  switch (node?.type) {
+    case 'CallExpression': {
+      const { arguments: args, callee } = node
 
-  const { arguments: args, callee } = node
+      if (callee.type !== 'Identifier') {
+        return []
+      }
 
-  if (callee.type !== 'Identifier') {
-    return []
-  }
+      return [{ args, local: callee.name, type: 'call-expression' }]
+    }
 
-  const exports = [{ local: callee.name, args }]
+    default: {
+      if (node !== undefined) {
+        return [{ type: 'other' }]
+      }
 
-  return exports
+      return []
+    }
+  }
 }

tests/unit/runtimes/node/in_source_config.test.ts

@@ -36,6 +36,18 @@ describe('`schedule` helper', () => {
     expect(isc).toEqual({ runtimeAPIVersion: 1 })
   })
 
+  test.todo('CommonJS file with `schedule` helper exported from a variable', () => {
+    const source = `const { schedule } = require("@netlify/functions")
+
+    const handler = schedule("@daily", () => {})
+
+    exports.handler = handler`
+
+    const isc = findISCDeclarations(source, options)
+
+    expect(isc).toEqual({ schedule: '@daily', runtimeAPIVersion: 1 })
+  })
+
   test('ESM file with `schedule` helper', () => {
     const source = `import { schedule } from "@netlify/functions"
 
@@ -65,6 +77,18 @@ describe('`schedule` helper', () => {
 
     expect(isc).toEqual({ runtimeAPIVersion: 1 })
   })
+
+  test('ESM file with `handler` exported from a variable', () => {
+    const source = `import { schedule } from "@netlify/functions"
+
+    const handler = schedule("@daily", () => {})
+    
+    export { handler }`
+
+    const isc = findISCDeclarations(source, options)
+
+    expect(isc).toEqual({ schedule: '@daily', runtimeAPIVersion: 1 })
+  })
 })
 
 describe('`stream` helper', () => {
@@ -100,76 +124,90 @@ describe('V2 API', () => {
     logger: getLogger(),
   }
 
-  test('ESM file with a default export and no `handler` export', () => {
-    const source = `export default async () => {
-      return new Response("Hello!")
-    }`
+  describe('Detects the correct runtime API version', () => {
+    test('ESM file with a default export and no `handler` export', () => {
+      const source = `export default async () => {
+        return new Response("Hello!")
+      }`
 
-    const systemLog = vi.fn()
+      const systemLog = vi.fn()
 
-    const isc = findISCDeclarations(source, { ...options, logger: getLogger(systemLog) })
+      const isc = findISCDeclarations(source, { ...options, logger: getLogger(systemLog) })
 
-    expect(systemLog).toHaveBeenCalledOnce()
-    expect(systemLog).toHaveBeenCalledWith('detected v2 function')
-    expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2 })
-  })
+      expect(systemLog).toHaveBeenCalledOnce()
+      expect(systemLog).toHaveBeenCalledWith('detected v2 function')
+      expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2 })
+    })
 
-  test('ESM file with a default export and a `handler` export', () => {
-    const source = `export default async () => {
-      return new Response("Hello!")
-    }
+    test('ESM file with a default export and a `handler` export', () => {
+      const source = `export default async () => {
+        return new Response("Hello!")
+      }
+  
+      export const handler = function () { return { statusCode: 200, body: "Hello!" } }`
 
-    export const handler = async () => ({ statusCode: 200, body: "Hello!" })`
+      const isc = findISCDeclarations(source, options)
 
-    const isc = findISCDeclarations(source, options)
+      expect(isc).toEqual({ runtimeAPIVersion: 1 })
+    })
 
-    expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2 })
-  })
+    test('ESM file with no default export and a `handler` export', () => {
+      const source = `const handler = async () => ({ statusCode: 200, body: "Hello" })
+      
+      export { handler }`
 
-  test('TypeScript file with a default export and no `handler` export', () => {
-    const source = `export default async (req: Request) => {
-      return new Response("Hello!")
-    }`
+      const isc = findISCDeclarations(source, options)
 
-    const isc = findISCDeclarations(source, options)
+      expect(isc).toEqual({ runtimeAPIVersion: 1 })
+    })
 
-    expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2 })
-  })
+    test('TypeScript file with a default export and no `handler` export', () => {
+      const source = `export default async (req: Request) => {
+        return new Response("Hello!")
+      }`
 
-  test('CommonJS file with a default export and a `handler` export', () => {
-    const source = `exports.default = async () => {
-      return new Response("Hello!")
-    }
+      const isc = findISCDeclarations(source, options)
 
-    exports.handler = async () => ({ statusCode: 200, body: "Hello!" })`
+      expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2 })
+    })
 
-    const isc = findISCDeclarations(source, options)
+    test('CommonJS file with a default export and a `handler` export', () => {
+      const source = `exports.default = async () => {
+        return new Response("Hello!")
+      }
+  
+      exports.handler = async () => ({ statusCode: 200, body: "Hello!" })`
 
-    expect(isc).toEqual({ runtimeAPIVersion: 1 })
-  })
+      const isc = findISCDeclarations(source, options)
 
-  test('CommonJS file with a default export and no `handler` export', () => {
-    const source = `exports.default = async () => {
-      return new Response("Hello!")
-    }`
+      expect(isc).toEqual({ runtimeAPIVersion: 1 })
+    })
 
-    const isc = findISCDeclarations(source, options)
+    test('CommonJS file with a default export and no `handler` export', () => {
+      const source = `exports.default = async () => {
+        return new Response("Hello!")
+      }`
 
-    expect(isc).toEqual({ runtimeAPIVersion: 1 })
-  })
+      const isc = findISCDeclarations(source, options)
 
-  test('Config object with `schedule` property', () => {
-    const source = `export default async () => {
-      return new Response("Hello!")
-    }
+      expect(isc).toEqual({ runtimeAPIVersion: 1 })
+    })
+  })
 
-    export const config = {
-      schedule: "@daily"
-    }`
+  describe('`scheduled` property', () => {
+    test('Using a cron expression string', () => {
+      const source = `export default async () => {
+        return new Response("Hello!")
+      }
+  
+      export const config = {
+        schedule: "@daily"
+      }`
 
-    const isc = findISCDeclarations(source, options)
+      const isc = findISCDeclarations(source, options)
 
-    expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2, schedule: '@daily' })
+      expect(isc).toEqual({ routes: [], runtimeAPIVersion: 2, schedule: '@daily' })
+    })
   })
 
   describe('`path` property', () => {