Skip to content

Commit 20f74fc

Browse files
fix(testing): perform string -> boolean type casting for Jest config (#5672)
This adds some manual string -> boolean type casting to the code that we use to convert CLI flags to a Jest config (after parsing them with yargs). This fixes an issue documented in #5640 where boolean CLI flags like `--watchAll` were being passed to Jest as strings, instead of as booleans, so that it was not possible to _disable_ a flag by doing `--flagName=false` (the value of the flag would be set to the string `"false"` which is a truthy value). There was also a somewhat related issue with not properly parsing boolean flags in our CLI parser which was recently fixed in #5646. Note that to get this working it was necessary to make some module resolution-related changes in a few places since there's now a dependency between the `testing/` and `cli/` modules (`testing/` now imports and using `BOOLEAN_CLI_FLAGS`). This involved a `paths` alias in the TypeScript config, esbuild- and rollup-specific module mapping configuration / aliases, and a change in the Jest config to allow `@stencil/core/cli` to be resolved in all these contexts. fixes #5640 STENCIL-1259
1 parent dcbaaf3 commit 20f74fc

File tree

12 files changed

+78
-3
lines changed

12 files changed

+78
-3
lines changed

jest.config.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ module.exports = {
44
'@app-globals': '<rootDir>/internal/app-globals/index.cjs',
55
'@platform': '<rootDir>/internal/testing/index.js',
66
'@runtime': '<rootDir>/internal/testing/index.js',
7-
'@stencil/core/cli': '<rootDir>/cli/index.js',
7+
'@stencil/core/cli': '<rootDir>/cli/index.cjs',
88
'@stencil/core/compiler': '<rootDir>/compiler/stencil.js',
99
'@stencil/core/mock-doc': '<rootDir>/mock-doc/index.cjs',
1010
'@stencil/core/testing': '<rootDir>/testing/index.js',

scripts/bundles/plugins/alias-plugin.ts

+3
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ export function aliasPlugin(opts: BuildOptions): Plugin {
5959
if (id === '@sys-api-node') {
6060
return join(opts.buildDir, 'sys', 'node', 'index.js');
6161
}
62+
if (id === '@stencil/core/cli') {
63+
return join(opts.buildDir, 'cli', 'index.js');
64+
}
6265
if (helperResolvers.has(id)) {
6366
return join(opts.bundleHelpersDir, `${id}.js`);
6467
}

scripts/esbuild/testing.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ export async function buildTesting(opts: BuildOptions) {
4949
'../compiler/stencil.js',
5050
];
5151

52+
const aliases = getEsbuildAliases();
53+
// we want to point at the cjs module here because we're building cjs
54+
aliases['@stencil/core/cli'] = './cli/index.cjs';
55+
5256
const testingEsbuildOptions: ESBuildOptions = {
5357
...getBaseEsbuildOptions(),
5458
entryPoints: [join(sourceDir, 'index.ts')],
@@ -63,7 +67,7 @@ export async function buildTesting(opts: BuildOptions) {
6367
* in `lazyRequirePlugin` and modify the imports
6468
*/
6569
write: false,
66-
alias: getEsbuildAliases(),
70+
alias: aliases,
6771
banner: { js: getBanner(opts, `Stencil Testing`, true) },
6872
plugins: [
6973
externalAlias('@app-data', '@stencil/core/internal/app-data'),

scripts/esbuild/util.ts

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export function getEsbuildAliases(): Record<string, string> {
2121
'@stencil/core/dev-server': './dev-server/index.js',
2222
'@stencil/core/mock-doc': './mock-doc/index.cjs',
2323
'@stencil/core/internal/testing': './internal/testing/index.js',
24+
'@stencil/core/cli': './cli/index.js',
2425
'@sys-api-node': './sys/node/index.js',
2526

2627
// mapping node.js module names to `sys/node` "cache"

src/cli/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
export { ConfigFlags } from './config-flags';
1+
export { BOOLEAN_CLI_FLAGS, ConfigFlags } from './config-flags';
22
export { parseFlags } from './parse-flags';
33
export { run, runTask } from './run';

src/testing/jest/jest-27-and-under/jest-config.ts

+7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/internal';
34
import { isString } from '@utils';
45

@@ -85,6 +86,12 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
8586
jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
8687
}
8788

89+
for (const flag of BOOLEAN_CLI_FLAGS) {
90+
if (typeof jestArgv[flag] === 'string') {
91+
jestArgv[flag] = jestArgv[flag] === 'true';
92+
}
93+
}
94+
8895
return jestArgv;
8996
}
9097

src/testing/jest/jest-27-and-under/test/jest-config.spec.ts

+15
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/declarations';
34
import { mockValidatedConfig } from '@stencil/core/testing';
45
import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
2021
expect(jestArgv.maxWorkers).toBe(2);
2122
});
2223

24+
// the 'help' and 'version' flags are problematic with yargs (they cause a
25+
// hang) and we intercept those flags and do our own thing before turning
26+
// over to jest anyway
27+
const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
28+
describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
29+
it('converts boolean flag to boolean', () => {
30+
const args = ['test', `--${cliArg}`];
31+
const config = mockValidatedConfig();
32+
config.flags = parseFlags(args);
33+
const jestArgv = buildJestArgv(config);
34+
expect(jestArgv[cliArg]).toBe(true);
35+
});
36+
});
37+
2338
it('marks outputFile as a Jest argument', () => {
2439
const args = ['test', '--ci', '--outputFile=path/to/my-file'];
2540
const config = mockValidatedConfig();

src/testing/jest/jest-28/jest-config.ts

+7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/internal';
34
import { isString } from '@utils';
45

@@ -49,6 +50,12 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
4950
jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
5051
}
5152

53+
for (const flag of BOOLEAN_CLI_FLAGS) {
54+
if (typeof jestArgv[flag] === 'string') {
55+
jestArgv[flag] = jestArgv[flag] === 'true';
56+
}
57+
}
58+
5259
return jestArgv;
5360
}
5461

src/testing/jest/jest-28/test/jest-config.spec.ts

+15
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/declarations';
34
import { mockValidatedConfig } from '@stencil/core/testing';
45
import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
2021
expect(jestArgv.maxWorkers).toBe(2);
2122
});
2223

