Skip to content
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

fix(cli): add explicit support for Jest CLI arguments #3444

Merged
merged 11 commits into from
Jun 28, 2022
137 changes: 134 additions & 3 deletions src/cli/config-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,65 @@ export const BOOLEAN_CLI_ARGS = [
'verbose',
'version',
'watch',

// JEST CLI OPTIONS
Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs Jun 28, 2022

Choose a reason for hiding this comment

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

I separated these with a little comment heading for ease of editing later if / when Jest's supported arguments change

Choose a reason for hiding this comment

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

please add outputFile

Copy link
Member

Choose a reason for hiding this comment

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

'all',
'automock',
'bail',
// 'cache', Stencil already supports this argument
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some of these options are dupes of options that Stencil already supports. I thought we might want to keep them in the list here just so that the list is, in the code at least, a complete list of the Jest CLI arguments. Open to just deleting these commented-out lines as well though!

'changedFilesWithAncestor',
// 'ci', Stencil already supports this argument
'clearCache',
'clearMocks',
'collectCoverage',
'color',
'colors',
'coverage',
// 'debug', Stencil already supports this argument
'detectLeaks',
'detectOpenHandles',
'errorOnDeprecated',
'expand',
'findRelatedTests',
'forceExit',
'init',
'injectGlobals',
'json',
'lastCommit',
'listTests',
'logHeapUsage',
'noStackTrace',
'notify',
'onlyChanged',
'onlyFailures',
'passWithNoTests',
'resetMocks',
'resetModules',
'restoreMocks',
'runInBand',
'runTestsByPath',
'showConfig',
'silent',
'skipFilter',
'testLocationInResults',
'updateSnapshot',
'useStderr',
// 'verbose', Stencil already supports this argument
// 'version', Stencil already supports this argument
// 'watch', Stencil already supports this argument
'watchAll',
'watchman',
] as const;

/**
* All the Number options supported by the Stencil CLI
*/
export const NUMBER_CLI_ARGS = ['maxWorkers', 'port'] as const;
export const NUMBER_CLI_ARGS = [
'port',
// JEST CLI ARGS
'maxConcurrency',
'testTimeout',
] as const;

/**
* All the String options supported by the Stencil CLI
Expand All @@ -53,8 +106,71 @@ export const STRING_CLI_ARGS = [
'emulate',
'root',
'screenshotConnector',

// JEST CLI ARGS
'cacheDirectory',
'changedSince',
'collectCoverageFrom',
// 'config', Stencil already supports this argument
'coverageDirectory',
'coverageThreshold',
'env',
'filter',
'globalSetup',
'globalTeardown',
'globals',
'haste',
'moduleNameMapper',
'notifyMode',
'outputFile',
'preset',
'prettierPath',
'resolver',
'rootDir',
'runner',
'testEnvironment',
'testEnvironmentOptions',
'testFailureExitCode',
'testNamePattern',
'testResultsProcessor',
'testRunner',
'testSequencer',
'testURL',
'timers',
'transform',

// ARRAY ARGS
'collectCoverageOnlyFrom',
'coveragePathIgnorePatterns',
'coverageReporters',
'moduleDirectories',
'moduleFileExtensions',
'modulePathIgnorePatterns',
'modulePaths',
'projects',
'reporters',
'roots',
'selectProjects',
'setupFiles',
'setupFilesAfterEnv',
'snapshotSerializers',
'testMatch',
'testPathIgnorePatterns',
'testPathPattern',
'testRegex',
'transformIgnorePatterns',
'unmockedModulePathPatterns',
'watchPathIgnorePatterns',
] as const;

/**
* All the CLI arguments which may have string or number values
*
* `maxWorkers` is an argument which is used both by Stencil _and_ by Jest,
* which means that we need to support parsing both string and number values.
*/
export const STRING_NUMBER_CLI_ARGS = ['maxWorkers'] as const;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maxWorkers can according to the Jest docs be either a string or a number, so we need to introduce a new argument type to support that.


/**
* All the LogLevel-type options supported by the Stencil CLI
*
Expand All @@ -74,9 +190,10 @@ type ArrayValuesAsUnion<T extends ReadonlyArray<string>> = T[number];
export type BooleanCLIArg = ArrayValuesAsUnion<typeof BOOLEAN_CLI_ARGS>;
export type StringCLIArg = ArrayValuesAsUnion<typeof STRING_CLI_ARGS>;
export type NumberCLIArg = ArrayValuesAsUnion<typeof NUMBER_CLI_ARGS>;
export type StringNumberCLIArg = ArrayValuesAsUnion<typeof STRING_NUMBER_CLI_ARGS>;
export type LogCLIArg = ArrayValuesAsUnion<typeof LOG_LEVEL_CLI_ARGS>;

type KnownCLIArg = BooleanCLIArg | StringCLIArg | NumberCLIArg | LogCLIArg;
type KnownCLIArg = BooleanCLIArg | StringCLIArg | NumberCLIArg | StringNumberCLIArg | LogCLIArg;

type AliasMap = Partial<Record<KnownCLIArg, string>>;

Expand Down Expand Up @@ -107,16 +224,25 @@ type ObjectFromKeys<K extends ReadonlyArray<string>, T> = {
* in ConfigFlags, below
*/
type BooleanConfigFlags = ObjectFromKeys<typeof BOOLEAN_CLI_ARGS, boolean>;

/**
* Type containing the possible String configuration flags, to be included
* in ConfigFlags, below
*/
type StringConfigFlags = ObjectFromKeys<typeof STRING_CLI_ARGS, string>;

/**
* Type containing the possible numeric configuration flags, to be included
* in ConfigFlags, below
*/
type NumberConfigFlags = ObjectFromKeys<typeof NUMBER_CLI_ARGS, number>;

/**
* Type containing the configuration flags which may be set to either string
* or number values.
*/
type StringNumberConfigFlags = ObjectFromKeys<typeof STRING_NUMBER_CLI_ARGS, string | number>;

/**
* Type containing the possible LogLevel configuration flags, to be included
* in ConfigFlags, below
Expand All @@ -137,7 +263,12 @@ type LogLevelFlags = ObjectFromKeys<typeof LOG_LEVEL_CLI_ARGS, LogLevel>;
* options we support and a runtime list of strings which can be used to match
* on actual flags passed by the user.
*/
export interface ConfigFlags extends BooleanConfigFlags, StringConfigFlags, NumberConfigFlags, LogLevelFlags {
export interface ConfigFlags
extends BooleanConfigFlags,
StringConfigFlags,
NumberConfigFlags,
StringNumberConfigFlags,
LogLevelFlags {
task?: TaskCommand | null;
args?: string[];
knownArgs?: string[];
Expand Down
64 changes: 58 additions & 6 deletions src/cli/parse-flags.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { CompilerSystem, LogLevel, LOG_LEVELS, TaskCommand } from '../declarations';
import { dashToPascalCase, toDashCase } from '@utils';
import {
ConfigFlags,
BOOLEAN_CLI_ARGS,
BooleanCLIArg,
LogCLIArg,
NumberCLIArg,
StringCLIArg,
CLI_ARG_ALIASES,
BOOLEAN_CLI_ARGS,
ConfigFlags,
LOG_LEVEL_CLI_ARGS,
LogCLIArg,
NUMBER_CLI_ARGS,
NumberCLIArg,
STRING_CLI_ARGS,
STRING_NUMBER_CLI_ARGS,
StringCLIArg,
StringNumberCLIArg,
} from './config-flags';

/**
Expand Down Expand Up @@ -73,6 +75,7 @@ const parseArgs = (flags: ConfigFlags, args: string[]) => {
BOOLEAN_CLI_ARGS.forEach((argName) => parseBooleanArg(flags, args, argName));
STRING_CLI_ARGS.forEach((argName) => parseStringArg(flags, args, argName));
NUMBER_CLI_ARGS.forEach((argName) => parseNumberArg(flags, args, argName));
STRING_NUMBER_CLI_ARGS.forEach((argName) => parseStringNumberArg(flags, args, argName));
LOG_LEVEL_CLI_ARGS.forEach((argName) => parseLogLevelArg(flags, args, argName));
};

Expand Down Expand Up @@ -161,6 +164,52 @@ const parseNumberArg = (flags: ConfigFlags, args: string[], configCaseName: Numb
}
};

/**
* Parse a CLI argument which may be either a string or a number
*
* @param flags the config flags object, while we'll modify
* @param args our CLI arguments
* @param configCaseName the argument we want to look at right now
*/
const parseStringNumberArg = (flags: ConfigFlags, args: string[], configCaseName: StringNumberCLIArg) => {
if (!['number', 'string'].includes(typeof flags[configCaseName])) {
flags[configCaseName] = null;
}

const { value, matchingArg } = getValue(args, configCaseName);

if (value !== undefined && matchingArg !== undefined) {
if (CLI_ARG_STRING_REGEX.test(value)) {
// if it matches the regex we treat it like a string
flags[configCaseName] = value;
} else {
// it was a number, great!
flags[configCaseName] = Number(value);
}
flags.knownArgs!.push(matchingArg);
flags.knownArgs!.push(value);
}
};

/**
* We use this regular expression to detect CLI parameters which
* should be parsed as string values (as opposed to numbers) for
* the argument types for which we support both a string and a
* number value.
*
* The regex tests for the presence of at least one character which is
* _not_ a digit (`\d`), a period (`\.`), or one of the characters `"e"`,
* `"E"`, `"+"`, or `"-"` (the latter four characters are necessary to
* support the admittedly unlikely use of scientific notation, like `"4e+0"`
* for `4`).
*
* Thus we'll match a string like `"50%"`, but not a string like `"50"` or
* `"5.0"`. If it matches a given string we conclude that the string should
* be parsed as a string literal, rather than using `Number` to convert it
* to a number.
*/
const CLI_ARG_STRING_REGEX = /[^\d\.Ee\+\-]+/g;

/**
* Parse a LogLevel CLI argument. These can be only a specific
* set of strings, so this function takes care of validating that
Expand Down Expand Up @@ -202,7 +251,10 @@ const parseLogLevelArg = (flags: ConfigFlags, args: string[], configCaseName: Lo
* @returns the value for the flag as well as the exact string which it matched from
* the user input.
*/
const getValue = (args: string[], configCaseName: StringCLIArg | NumberCLIArg | LogCLIArg): CLIArgValue => {
const getValue = (
args: string[],
configCaseName: StringCLIArg | NumberCLIArg | StringNumberCLIArg | LogCLIArg
): CLIArgValue => {
// for some CLI args we have a short alias, like 'c' for 'config'
const alias = CLI_ARG_ALIASES[configCaseName];
// we support supplying arguments in both dash-case and configCase
Expand Down
Loading