feat: export ServerConfigSchema from public validation module — closes #127#187
Conversation
|
@spiffamani Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
Code Review
This pull request introduces a runtime validation schema and utility function for server configurations, along with a comprehensive test suite. The feedback identifies that the schema is currently incomplete relative to the ServerConfig type, missing properties like corsOrigins and requestTimeout. Additionally, there are suggestions to improve the precision of test assertions and to verify the export of all validation utilities from the package index.
| export const ServerConfigSchema: Record<keyof Required<ServerConfig>, SchemaField> = { | ||
| host: { | ||
| type: 'string', | ||
| required: false, | ||
| description: 'Server host address. Defaults to 0.0.0.0', | ||
| validate: (value) => typeof value === 'string' && value.length > 0, | ||
| }, | ||
| port: { | ||
| type: 'number', | ||
| required: false, | ||
| description: 'Server port number. Defaults to 3000.', | ||
| validate: (value) => | ||
| typeof value === 'number' && | ||
| Number.isInteger(value) && | ||
| value > 0 && | ||
| value <= 65535, | ||
| }, | ||
| debug: { | ||
| type: 'boolean', | ||
| required: false, | ||
| description: 'Enable debug mode for verbose logging. Defaults to false.', | ||
| validate: (value) => typeof value === 'boolean', | ||
| }, | ||
| interactiveDomain: { | ||
| type: 'string', | ||
| required: false, | ||
| description: 'Interactive web portal domain/URL for SEP-24 flows.', | ||
| validate: (value) => { | ||
| if (typeof value !== 'string' || value.length === 0) return false; | ||
| try { | ||
| const url = new URL(value); | ||
| return url.protocol === 'http:' || url.protocol === 'https:'; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The ServerConfigSchema is incomplete. It is typed as Record<keyof Required<ServerConfig>, SchemaField>, which requires all properties of ServerConfig to be defined in the schema. However, corsOrigins and requestTimeout from the ServerConfig interface are missing.
This will likely cause a TypeScript compilation error. If not, it's a significant bug because the validation logic will not cover these properties, which could lead to invalid configuration being silently accepted.
Please add the missing properties to the schema with appropriate validation.
export const ServerConfigSchema: Record<keyof Required<ServerConfig>, SchemaField> = {
host: {
type: 'string',
required: false,
description: 'Server host address. Defaults to 0.0.0.0',
validate: (value) => typeof value === 'string' && value.length > 0,
},
port: {
type: 'number',
required: false,
description: 'Server port number. Defaults to 3000.',
validate: (value) =>
typeof value === 'number' &&
Number.isInteger(value) &&
value > 0 &&
value <= 65535,
},
debug: {
type: 'boolean',
required: false,
description: 'Enable debug mode for verbose logging. Defaults to false.',
validate: (value) => typeof value === 'boolean',
},
interactiveDomain: {
type: 'string',
required: false,
description: 'Interactive web portal domain/URL for SEP-24 flows.',
validate: (value) => {
if (typeof value !== 'string' || value.length === 0) return false;
try {
const url = new URL(value);
return url.protocol === 'http:' || url.protocol === 'https:';
} catch {
return false;
}
},
},
corsOrigins: {
type: 'string[]',
required: false,
description: 'Allowed origins for CORS.',
validate: (value) =>
Array.isArray(value) &&
value.every((item) => typeof item === 'string' && item.length > 0),
},
requestTimeout: {
type: 'number',
required: false,
description: 'Request timeout in milliseconds.',
validate: (value) =>
typeof value === 'number' && Number.isInteger(value) && value > 0,
},
};| port: 0, | ||
| debug: 'yes' as unknown as boolean, | ||
| }); | ||
| expect(errors.length).toBeGreaterThanOrEqual(2); |
There was a problem hiding this comment.
The assertion expect(errors.length).toBeGreaterThanOrEqual(2) is not very precise. For a more robust test that prevents future regressions, it's better to assert the exact length and content of the errors array.
expect(errors).toHaveLength(2);
expect(errors).toContain('port: invalid value');
expect(errors).toContain('debug: invalid value');|
|
||
| it('is accessible through the utils index', async () => { | ||
| const mod = await import('../../src/utils/index'); | ||
| expect(mod.ServerConfigSchema).toBeDefined(); |
There was a problem hiding this comment.
This test is incomplete. It only checks for ServerConfigSchema being exported from the utils index. Since validateServerConfig is also exported and intended for public use, the test should verify that it's also accessible from the index file.
expect(mod.ServerConfigSchema).toBeDefined();
expect(mod.validateServerConfig).toBeDefined();
Summary
Exports
ServerConfigSchemafrom the public validation module, closing #127.Changes
src/utils/validation.ts— AddedServerConfigSchemaandvalidateServerConfigtests/utils/server-config-schema.test.ts— 18 focused testsWhat's exported
ServerConfigSchema— runtime schema object withvalidate()per fieldvalidateServerConfig()— validates partial ServerConfig, returns error stringsUsage
Test Results
Checklist
ServerConfigSchemais publicly importablebun testpasses