feat: add additional function config properties#7064
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds support for optional function Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This pull request adds or modifies JavaScript ( |
region config propertyregion and memory config properties
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts (1)
6-20: ⚡ Quick winAdd a decimal-number test for the number-input branch.
The number path currently lacks a decimal case, so it won’t catch divergence from the “integer MB normalization” behavior.
Suggested test addition
describe('number inputs', () => { @@ test('passes through negative numbers (range checks are the caller’s concern)', () => { expect(parseMemoryMB(-128)).toBe(-128) }) + + test('normalizes decimal numbers to integer MB', () => { + expect(parseMemoryMB(1024.5)).toBe(1025) + }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts` around lines 6 - 20, Add a test case in the number-input suite of parse_memory.test.ts to assert parseMemoryMB preserves decimal numbers (e.g., 512.5) so the number branch is covered for non-integers; locate the describe('number inputs') block and add a new test that calls parseMemoryMB with a decimal value and expects the same decimal value to be returned, referencing parseMemoryMB to validate integer-normalization behavior doesn't inadvertently coerce or round decimals.packages/zip-it-and-ship-it/src/utils/format_result.ts (1)
28-29: ⚡ Quick winRemove unnecessary type assertion on
memorypropagation.Line 28's
as number | undefinedis redundant—botharchive.staticAnalysisResult?.config?.memoryandarchive.config?.memoryalready infer tonumber | undefinedfrom the underlying Zod schemas, so the nullish coalesce result is correctly typed without it. Removing the assertion keeps type drift detectable by TypeScript. Extracting both values to local variables (as suggested) improves consistency and readability.Suggested change
export const formatZipResult = (archive: FunctionArchive) => { + const memory = archive.staticAnalysisResult?.config?.memory ?? archive.config?.memory + const region = archive.staticAnalysisResult?.config?.region ?? archive.config?.region + const functionResult: FunctionResult = { ...archive, staticAnalysisResult: undefined, eventSubscriptions: archive.staticAnalysisResult?.eventSubscriptions, routes: archive.staticAnalysisResult?.routes, excludedRoutes: archive.staticAnalysisResult?.excludedRoutes, runtime: archive.runtime.name, - memory: (archive.staticAnalysisResult?.config?.memory ?? archive?.config?.memory) as number | undefined, - region: archive.staticAnalysisResult?.config?.region ?? archive?.config?.region, + memory, + region, schedule: archive.staticAnalysisResult?.config?.schedule ?? archive?.config?.schedule, runtimeAPIVersion: archive.staticAnalysisResult?.runtimeAPIVersion, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zip-it-and-ship-it/src/utils/format_result.ts` around lines 28 - 29, Remove the unnecessary type assertion on the memory propagation line: drop "as number | undefined" from the expression that assigns memory (the nullish-coalesced result of archive.staticAnalysisResult?.config?.memory ?? archive?.config?.memory) so TypeScript can correctly infer its type; optionally extract archive.staticAnalysisResult?.config?.memory and archive?.config?.memory into local variables first to improve readability and consistency with the region propagation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zip-it-and-ship-it/src/utils/parse_memory.ts`:
- Around line 1-23: The file fails the oxfmt formatting check; run the
repository formatter (run `oxfmt --apply` or your project's standard format
command) on packages/zip-it-and-ship-it/src/utils/parse_memory.ts to fix
whitespace/formatting around the constants and the parseMemoryMB function
(symbols: MEMORY_UNIT_TO_MB, MEMORY_PATTERN, parseMemoryMB), then re-commit the
formatted file so CI's `oxfmt --check` passes.
- Around line 10-11: In parseMemory, the numeric branch currently returns the
raw input which allows decimals to bypass normalization; update the numeric path
(the check using "if (typeof input === 'number')" / the input parameter) to
apply the same integer rounding used for string inputs (e.g., Math.round or the
existing rounding logic used on the string branch) so numbers are normalized to
integer MB before returning.
---
Nitpick comments:
In `@packages/zip-it-and-ship-it/src/utils/format_result.ts`:
- Around line 28-29: Remove the unnecessary type assertion on the memory
propagation line: drop "as number | undefined" from the expression that assigns
memory (the nullish-coalesced result of
archive.staticAnalysisResult?.config?.memory ?? archive?.config?.memory) so
TypeScript can correctly infer its type; optionally extract
archive.staticAnalysisResult?.config?.memory and archive?.config?.memory into
local variables first to improve readability and consistency with the region
propagation logic.
In `@packages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts`:
- Around line 6-20: Add a test case in the number-input suite of
parse_memory.test.ts to assert parseMemoryMB preserves decimal numbers (e.g.,
512.5) so the number branch is covered for non-integers; locate the
describe('number inputs') block and add a new test that calls parseMemoryMB with
a decimal value and expects the same decimal value to be returned, referencing
parseMemoryMB to validate integer-normalization behavior doesn't inadvertently
coerce or round decimals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d32dc83-bdcb-41f6-8cf2-661330163c84
📒 Files selected for processing (9)
packages/zip-it-and-ship-it/src/config.tspackages/zip-it-and-ship-it/src/manifest.tspackages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.tspackages/zip-it-and-ship-it/src/utils/format_result.tspackages/zip-it-and-ship-it/src/utils/parse_memory.tspackages/zip-it-and-ship-it/tests/fixtures-esm/v2-api-name-generator/.netlify/functions-internal/server.mjspackages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.tspackages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.tspackages/zip-it-and-ship-it/tests/v2api.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts
- packages/zip-it-and-ship-it/tests/v2api.test.ts
- packages/zip-it-and-ship-it/tests/unit/runtimes/node/in_source_config.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zip-it-and-ship-it/src/utils/parse_memory.ts`:
- Around line 9-12: The number branch in parseMemoryMB currently rounds numeric
input but doesn't guard against Infinity/-Infinity or negative numbers; update
the typeof input === 'number' branch in parseMemoryMB to first check
Number.isFinite(input) and that input is >= 0, then return Math.round(input),
otherwise return NaN so Infinity, -Infinity and negative numeric memory values
are treated as invalid consistent with string parsing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0e959d8-2f2c-4595-b611-680724609cbf
📒 Files selected for processing (4)
packages/config/src/functions_config.jspackages/config/src/validate/validations.jspackages/zip-it-and-ship-it/src/utils/parse_memory.tspackages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/config/src/functions_config.js
- packages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zip-it-and-ship-it/src/utils/parse_memory.ts`:
- Around line 22-25: The string parsing path in parse_memory.ts can produce
Infinity for extremely large inputs because Math.round(value *
MEMORY_UNIT_TO_MB[unit]) isn’t checked; mirror the numeric path’s guard by
computing the converted value into a variable (e.g., const bytes =
Math.round(value * MEMORY_UNIT_TO_MB[unit])) and then verify
Number.isFinite(bytes) (or Number.isFinite(value) before rounding) and return
NaN (or handle the same rejection as the numeric branch) if it's not finite;
reference the symbols value, unit, MEMORY_UNIT_TO_MB and the Math.round call
when making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16cbafee-75d1-4269-ba4d-195325849bf7
📒 Files selected for processing (3)
packages/zip-it-and-ship-it/src/utils/format_result.tspackages/zip-it-and-ship-it/src/utils/parse_memory.tspackages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/zip-it-and-ship-it/tests/unit/utils/parse_memory.test.ts
- packages/zip-it-and-ship-it/src/utils/format_result.ts
| const value = parseFloat(match[1]) | ||
| const unit = match[2] || 'mb' | ||
|
|
||
| return Math.round(value * MEMORY_UNIT_TO_MB[unit]) |
There was a problem hiding this comment.
Guard against overflow to Infinity in string path.
After unit conversion, extremely large inputs (e.g., "999999999999999gb") can overflow to Infinity. Unlike the numeric path, which explicitly rejects infinite values, Math.round(Infinity) returns Infinity rather than NaN.
This is an edge case unlikely to occur with valid memory values, but it's inconsistent with the finite guard in the numeric branch.
🛡️ Proposed fix to ensure consistency
const value = parseFloat(match[1])
const unit = match[2] || 'mb'
+ const result = value * MEMORY_UNIT_TO_MB[unit]
+ return Number.isFinite(result) ? Math.round(result) : Number.NaN
- return Math.round(value * MEMORY_UNIT_TO_MB[unit])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const value = parseFloat(match[1]) | |
| const unit = match[2] || 'mb' | |
| return Math.round(value * MEMORY_UNIT_TO_MB[unit]) | |
| const value = parseFloat(match[1]) | |
| const unit = match[2] || 'mb' | |
| const result = value * MEMORY_UNIT_TO_MB[unit] | |
| return Number.isFinite(result) ? Math.round(result) : Number.NaN |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/zip-it-and-ship-it/src/utils/parse_memory.ts` around lines 22 - 25,
The string parsing path in parse_memory.ts can produce Infinity for extremely
large inputs because Math.round(value * MEMORY_UNIT_TO_MB[unit]) isn’t checked;
mirror the numeric path’s guard by computing the converted value into a variable
(e.g., const bytes = Math.round(value * MEMORY_UNIT_TO_MB[unit])) and then
verify Number.isFinite(bytes) (or Number.isFinite(value) before rounding) and
return NaN (or handle the same rejection as the numeric branch) if it's not
finite; reference the symbols value, unit, MEMORY_UNIT_TO_MB and the Math.round
call when making this change.
region and memory config properties
Adds
region,memoryandvcpu.