Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ExtendedRoute, Route, getRoutes } from '../../../utils/routes.js'
import { RUNTIME } from '../../runtime.js'
import type { BindingMethod } from '../parser/bindings.js'
import { createBindingsMethod } from '../parser/bindings.js'
import { traverseNodes } from '../parser/exports.js'
import { parseObject, traverseNodes } from '../parser/exports.js'
import { getImports } from '../parser/imports.js'
import { safelyParseSource, safelyReadSource } from '../parser/index.js'
import type { ModuleFormat } from '../utils/module_format.js'
Expand Down Expand Up @@ -87,25 +87,38 @@ export const inSourceConfig = functionConfig
export type InSourceConfig = z.infer<typeof inSourceConfig>

/**
* Extracts event subscription slugs from the default export expression,
* if it's an object whose property names match known event handlers.
* Resolves the default export expression to an ObjectExpression if possible,
* following identifier bindings when needed.
*/
const getEventSubscriptions = (
const resolveObjectExpression = (
expression: Expression | Declaration | undefined,
getAllBindings: BindingMethod,
): string[] => {
let objectExpression: ObjectExpression | undefined

): ObjectExpression | undefined => {
if (expression?.type === 'ObjectExpression') {
objectExpression = expression
} else if (expression?.type === 'Identifier') {
return expression
}

if (expression?.type === 'Identifier') {
const binding = getAllBindings().get(expression.name)

if (binding?.type === 'ObjectExpression') {
objectExpression = binding
return binding
}
}

return undefined
}

/**
* Extracts event subscription slugs from the default export expression,
* if it's an object whose property names match known event handlers.
*/
const getEventSubscriptions = (
expression: Expression | Declaration | undefined,
getAllBindings: BindingMethod,
): string[] => {
const objectExpression = resolveObjectExpression(expression, getAllBindings)

if (!objectExpression) {
return []
}
Expand All @@ -130,6 +143,41 @@ const getEventSubscriptions = (
return events
}

/**
* Extracts a `config` property from the default export object expression,
* returning it as a plain object. This supports patterns like:
*
* ```js
* export default {
* fetch() { ... },
* config: { path: "/hello" }
* }
* ```
*/
const getConfigFromDefaultExport = (
expression: Expression | Declaration | undefined,
getAllBindings: BindingMethod,
): Record<string, unknown> | undefined => {
const objectExpression = resolveObjectExpression(expression, getAllBindings)

if (!objectExpression) {
return undefined
}

for (const property of objectExpression.properties) {
if (
property.type === 'ObjectProperty' &&
property.key.type === 'Identifier' &&
property.key.name === 'config' &&
property.value.type === 'ObjectExpression'
) {
Comment on lines +167 to +173
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

"config" string-literal keys are currently ignored.

getConfigFromDefaultExport(...) only matches config when the key is an Identifier. It should also accept StringLiteral keys to handle export default { "config": { ... } }.

💡 Proposed fix
   for (const property of objectExpression.properties) {
     if (
       property.type === 'ObjectProperty' &&
-      property.key.type === 'Identifier' &&
-      property.key.name === 'config' &&
+      ((property.key.type === 'Identifier' && property.key.name === 'config') ||
+        (property.key.type === 'StringLiteral' && property.key.value === 'config')) &&
       property.value.type === 'ObjectExpression'
     ) {
       return parseObject(property.value)
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts`
around lines 167 - 173, getConfigFromDefaultExport currently only recognizes a
config key when property.key is an Identifier, so objectLiteral forms like
export default { "config": {...} } are ignored; update the loop that inspects
objectExpression.properties to also accept property.key.type === 'StringLiteral'
and check the key value via property.key.value (in addition to
property.key.name) when matching 'config' so both Identifier and StringLiteral
keys are handled for the config property.

return parseObject(property.value)
}
}

return undefined
}

const validateScheduleFunction = (functionFound: boolean, scheduleFound: boolean, functionName: string): void => {
if (!functionFound) {
throw new FunctionBundlingUserError(
Expand Down Expand Up @@ -205,7 +253,11 @@ export const parseSource = (source: string, { functionName }: FindISCDeclaration
result.eventSubscriptions = eventSubscriptions
}

const { data, error, success } = inSourceConfig.safeParse(configExport)
// Config from the default export object's `config` property is used as a
// fallback when no separate `export const config` exists.
const inlineConfig = getConfigFromDefaultExport(defaultExportExpression, getAllBindings)
const mergedConfigExport = Object.keys(configExport).length > 0 ? configExport : (inlineConfig ?? {})
const { data, error, success } = inSourceConfig.safeParse(mergedConfigExport)
Comment on lines +256 to +260
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Explicit named config loses precedence when it is {}.

Line 259 uses key-count as an existence check. That makes export const config = {} fall back to inline default-export config, which violates the stated precedence rule.

💡 Proposed fix
diff --git a/packages/zip-it-and-ship-it/src/runtimes/node/parser/exports.ts b/packages/zip-it-and-ship-it/src/runtimes/node/parser/exports.ts
@@
 export const traverseNodes = (nodes: Statement[], getAllBindings: BindingMethod) => {
   const handlerExports: ISCExport[] = []
 
   let configExport: Record<string, unknown> = {}
+  let hasConfigExport = false
@@
     if (esmConfigExports.length !== 0 && esmConfigExports[0].type === 'object-expression') {
       configExport = esmConfigExports[0].object
+      hasConfigExport = true
     }
@@
     if (esmConfig !== undefined) {
       configExport = esmConfig
+      hasConfigExport = true
 
       return
     }
@@
     if (cjsConfigExports.length !== 0 && cjsConfigExports[0].type === 'object-expression') {
       configExport = cjsConfigExports[0].object
+      hasConfigExport = true
     }
   })
 
-  return { configExport, handlerExports, hasDefaultExport, defaultExportExpression, inputModuleFormat }
+  return { configExport, hasConfigExport, handlerExports, hasDefaultExport, defaultExportExpression, inputModuleFormat }
 }
diff --git a/packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts b/packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts
@@
-  const { configExport, handlerExports, hasDefaultExport, defaultExportExpression, inputModuleFormat } = traverseNodes(
+  const { configExport, hasConfigExport, handlerExports, hasDefaultExport, defaultExportExpression, inputModuleFormat } = traverseNodes(
     ast.body,
     getAllBindings,
   )
@@
-    const mergedConfigExport = Object.keys(configExport).length > 0 ? configExport : (inlineConfig ?? {})
+    const mergedConfigExport = hasConfigExport ? configExport : (inlineConfig ?? {})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts`
around lines 256 - 260, The code treats an explicitly exported empty object as
absent by using Object.keys(configExport).length > 0; change this to detect the
presence of a named export instead of key-count. Replace the condition that
builds mergedConfigExport (currently using Object.keys(configExport).length > 0)
with a presence check such as checking configExport is not undefined or using a
dedicated boolean like configExportIsPresent (set where exports are parsed) so
that export const config = {} takes precedence over inline default export config
from getConfigFromDefaultExport; keep the rest (mergedConfigExport and
inSourceConfig.safeParse) unchanged.


if (success) {
result.config = data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ const parseConfigESMExport = (node: Statement) => {
* subtree. Only values supported by the `parsePrimitive` method are returned,
* and any others will be ignored and excluded from the resulting object.
*/
const parseObject = (node: ObjectExpression) =>
export const parseObject = (node: ObjectExpression) =>
node.properties.reduce(
(acc, property): Record<string, unknown> => {
if (property.type === 'ObjectProperty' && property.key.type === 'Identifier') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -905,4 +905,57 @@ describe('V2 API', () => {
runtimeAPIVersion: 2,
})
})

describe('Inline config in default export object', () => {
test('Extracts config from the default export object', () => {
const source = `export default {
fetch() { return new Response("Hello") },
config: { path: "/hello" }
}`

const isc = parseSource(source, options)

expect(isc.runtimeAPIVersion).toBe(2)
expect(isc.config).toEqual({ path: ['/hello'] })
expect(isc.routes).toEqual([{ pattern: '/hello', literal: '/hello', methods: [], prefer_static: undefined }])
expect(isc.eventSubscriptions).toEqual(['fetch'])
})

test('Extracts config from a binding-resolved default export object', () => {
const source = `const handlers = {
fetch() { return new Response("Hello") },
config: { path: "/api" }
}
export default handlers`

const isc = parseSource(source, options)

expect(isc.runtimeAPIVersion).toBe(2)
expect(isc.config).toEqual({ path: ['/api'] })
expect(isc.routes).toHaveLength(1)
})

test('Named config export takes precedence over inline config', () => {
const source = `export default {
fetch() { return new Response("Hello") },
config: { path: "/inline" }
}
export const config = { path: "/named" }`

const isc = parseSource(source, options)

expect(isc.config).toEqual({ path: ['/named'] })
})

test('Ignores non-object config property', () => {
const source = `export default {
fetch() { return new Response("Hello") },
config: "not-an-object"
}`

const isc = parseSource(source, options)

expect(isc.config).toEqual({})
})
})
})
Loading