24+
// the 'help' and 'version' flags are problematic with yargs (they cause a
25+
// hang) and we intercept those flags and do our own thing before turning
26+
// over to jest anyway
27+
const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
28+
describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
29+
it('converts boolean flag to boolean', () => {
30+
const args = ['test', `--${cliArg}`];
31+
const config = mockValidatedConfig();
32+
config.flags = parseFlags(args);
33+
const jestArgv = buildJestArgv(config);
34+
expect(jestArgv[cliArg]).toBe(true);
35+
});
36+
});
37+
2338
it('marks outputFile as a Jest argument', () => {
2439
const args = ['test', '--ci', '--outputFile=path/to/my-file'];
2540
const config = mockValidatedConfig();

src/testing/jest/jest-29/jest-config.ts

+7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/internal';
34
import { isString } from '@utils';
45

@@ -49,6 +50,12 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
4950
jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
5051
}
5152

53+
for (const flag of BOOLEAN_CLI_FLAGS) {
54+
if (typeof jestArgv[flag] === 'string') {
55+
jestArgv[flag] = jestArgv[flag] === 'true';
56+
}
57+
}
58+
5259
return jestArgv;
5360
}
5461

src/testing/jest/jest-29/test/jest-config.spec.ts

+15
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Config } from '@jest/types';
2+
import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
23
import type * as d from '@stencil/core/declarations';
34
import { mockValidatedConfig } from '@stencil/core/testing';
45
import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
2021
expect(jestArgv.maxWorkers).toBe(2);
2122
});
2223

24+
// the 'help' and 'version' flags are problematic with yargs (they cause a
25+
// hang) and we intercept those flags and do our own thing before turning
26+
// over to jest anyway
27+
const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
28+
describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
29+
it('converts boolean flag to boolean', () => {
30+
const args = ['test', `--${cliArg}`];
31+
const config = mockValidatedConfig();
32+
config.flags = parseFlags(args);
33+
const jestArgv = buildJestArgv(config);
34+
expect(jestArgv[cliArg]).toBe(true);
35+
});
36+
});
37+
2338
it('marks outputFile as a Jest argument', () => {
2439
const args = ['test', '--ci', '--outputFile=path/to/my-file'];
2540
const config = mockValidatedConfig();

tsconfig.json

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"@stencil/core/internal/testing": ["src/testing/platform/index.ts"],
3838
"@stencil/core/mock-doc": ["src/mock-doc/index.ts"],
3939
"@stencil/core/testing": ["src/testing/index.ts"],
40+
"@stencil/core/cli": ["src/cli/index.ts"],
4041
"@sys-api-node": ["src/sys/node/index.ts"],
4142
"@utils": ["src/utils/index.ts"],
4243
"@utils/shadow-css": ["src/utils/shadow-css"]

0 commit comments

Comments
 (0)