-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement Zod for Robust Configuration Validation #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1ec1b2b
d37a6a6
cf5b6a8
b9e39b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import path from 'node:path'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { loadFileConfig, mergeConfigs } from '../../config/configLoader.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RepopackConfigCli, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RepopackConfigFile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RepopackConfigMerged, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RepopackOutputStyle, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '../../config/configTypes.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigCli, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigFile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigMerged, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackOutputStyle, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| repopackConfigCliSchema, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '../../config/configSchema.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { type PackResult, pack } from '../../core/packager.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { rethrowValidationErrorIfZodError } from '../../shared/errorHandler.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from '../../shared/logger.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { printCompletion, printSecurityCheck, printSummary, printTopFiles } from '../cliPrinter.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { CliOptions } from '../cliRunner.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,26 +30,8 @@ export const runDefaultAction = async ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fileConfig: RepopackConfigFile = await loadFileConfig(cwd, options.config ?? null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.trace('Loaded file config:', fileConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parse the CLI options | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cliConfig: RepopackConfigCli = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.output) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { filePath: options.output }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.include) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.include = options.include.split(','); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.ignore) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.ignore = { customPatterns: options.ignore.split(',') }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.topFilesLen !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, topFilesLength: options.topFilesLen }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.outputShowLineNumbers !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, showLineNumbers: options.outputShowLineNumbers }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.style) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, style: options.style.toLowerCase() as RepopackOutputStyle }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parse the CLI options into a config | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cliConfig: RepopackConfigCli = buildCliConfig(options); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.trace('CLI config:', cliConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Merge default, file, and CLI configs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -99,3 +83,33 @@ export const runDefaultAction = async ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| config, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const buildCliConfig = (options: CliOptions): RepopackConfigCli => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cliConfig: RepopackConfigCli = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.output) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { filePath: options.output }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.include) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.include = options.include.split(','); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.ignore) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.ignore = { customPatterns: options.ignore.split(',') }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.topFilesLen !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, topFilesLength: options.topFilesLen }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.outputShowLineNumbers !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, showLineNumbers: options.outputShowLineNumbers }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (options.style) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig.output = { ...cliConfig.output, style: options.style.toLowerCase() as RepopackOutputStyle }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return repopackConfigCliSchema.parse(cliConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rethrowValidationErrorIfZodError(error, 'Invalid cli arguments'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+110
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance Error Handling in Configuration Parsing While the current error handling rethrows validation errors, consider providing more detailed feedback to the user. This can include which specific CLI option caused the validation to fail, making it easier for users to correct their input. You might adjust the catch block as follows to include detailed error messages: } catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid CLI arguments');
+ if (error instanceof ZodError) {
+ logger.error('Configuration validation failed:', error.errors);
+ }
throw error;
}This change logs detailed validation errors without affecting the existing error flow. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential TypeError When Spreading Undefined In the To fix this issue, ensure that const buildCliConfig = (options: CliOptions): RepopackConfigCli => {
const cliConfig: RepopackConfigCli = {};
+ // Initialize cliConfig.output to an empty object
+ cliConfig.output = {};
if (options.output) {
cliConfig.output = { filePath: options.output };
}
if (options.include) {
cliConfig.include = options.include.split(',');
}
if (options.ignore) {
cliConfig.ignore = { customPatterns: options.ignore.split(',') };
}
if (options.topFilesLen !== undefined) {
- cliConfig.output = { ...cliConfig.output, topFilesLength: options.topFilesLen };
+ cliConfig.output = {
+ ...cliConfig.output,
+ topFilesLength: options.topFilesLen,
+ };
}
if (options.outputShowLineNumbers !== undefined) {
cliConfig.output = {
...cliConfig.output,
showLineNumbers: options.outputShowLineNumbers,
};
}
if (options.style) {
cliConfig.output = {
...cliConfig.output,
style: options.style.toLowerCase() as RepopackOutputStyle,
};
}
try {
return repopackConfigCliSchema.parse(cliConfig);
} catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid CLI arguments');
throw error;
}
};Alternatively, you can check if if (options.topFilesLen !== undefined) {
- cliConfig.output = { ...cliConfig.output, topFilesLength: options.topFilesLen };
+ cliConfig.output = {
+ ...(cliConfig.output || {}),
+ topFilesLength: options.topFilesLen,
+ };
}Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as fs from 'node:fs/promises'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import path from 'node:path'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { RepopackError } from '../shared/errorHandler.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { z } from 'zod'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { RepopackError, rethrowValidationErrorIfZodError } from '../shared/errorHandler.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused import: 'z' from 'zod' The imported Consider removing the unused import to keep the code clean: -import { z } from 'zod';Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from '../shared/logger.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { RepopackConfigCli, RepopackConfigFile, RepopackConfigMerged } from './configTypes.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { RepopackConfigValidationError, validateConfig } from './configValidator.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigCli, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigFile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RepopackConfigMerged, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| repopackConfigFileSchema, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| repopackConfigMergedSchema, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from './configSchema.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { defaultConfig } from './defaultConfig.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getGlobalDirectory } from './globalDirectory.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,7 +31,6 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.trace('Loading local config from:', fullPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check local file existence | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isLocalFileExists = await fs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .stat(fullPath) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .then((stats) => stats.isFile()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -36,7 +41,6 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (useDefaultConfig) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Try to load global config | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const globalConfigPath = getGlobalConfigPath(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.trace('Loading global config from:', globalConfigPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -61,12 +65,9 @@ const loadAndValidateConfig = async (filePath: string): Promise<RepopackConfigFi | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fileContent = await fs.readFile(filePath, 'utf-8'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const config = JSON.parse(fileContent); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| validateConfig(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return config; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return repopackConfigFileSchema.parse(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (error instanceof RepopackConfigValidationError) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RepopackError(`Invalid configuration in ${filePath}: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rethrowValidationErrorIfZodError(error, 'Invalid config schema'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+68
to
+70
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent error handling in configuration parsing When parsing the configuration file with Apply this diff to streamline the error handling: } catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid config schema');
- if (error instanceof SyntaxError) {
- throw new RepopackError(`Invalid JSON in config file ${filePath}: ${error.message}`);
- }
- if (error instanceof Error) {
- throw new RepopackError(`Error loading config from ${filePath}: ${error.message}`);
- }
- throw new RepopackError(`Error loading config from ${filePath}`);
}Ensure that
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (error instanceof SyntaxError) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new RepopackError(`Invalid JSON in config file ${filePath}: ${error.message}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -81,27 +82,36 @@ export const mergeConfigs = ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fileConfig: RepopackConfigFile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cliConfig: RepopackConfigCli, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): RepopackConfigMerged => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| output: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ignore: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| customPatterns: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(defaultConfig.ignore.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(fileConfig.ignore?.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(cliConfig.ignore?.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include: [...(defaultConfig.include || []), ...(fileConfig.include || []), ...(cliConfig.include || [])], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| security: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): RepopackConfigMerged => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mergedConfig = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| output: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ignore: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.ignore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| customPatterns: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(defaultConfig.ignore.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(fileConfig.ignore?.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(cliConfig.ignore?.customPatterns || []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include: [...(defaultConfig.include || []), ...(fileConfig.include || []), ...(cliConfig.include || [])], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| security: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...defaultConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...fileConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cliConfig.security, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+85
to
+110
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider deep merging for nested configuration objects When merging configurations, properties like To ensure all nested properties are correctly merged, consider using a deep merge utility like + import merge from 'lodash.merge';
...
export const mergeConfigs = (
cwd: string,
fileConfig: RepopackConfigFile,
cliConfig: RepopackConfigCli,
): RepopackConfigMerged => {
- const mergedConfig = {
- cwd,
- output: {
- ...defaultConfig.output,
- ...fileConfig.output,
- ...cliConfig.output,
- },
- ignore: {
- ...defaultConfig.ignore,
- ...fileConfig.ignore,
- ...cliConfig.ignore,
- customPatterns: [
- ...(defaultConfig.ignore.customPatterns || []),
- ...(fileConfig.ignore?.customPatterns || []),
- ...(cliConfig.ignore?.customPatterns || []),
- ],
- },
- include: [...(defaultConfig.include || []), ...(fileConfig.include || []), ...(cliConfig.include || [])],
- security: {
- ...defaultConfig.security,
- ...fileConfig.security,
- ...cliConfig.security,
- },
- };
+ const mergedConfig = merge({}, defaultConfig, fileConfig, cliConfig, { cwd });
try {
return repopackConfigMergedSchema.parse(mergedConfig);
} catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid merged config');
throw error;
}
};This approach provides a more robust merging strategy, ensuring that all configuration values are appropriately combined without unintended overwrites. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return repopackConfigMergedSchema.parse(mergedConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rethrowValidationErrorIfZodError(error, 'Invalid merged config'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+112
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove redundant error rethrow in merged configuration validation After calling Apply this diff to eliminate the redundant throw statement: } catch (error) {
rethrowValidationErrorIfZodError(error, 'Invalid merged config');
- throw error;
}This change ensures that the error handling is clear and that only the intended error is propagated. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||
| import { z } from 'zod'; | ||||||||||||||||||||||||||||||||||
| import { RepopackError } from '../shared/errorHandler.js'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const repopackOutputStyleSchema = z.enum(['plain', 'xml']); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type RepopackOutputStyle = z.infer<typeof repopackOutputStyleSchema>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const repopackConfigBaseSchema = z.object({ | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Export Since Apply this diff to export the base schema: -const repopackConfigBaseSchema = z.object({
+export const repopackConfigBaseSchema = z.object({Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||
| output: z | ||||||||||||||||||||||||||||||||||
| .object({ | ||||||||||||||||||||||||||||||||||
| filePath: z.string().optional(), | ||||||||||||||||||||||||||||||||||
| style: repopackOutputStyleSchema.optional(), | ||||||||||||||||||||||||||||||||||
| headerText: z.string().optional(), | ||||||||||||||||||||||||||||||||||
| instructionFilePath: z.string().optional(), | ||||||||||||||||||||||||||||||||||
| removeComments: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| removeEmptyLines: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| topFilesLength: z.number().optional(), | ||||||||||||||||||||||||||||||||||
| showLineNumbers: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .optional(), | ||||||||||||||||||||||||||||||||||
| include: z.array(z.string()).optional(), | ||||||||||||||||||||||||||||||||||
| ignore: z | ||||||||||||||||||||||||||||||||||
| .object({ | ||||||||||||||||||||||||||||||||||
| useGitignore: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| useDefaultPatterns: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| customPatterns: z.array(z.string()).optional(), | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .optional(), | ||||||||||||||||||||||||||||||||||
| security: z | ||||||||||||||||||||||||||||||||||
| .object({ | ||||||||||||||||||||||||||||||||||
| enableSecurityCheck: z.boolean().optional(), | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .optional(), | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const repopackConfigDefaultSchema = repopackConfigBaseSchema.and( | ||||||||||||||||||||||||||||||||||
| z.object({ | ||||||||||||||||||||||||||||||||||
| output: z.object({ | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use When extending an object schema, it's more appropriate to use Apply this diff to replace -export const repopackConfigDefaultSchema = repopackConfigBaseSchema.and(
+export const repopackConfigDefaultSchema = repopackConfigBaseSchema.extend(
z.object({
output: z.object({
filePath: z.string(),
style: repopackOutputStyleSchema,
headerText: z.string().optional(),
instructionFilePath: z.string().optional(),
removeComments: z.boolean(),
removeEmptyLines: z.boolean(),
topFilesLength: z.number(),
showLineNumbers: z.boolean(),
}),
include: z.array(z.string()),
ignore: z.object({
useGitignore: z.boolean(),
useDefaultPatterns: z.boolean(),
customPatterns: z.array(z.string()).optional(),
}),
security: z.object({
enableSecurityCheck: z.boolean(),
}),
}),
);Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||
| filePath: z.string(), | ||||||||||||||||||||||||||||||||||
| style: repopackOutputStyleSchema, | ||||||||||||||||||||||||||||||||||
| headerText: z.string().optional(), | ||||||||||||||||||||||||||||||||||
| instructionFilePath: z.string().optional(), | ||||||||||||||||||||||||||||||||||
| removeComments: z.boolean(), | ||||||||||||||||||||||||||||||||||
| removeEmptyLines: z.boolean(), | ||||||||||||||||||||||||||||||||||
| topFilesLength: z.number(), | ||||||||||||||||||||||||||||||||||
| showLineNumbers: z.boolean(), | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| include: z.array(z.string()), | ||||||||||||||||||||||||||||||||||
| ignore: z.object({ | ||||||||||||||||||||||||||||||||||
| useGitignore: z.boolean(), | ||||||||||||||||||||||||||||||||||
| useDefaultPatterns: z.boolean(), | ||||||||||||||||||||||||||||||||||
| customPatterns: z.array(z.string()).optional(), | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| security: z.object({ | ||||||||||||||||||||||||||||||||||
| enableSecurityCheck: z.boolean(), | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type RepopackConfigDefault = z.infer<typeof repopackConfigDefaultSchema>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const repopackConfigFileSchema = repopackConfigBaseSchema; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type RepopackConfigFile = z.infer<typeof repopackConfigFileSchema>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const repopackConfigCliSchema = repopackConfigBaseSchema; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type RepopackConfigCli = z.infer<typeof repopackConfigCliSchema>; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const repopackConfigMergedSchema = repopackConfigDefaultSchema | ||||||||||||||||||||||||||||||||||
| .and(repopackConfigFileSchema) | ||||||||||||||||||||||||||||||||||
| .and(repopackConfigCliSchema) | ||||||||||||||||||||||||||||||||||
| .and( | ||||||||||||||||||||||||||||||||||
| z.object({ | ||||||||||||||||||||||||||||||||||
| cwd: z.string(), | ||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+70
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address Overlapping Keys in Combined Schemas When combining multiple schemas that may have overlapping keys, using Apply this diff to use -export const repopackConfigMergedSchema = repopackConfigDefaultSchema
- .and(repopackConfigFileSchema)
- .and(repopackConfigCliSchema)
- .and(
+export const repopackConfigMergedSchema = repopackConfigDefaultSchema
+ .merge(repopackConfigFileSchema)
+ .merge(repopackConfigCliSchema)
+ .extend(
z.object({
cwd: z.string(),
}),
);Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export type RepopackConfigMerged = z.infer<typeof repopackConfigMergedSchema>; | ||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.