Skip to content

Commit

Permalink
refactor: onError param without extra object
Browse files Browse the repository at this point in the history
  • Loading branch information
fvsch committed Nov 24, 2024
1 parent 3851315 commit 3b38781
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 26 deletions.
5 changes: 1 addition & 4 deletions src/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ function normalizeExt(value: string = ''): string {
return value;
}

export function parseArgs(
args: CLIArgs,
{ onError }: { onError(msg: string): void },
): Partial<ServerOptions> {
export function parseArgs(args: CLIArgs, onError: (msg: string) => void): Partial<ServerOptions> {
const invalid = (optName = '', input = '') => {
const value =
typeof input === 'string' ? `'${input.replaceAll(`'`, `\'`)}'` : JSON.stringify(input);
Expand Down
6 changes: 3 additions & 3 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ export async function run() {
}

const onError = errorList();
const userOptions = parseArgs(args, { onError });
const options = serverOptions({ root: '', ...userOptions }, { onError });
await checkDirAccess(options.root, { onError });
const userOptions = parseArgs(args, onError);
const options = serverOptions({ root: '', ...userOptions }, onError);
await checkDirAccess(options.root, onError);

if (onError.list.length) {
logger.error(...onError.list);
Expand Down
6 changes: 4 additions & 2 deletions src/fs-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { FSKind, FSLocation } from './types.d.ts';

export async function checkDirAccess(
dirPath: string,
context: { onError(msg: string): void },
onError?: (msg: string) => void,
): Promise<boolean> {
let msg = '';
try {
Expand All @@ -26,7 +26,9 @@ export async function checkDirAccess(
msg = err.toString();
}
}
if (msg) context.onError(msg);
if (msg && onError) {
onError(msg);
}
return false;
}

Expand Down
5 changes: 2 additions & 3 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import type { HttpHeaderRule, ServerOptions } from './types.d.ts';

export function serverOptions(
options: ServerOptions,
context: { onError(msg: string): void },
onError: (msg: string) => void,
): Required<ServerOptions> {
const validator = new OptionsValidator(context.onError);
const validator = new OptionsValidator(onError);

const checked: Omit<ServerOptions, 'root'> = {
ports: validator.ports(options.ports),
Expand All @@ -26,7 +26,6 @@ export function serverOptions(
...DEFAULT_OPTIONS,
});
for (const [key, value] of Object.entries(checked)) {
const valid = typeof value !== 'undefined';
if (typeof value !== 'undefined') {
(final as Record<string, any>)[key] = value;
}
Expand Down
12 changes: 6 additions & 6 deletions test/args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,22 @@ suite('CLIArgs', () => {
suite('parseArgs', () => {
test('no errors for empty args', () => {
const onError = errorList();
parseArgs(new CLIArgs([]), { onError });
parseArgs(new CLIArgs([]), onError);
expect(onError.list).toEqual([]);
});

test('does not validate host and root strings', () => {
const onError = errorList();
const args = new CLIArgs(['--host', ' not a hostname!\n', 'https://not-a-valid-root']);
const options = parseArgs(args, { onError });
const options = parseArgs(args, onError);
expect(options.host).toBe('not a hostname!');
expect(options.root).toBe('https://not-a-valid-root');
expect(onError.list).toEqual([]);
});

test('validates --port syntax', () => {
const onError = errorList();
const parse = (str = '') => parseArgs(argify(str), { onError });
const parse = (str = '') => parseArgs(argify(str), onError);
expect(parse(`--port 1000+`)).toEqual({ ports: intRange(1000, 1009) });
expect(parse(`--port +1000`)).toEqual({});
expect(parse(`--port whatever`)).toEqual({});
Expand All @@ -129,7 +129,7 @@ suite('parseArgs', () => {
test('accepts valid --header syntax', () => {
const onError = errorList();
const getRule = (value = '') =>
parseArgs(new CLIArgs(['--header', value]), { onError }).headers?.at(0);
parseArgs(new CLIArgs(['--header', value]), onError).headers?.at(0);
expect(getRule('x-header-1: true')).toEqual({
headers: { 'x-header-1': 'true' },
});
Expand All @@ -146,7 +146,7 @@ suite('parseArgs', () => {
const onError = errorList();
const getRule = (value = '') => {
const args = new CLIArgs(['--header', value]);
return parseArgs(args, { onError }).headers?.at(0);
return parseArgs(args, onError).headers?.at(0);
};

expect(getRule('basic string')).toBe(undefined);
Expand All @@ -160,7 +160,7 @@ suite('parseArgs', () => {
test('sets warnings for unknown args', () => {
const onError = errorList();
const args = argify`--help --port=9999 --never gonna -GiveYouUp`;
parseArgs(args, { onError });
parseArgs(args, onError);
expect(onError.list).toEqual([`unknown option '--never'`, `unknown option '-GiveYouUp'`]);
});
});
Expand Down
6 changes: 3 additions & 3 deletions test/fs-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ suite('fs utils', async () => {

test('checkDirAccess', async () => {
const onError = errorList();
expect(await checkDirAccess(path``, { onError })).toBe(true);
expect(await checkDirAccess(path`section1`, { onError })).toBe(true);
expect(await checkDirAccess(path``, onError)).toBe(true);
expect(await checkDirAccess(path`section1`, onError)).toBe(true);
const notAFolder = path`doesnt/exist`;
expect(await checkDirAccess(notAFolder, { onError })).toBe(false);
expect(await checkDirAccess(notAFolder, onError)).toBe(false);
expect(onError.list).toEqual([`not a directory: ${notAFolder}`]);
});

Expand Down
10 changes: 5 additions & 5 deletions test/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ suite('OptionsValidator', () => {
suite('serverOptions', () => {
test('returns default options with empty input', () => {
const onError = errorList();
const { root, ...result } = serverOptions({ root: cwd() }, { onError });
const { root, ...result } = serverOptions({ root: cwd() }, onError);
expect(result).toEqual(DEFAULT_OPTIONS);
expect(onError.list).toEqual([]);
});
Expand All @@ -319,7 +319,7 @@ suite('serverOptions', () => {
gzip: false,
cors: true,
};
expect(serverOptions(testOptions1, { onError })).toEqual({
expect(serverOptions(testOptions1, onError)).toEqual({
...DEFAULT_OPTIONS,
...testOptions1,
});
Expand All @@ -332,7 +332,7 @@ suite('serverOptions', () => {
headers: [{ include: ['*.md', '*.html'], headers: { dnt: 1 } }],
host: '192.168.1.199',
};
expect(serverOptions(testOptions2, { onError })).toEqual({
expect(serverOptions(testOptions2, onError)).toEqual({
...DEFAULT_OPTIONS,
...testOptions2,
});
Expand All @@ -357,9 +357,9 @@ suite('serverOptions', () => {
const { root, ...result } = serverOptions(
// @ts-expect-error
inputs,
{ onError },
onError,
);
expect(typeof root).toBe('string');
expect(root).toBeTypeOf('string');
expect(Object.keys(result).length).toBeGreaterThanOrEqual(9);
expect(result).toEqual(DEFAULT_OPTIONS);
});
Expand Down

0 comments on commit 3b38781

Please sign in to comment.