feat: support config property in default export

[eduardoboucas]

Currently, config is only supported with a named config export:

export const config = {
  path: "/hello"
}

Now that we allow a function to be defined with a default export object containing multiple event handlers, it would be nice if the config could also be part of that:

export default {
  fetch(req, context) {
    return new Response("Hello")
  },

  config: {
    path: "/hello"
  },

  deplotSucceeded(event) {
    console.log("New deploy!")
  }
}

This PR adds support for that.

Part of https://linear.app/netlify/issue/RUN-2652/improve-syntax-of-event-triggered-functions.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request extends the in-source configuration parsing for Node.js runtimes to support configuration objects embedded directly within module default exports. The changes introduce a new resolveObjectExpression helper function to dereference identifier-bound values, add getConfigFromDefaultExport to extract and parse inline config objects, and modify the config parsing logic to prioritize explicit exports over inline configurations with a fallback mechanism. Additionally, the parseObject function is exported to enable reuse by the new in-source configuration module. New test coverage validates the parsing behavior for inline default-export configurations, binding resolution, and precedence rules.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Notes on the changes

The modifications introduce new logic patterns for object expression resolution and configuration extraction that require careful validation of control flow and edge cases. The logic density is moderate, with new helper functions (resolveObjectExpression, getConfigFromDefaultExport) operating on AST nodes. The changes span multiple areas (config extraction, parsing, and event subscription detection) but remain within the in-source configuration parsing scope. The addition of comprehensive test coverage aids review but the logic itself demands understanding of the new resolution mechanisms and precedence rules.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The PR description clearly explains the motivation and provides code examples showing the before/after behavior, but does not follow the required template structure with checkboxes and sections.	Align the description with the repository template by adding the required checklist items (issue reference, contribution guidelines acknowledgment, testing confirmation, documentation updates, and status checks) and the optional cute animal picture.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: adding support for a config property in default exports, which aligns with the implemented changes across all modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/config-default-export

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

packages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.ts (1)

938-948: Add a precedence regression test for empty named config export.

Please add a case where named config is explicitly empty (export const config = {}) and inline config is present, asserting the result stays {}. This will lock in precedence semantics.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.ts`
around lines 938 - 948, Add a regression test to cover the case where a named
config export is explicitly empty so that it overrides any inline config: in the
same test file add a new test (similar to the existing 'Named config export
takes precedence over inline config') that defines source with an inline config
(e.g., config: { path: "/inline" }) but also declares `export const config = {}`
and then calls `parseSource(source, options)` and asserts `isc.config` equals an
empty object `{}`; reference `parseSource` and `isc.config` to locate where to
add this test and mirror the surrounding test structure for consistency.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts`:
- Around line 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.
- Around line 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.

---

Nitpick comments:
In
`@packages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.ts`:
- Around line 938-948: Add a regression test to cover the case where a named
config export is explicitly empty so that it overrides any inline config: in the
same test file add a new test (similar to the existing 'Named config export
takes precedence over inline config') that defines source with an inline config
(e.g., config: { path: "/inline" }) but also declares `export const config = {}`
and then calls `parseSource(source, options)` and asserts `isc.config` equals an
empty object `{}`; reference `parseSource` and `isc.config` to locate where to
add this test and mirror the surrounding test structure for consistency.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 06d3ece7-c405-4339-9497-955f75ac3545

📥 Commits

Reviewing files that changed from the base of the PR and between 4c1f6b1 and f94a63a.

📒 Files selected for processing (3)

• packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts
• packages/zip-it-and-ship-it/src/runtimes/node/parser/exports.ts
• packages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.ts

[coderabbitai]

⚠️ 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.

[coderabbitai]

⚠️ 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.