Skip to content

Simplify function schemas#196

Merged
artemdemo merged 4 commits intomainfrom
refactor-function-schema
Feb 5, 2026
Merged

Simplify function schemas#196
artemdemo merged 4 commits intomainfrom
refactor-function-schema

Conversation

@artemdemo
Copy link
Contributor

@artemdemo artemdemo commented Feb 5, 2026

Note

Description

Refactors the function resource module to simplify schema validation and improve naming clarity. Removes redundant runtime Zod validation in readFunction() since the config is already validated by FunctionConfigSchema. Renames FunctionSchema to BackendFunctionSchema and the files property to filePaths for better semantic accuracy.

Related Issue

None

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Other (please describe):

Changes Made

  • Renamed FunctionSchema to BackendFunctionSchema for naming consistency with other resource schemas
  • Renamed files property to filePaths to accurately reflect that it stores file paths, not contents
  • Removed redundant Zod validation in readFunction() - config is already validated by readFunctionConfig()
  • Simplified function reading logic by directly constructing the BackendFunction object
  • Updated import order to follow project conventions (external imports first)

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed (existing tests cover this refactoring)
  • All tests pass (npm test)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas (not needed for this refactoring)
  • I have made corresponding changes to the documentation (not applicable)
  • My changes generate no new warnings
  • I have updated AGENTS.md if I made architectural changes (not applicable - internal refactoring only)

Additional Notes

This refactoring removes defensive validation that was unnecessary since:

  1. The config file is validated by FunctionConfigSchema in readFunctionConfig()
  2. The additional properties (entryPath, filePaths) are constructed internally and type-safe

The rename from files to filePaths makes it clear that these are paths to files (strings), not file content objects.


🤖 Generated by Claude | 2026-02-05 08:46 UTC

@artemdemo
Copy link
Contributor Author

@claude review

@claude
Copy link

claude bot commented Feb 5, 2026

Claude finished @artemdemo's task in 1m 9s —— View job


Code Review

Tasks:

  • Read modified files to understand changes
  • Analyze git diff to see exact modifications
  • Review changes for correctness and best practices
  • Provide comprehensive feedback

Summary

This PR simplifies the function schema by:

  1. Renaming FunctionSchema to BackendFunctionSchema (better naming)
  2. Renaming the files property to filePaths in BackendFunctionSchema
  3. Removing redundant schema validation in readFunction()

Issues Found

🚨 Critical: Breaking Change

The PR introduces a breaking bug by renaming files to filePaths in BackendFunctionSchema, but the code in deploy.ts:15 still references fn.files:

// src/core/resources/function/deploy.ts:15
fn.files.map(async (filePath) => {
  // This will fail at runtime!
})

This will cause a runtime error when attempting to deploy functions because BackendFunction no longer has a files property - it's now filePaths.

Fix required: Update deploy.ts line 15 to use fn.filePaths instead of fn.files.

Fix this →


Positive Changes

Improved Schema Simplification

Removing the redundant validation in config.ts is a good simplification:

Before (lines 45-57):

const functionData = { ...config, entryPath, files };
const result = FunctionSchema.safeParse(functionData);
if (!result.success) {
  throw new SchemaValidationError("Invalid function", result.error, configPath);
}
return result.data;

After (lines 50-51):

const functionData: BackendFunction = { ...config, entryPath, filePaths };
return functionData;

This is valid because:

  1. config is already validated by FunctionConfigSchema in readFunctionConfig()
  2. entryPath is validated to exist via pathExists() check
  3. filePaths comes from globby() which always returns an array
  4. TypeScript enforces the shape at compile time

Better Naming

  • FunctionSchemaBackendFunctionSchema is more descriptive
  • filesfilePaths clarifies that it's an array of file paths (strings), not file objects

Recommendations

Test Coverage

The existing test (tests/cli/functions_deploy.spec.ts) should catch this bug if it's executed, since it attempts to deploy functions. Before merging, ensure:

bun run build && bun run test

The test fixture at tests/fixtures/with-functions-and-entities/ should trigger the fn.files access.

Consider TypeScript Strictness

The bug would have been caught at compile time if TypeScript's type checking was stricter. The current code uses:

async function loadFunctionCode(fn: BackendFunction): Promise<FunctionWithCode>

TypeScript should error on fn.files since BackendFunction only has filePaths. This might indicate:

  • The types aren't being checked properly during development
  • Or there's a type assertion somewhere bypassing the check

Summary

Verdict:Changes requested - Cannot merge due to breaking bug

Action items:

  1. Fix deploy.ts:15 to use fn.filePaths instead of fn.files
  2. Run tests to verify the fix: bun run build && bun run test
  3. Consider why TypeScript didn't catch this at compile time

The refactoring direction is good, but the incomplete property rename will break function deployments at runtime.


@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.28-pr.196.60a4444

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.28-pr.196.60a4444"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.28-pr.196.60a4444"
  }
}

Preview published to npm registry — try new features instantly!

Comment on lines +50 to +51
const functionData: BackendFunction = { ...config, entryPath, filePaths };
return functionData;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's enough to validate it based on paths - this is not user providing us with arbitrary data. It's our code

export const BackendFunctionSchema = FunctionConfigSchema.extend({
entryPath: z.string().min(1, "Entry path cannot be empty"),
files: z.array(z.string()).min(1, "Function must have at least one file"),
filePaths: z.array(z.string()).min(1, "Function must have at least one file"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

files used twice in schemas as "file path" and as actual "file content" I want to make names clear

});

export const FunctionSchema = FunctionConfigSchema.extend({
export const BackendFunctionSchema = FunctionConfigSchema.extend({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's call in BackendFunctionSchema since type then called BackendFunction

@artemdemo artemdemo marked this pull request as ready for review February 5, 2026 08:11
@kfirstri kfirstri moved this from Backlog to In review in CLI Development Feb 5, 2026
Copy link
Collaborator

@kfirstri kfirstri left a comment

Choose a reason for hiding this comment

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

Thank you

@artemdemo artemdemo merged commit 2fc9c1d into main Feb 5, 2026
7 checks passed
@artemdemo artemdemo deleted the refactor-function-schema branch February 5, 2026 08:35
@github-project-automation github-project-automation bot moved this from In review to Done in CLI Development Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants