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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ad90d58
fix(cli): add explicit support for Jest CLI arguments
alicewriteswrongs Jun 28, 2022
2b01727
add support for a CLI argument which might have a string or number
alicewriteswrongs Jun 28, 2022
7f1ef5f
iron out a little thing w/ string vs number
alicewriteswrongs Jun 28, 2022
1df876c
change to silly fake args since those are now included as jest args
alicewriteswrongs Jun 28, 2022
9eca7dc
prettier
alicewriteswrongs Jun 28, 2022
a1d7b66
fix issue with some flags being double-included
alicewriteswrongs Jun 28, 2022
5187584
alphabetize those
alicewriteswrongs Jun 28, 2022
b63f57e
review feedback
alicewriteswrongs Jun 28, 2022
723df32
add support for scientific notation
alicewriteswrongs Jun 28, 2022
695d3d7
add a comment to explain that
alicewriteswrongs Jun 28, 2022
030b699
fmt!!!!
alicewriteswrongs Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 134 additions & 3 deletions src/cli/config-flags.ts
View file
Open in desktop
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 •
edited
Loading

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

Copy link

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
Contributor

Choose a reason for hiding this comment

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

@deleonio FWIW, outputFile is listed here - https://github.com/ionic-team/stencil/pull/3444/files#diff-020f86cbdec56ce2dcab01f41db9a6f8ec09c1f6c081b45bff6ca4661ddcf315R125

'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
View file
Open in desktop
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