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): refactor CLI argument parser #3765

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Conversation

alicewriteswrongs
Copy link
Contributor

@alicewriteswrongs alicewriteswrongs commented Oct 25, 2022

Jest supports passing certain arguments multiple times. For instance, in the following example:

jest --coverage --reporters="default" --reporters="jest-junit"

all of the values for the --reporters flag ("default" and "jest-junit") should be collected into an array of values, instead of simply recording whichever value is farther to the right (Stencil's behavior before this commit).

To support passing such arguments to the stencil test subcommand this commit adds a new recursive-descent parser in src/cli/parse-flags.ts to replace the somewhat ad-hoc approach that was there previously. It parses the following grammar:

CLIArgments     → ""
                | CLITerm ( " " CLITerm )* ;
CLITerm         → EqualsArg
                | AliasEqualsArg
                | AliasArg
                | NegativeDashArg
                | NegativeArg
                | SimpleArg ;
EqualsArg       → "--" ArgName "=" CLIValue ;
AliasEqualsArg  → "-" AliasName "=" CLIValue ;
AliasArg        → "-" AliasName ( " " CLIValue )? ;
NegativeDashArg → "--no-" ArgName ;
NegativeArg     → "--no" ArgName ;
SimpleArg       → "--" ArgName ( " " CLIValue )? ;
ArgName         → /^[a-zA-Z-]+$/ ;
AliasName       → /^[a-z]{1}$/ ;
CLIValue        → ('"')? /^[a-zA-Z0-9]+$/ ('"')? ;

(the regexes are a little fuzzy, but this is sort of an informal presentation).

Refactoring this to use a proper parser (albeit a pretty simple one) allows our implementation to much more clearly conform to this defined grammar, and should hopefully both help us avoid other bugs in the future and be easier to maintain.

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

GitHub Issue Number: #3712

Passing flags like the Jest flag --reporters which should accept multiple values, like

stencil test --coverage --reporters="default" --reporters="jest-junit"

doesn't work at present. Instead of bundling together the two inputs into an array like ["default", "jest-junit"] instead the rightmost one just wins :/

What is the new behavior?

This refactors the parser for the CLI, starting by defining a grammar for CLI flags and values and implementing a recursive-descent parser (not a very deep one 😄) which parses that grammar and, as it traverses the arguments, sets values as we intend on the ConfigFlags object.

Does this introduce a breaking change?

  • Yes
  • No

Testing

I confirmed that this works with the provided reproduction case. It should be tested pretty exhaustively! Please find the bugs!!! 🐛🐞

Other information

@alicewriteswrongs alicewriteswrongs requested a review from a team as a code owner October 25, 2022 16:00
@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2022

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 2161 errors on this branch.

That's 2 fewer than on main! 🎉🎉🎉

reports and statistics

Our most error-prone files
Path Error Count
src/compiler/config/test/validate-dev-server.spec.ts 45
src/compiler/config/test/validate-output-www.spec.ts 40
src/screenshot/connector-base.ts 40
src/dev-server/index.ts 38
src/mock-doc/serialize-node.ts 36
src/compiler/sys/tests/in-memory-fs.spec.ts 34
src/screenshot/screenshot-compare.ts 33
src/compiler/transformers/test/parse-props.spec.ts 32
src/dev-server/server-process.ts 32
src/runtime/vdom/vdom-render.ts 31
src/compiler/sys/typescript/typescript-config.ts 28
src/compiler/build/build-stats.ts 27
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts 26
src/compiler/prerender/prerender-main.ts 25
src/sys/node/test/worker-manager.spec.ts 24
src/compiler/style/test/optimize-css.spec.ts 23
src/compiler/sys/in-memory-fs.ts 23
src/runtime/update-component.ts 23
src/compiler/config/test/validate-paths.spec.ts 22
src/utils/test/message-utils.spec.ts 21
Our most common errors
Typescript Error Code Count Error messages
TS2532 646
Error messages Object is possibly 'undefined'.
TS2345 620
Error messages Argument of type 'CompilerSystem | undefined' is not assignable to parameter of type 'CompilerSystem'.
Type 'undefined' is not assignable to type 'CompilerSystem'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type 'Promise<(() => void) | null>' is not assignable to parameter of type 'Promise<() => void>'.

Argument of type 'null' is not assignable to parameter of type 'BuildResultsComponentGraph'.

Argument of type 'DevServerConfig | undefined' is not assignable to parameter of type 'StencilDevServerConfig'.
Type 'undefined' is not assignable to type 'StencilDevServerConfig'.

Argument of type 'Promise<(() => void) | null>' is not assignable to parameter of type 'Promise<() => void>'.
Type '(() => void) | null' is not assignable to type '() => void'.
Type 'null' is not assignable to type '() => void'.

Argument of type 'ComponentRuntimeHostListener[] | undefined' is not assignable to parameter of type 'ComponentRuntimeHostListener[]'.
Type 'undefined' is not assignable to type 'ComponentRuntimeHostListener[]'.

Argument of type 'HTMLScriptElement | null | undefined' is not assignable to parameter of type 'HTMLScriptElement'.
Type 'undefined' is not assignable to type 'HTMLScriptElement'.

Argument of type 'string | null' is not assignable to parameter of type 'string'.
Type 'null' is not assignable to type 'string'.

Argument of type 'Logger | undefined' is not assignable to parameter of type 'Logger'.
Type 'undefined' is not assignable to type 'Logger'.

Argument of type 'string[] | undefined' is not assignable to parameter of type 'string[]'.
Type 'undefined' is not assignable to type 'string[]'.

Argument of type 'string' is not assignable to parameter of type 'never'.

Argument of type 'Diagnostic[] | undefined' is not assignable to parameter of type 'readonly Diagnostic[]'.
Type 'undefined' is not assignable to type 'readonly Diagnostic[]'.

Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.

Argument of type 'BuildConditionals | undefined' is not assignable to parameter of type 'BuildConditionals'.
Type 'undefined' is not assignable to type 'BuildConditionals'.

Argument of type 'Diagnostic[] | undefined' is not assignable to parameter of type 'Diagnostic[]'.
Type 'undefined' is not assignable to type 'Diagnostic[]'.

Argument of type 'WorkerMeta | undefined' is not assignable to parameter of type 'WorkerMeta'.
Type 'undefined' is not assignable to type 'WorkerMeta'.

Argument of type 'CopyTask[] | undefined' is not assignable to parameter of type 'boolean | CopyTask[]'.

Argument of type 'CopyTask[] | undefined' is not assignable to parameter of type 'boolean | CopyTask[]'.
Type 'undefined' is not assignable to type 'boolean | CopyTask[]'.

Argument of type 'OutputTargetWww' is not assignable to parameter of type 'never'.

Argument of type 'string | null | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type 'Set<string | null>' is not assignable to parameter of type 'Set'.
Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Argument of type 'Set<string | null>' is not assignable to parameter of type 'Set'.

Argument of type 'RollupConfig | undefined' is not assignable to parameter of type 'RollupConfig'.
Type 'undefined' is not assignable to type 'RollupConfig'.

Argument of type '(string[] | undefined)[]' is not assignable to parameter of type 'string[][]'.
Type 'string[] | undefined' is not assignable to type 'string[]'.
Type 'undefined' is not assignable to type 'string[]'.

Argument of type 'ComponentCompilerMeta | undefined' is not assignable to parameter of type 'ComponentCompilerMeta'.
Type 'undefined' is not assignable to type 'ComponentCompilerMeta'.

Argument of type 'true | Object | undefined' is not assignable to parameter of type 'boolean | Object | null'.
Type 'undefined' is not assignable to type 'boolean | Object | null'.

Argument of type 'SourceTarget | undefined' is not assignable to parameter of type 'SourceTarget'.
Type 'undefined' is not assignable to type 'SourceTarget'.

Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
Type 'undefined' is not assignable to type 'boolean'.

Argument of type '(string | undefined)[]' is not assignable to parameter of type 'string[]'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type '(string | undefined)[]' is not assignable to parameter of type 'string[]'.

Argument of type 'RollupSourceMap | undefined' is not assignable to parameter of type 'SourceMap'.
Type 'undefined' is not assignable to type 'SourceMap'.

Argument of type 'EntryModule | undefined' is not assignable to parameter of type 'EntryModule'.
Type 'undefined' is not assignable to type 'EntryModule'.

Argument of type '(Module | undefined)[]' is not assignable to parameter of type 'Module[]'.
Type 'Module | undefined' is not assignable to type 'Module'.
Type 'undefined' is not assignable to type 'Module'.

Argument of type 'Document | null' is not assignable to parameter of type 'Document'.
Type 'null' is not assignable to type 'Document'.

Argument of type 'Document | null' is not assignable to parameter of type 'Node | MockNode'.
Type 'null' is not assignable to type 'Node | MockNode'.

Argument of type 'HydrateAnchorElement' is not assignable to parameter of type '{ [attrName: string]: string; }'.
'string' index signatures are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type 'URL | null' is not assignable to parameter of type 'URL'.
Type 'null' is not assignable to type 'URL'.

Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
Type 'undefined' is not assignable to type 'number'.

Argument of type 'SitemapXmpResults | null' is not assignable to parameter of type 'SitemapXmpResults'.
Type 'null' is not assignable to type 'SitemapXmpResults'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | URL'.

Argument of type 'Map<string, string[]> | undefined' is not assignable to parameter of type 'Map<string, string[]>'.
Type 'undefined' is not assignable to type 'Map<string, string[]>'.

Argument of type 'undefined' is not assignable to parameter of type 'string'.

Argument of type 'ServiceWorkerConfig | undefined' is not assignable to parameter of type 'ServiceWorkerConfig'.
Type 'undefined' is not assignable to type 'ServiceWorkerConfig'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | URL'.
Type 'undefined' is not assignable to type 'string | URL'.

Argument of type 'CssNode[] | null | undefined' is not assignable to parameter of type 'void | CssNode[]'.

Argument of type 'CssNode[] | null | undefined' is not assignable to parameter of type 'void | CssNode[]'.
Type 'null' is not assignable to type 'void | CssNode[]'.

Argument of type 'null' is not assignable to parameter of type 'string'.

Argument of type 'CompilerSystemCreateDirectoryOptions | undefined' is not assignable to parameter of type 'CompilerSystemCreateDirectoryOptions'.
Type 'undefined' is not assignable to type 'CompilerSystemCreateDirectoryOptions'.

Argument of type 'null' is not assignable to parameter of type 'CompilerFileWatcherEvent'.

Argument of type 'null' is not assignable to parameter of type '{ access: (filePath: string) => Promise; accessSync: (filePath: string) => boolean; cancelDeleteDirectoriesFromDisk: (dirPaths: string[]) => void; cancelDeleteFilesFromDisk: (filePaths: string[]) => void; ... 17 more ...; writeFiles: (files: { ...; } | Map<...>, opts?: FsWriteOptions | undefined) => Promise...'.

Argument of type 'undefined' is not assignable to parameter of type 'TypeChecker'.

Argument of type 'CollectionBundleManifest[] | undefined' is not assignable to parameter of type 'any[]'.
Type 'undefined' is not assignable to type 'any[]'.

Argument of type 'Module | undefined' is not assignable to parameter of type 'Module'.
Type 'undefined' is not assignable to type 'Module'.

Argument of type 'NodeArray | undefined' is not assignable to parameter of type 'NodeArray | HeritageClause[]'.
Type 'undefined' is not assignable to type 'NodeArray | HeritageClause[]'.

Argument of type 'Block | undefined' is not assignable to parameter of type 'Block'.
Type 'undefined' is not assignable to type 'Block'.

Argument of type 'undefined' is not assignable to parameter of type 'readonly ParameterDeclaration[]'.

Argument of type 'Expression | undefined' is not assignable to parameter of type 'Expression'.
Type 'undefined' is not assignable to type 'Expression'.

Argument of type 'Symbol | undefined' is not assignable to parameter of type 'Symbol'.
Type 'undefined' is not assignable to type 'Symbol'.

Argument of type 'TypeNode | undefined' is not assignable to parameter of type 'Node'.
Type 'undefined' is not assignable to type 'Node'.

Argument of type '(PropertyAssignment | null)[]' is not assignable to parameter of type 'readonly ObjectLiteralElementLike[]'.
Type 'PropertyAssignment | null' is not assignable to type 'ObjectLiteralElementLike'.
Type 'null' is not assignable to type 'ObjectLiteralElementLike'.

Argument of type 'Signature | undefined' is not assignable to parameter of type 'Signature'.
Type 'undefined' is not assignable to type 'Signature'.

Argument of type '(PropertyAssignment | null)[]' is not assignable to parameter of type 'readonly ObjectLiteralElementLike[]'.

Argument of type 'Identifier | undefined' is not assignable to parameter of type 'Node'.
Type 'undefined' is not assignable to type 'Node'.

Argument of type 'undefined' is not assignable to parameter of type 'readonly ClassElement[]'.

Argument of type 'Config | null | undefined' is not assignable to parameter of type 'Config'.
Type 'undefined' is not assignable to type 'Config'.

Argument of type 'null' is not assignable to parameter of type 'CollectionCompilerMeta'.

Argument of type 'PropertyName | undefined' is not assignable to parameter of type 'PropertyName'.
Type 'undefined' is not assignable to type 'PropertyName'.

Argument of type 'NamedExportBindings | undefined' is not assignable to parameter of type 'Node'.
Type 'undefined' is not assignable to type 'Node'.

Argument of type 'Declaration | undefined' is not assignable to parameter of type 'Node'.
Type 'undefined' is not assignable to type 'Node'.

Argument of type 'Identifier | undefined' is not assignable to parameter of type 'string | BindingName'.
Type 'undefined' is not assignable to type 'string | BindingName'.

Argument of type 'ScriptTarget | undefined' is not assignable to parameter of type 'ScriptTarget | CreateSourceFileOptions'.
Type 'undefined' is not assignable to type 'ScriptTarget | CreateSourceFileOptions'.

Argument of type 'Diagnostic' is not assignable to parameter of type 'DiagnosticWithLocation'.
Types of property 'file' are incompatible.
Type 'SourceFile | undefined' is not assignable to type 'SourceFile'.
Type 'undefined' is not assignable to type 'SourceFile'.

Argument of type '(...pathSegments: string[]) => string | undefined' is not assignable to parameter of type '(...args: string[]) => string'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type 'OpenInEditorCallback | undefined' is not assignable to parameter of type 'OpenInEditorCallback'.
Type 'undefined' is not assignable to type 'OpenInEditorCallback'.

Argument of type 'CompilerWatcher | undefined' is not assignable to parameter of type 'CompilerWatcher'.
Type 'undefined' is not assignable to type 'CompilerWatcher'.

Argument of type '{ editor: string | undefined; }' is not assignable to parameter of type 'OpenInEditorOptions'.
Types of property 'editor' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Argument of type 'string | undefined' is not assignable to parameter of type 'PathLike'.

Argument of type 'DevServerConfig | undefined' is not assignable to parameter of type 'DevServerConfig'.
Type 'undefined' is not assignable to type 'DevServerConfig'.

Argument of type 'CompilerBuildResults | undefined' is not assignable to parameter of type 'CompilerBuildResults'.
Type 'undefined' is not assignable to type 'CompilerBuildResults'.

Argument of type 'Window | undefined' is not assignable to parameter of type 'Window'.
Type 'undefined' is not assignable to type 'Window'.

Argument of type 'SerializeDocumentOptions | undefined' is not assignable to parameter of type 'HydrateDocumentOptions'.
Type 'undefined' is not assignable to type 'HydrateDocumentOptions'.

Argument of type 'HydrateDocumentOptions | undefined' is not assignable to parameter of type 'HydrateDocumentOptions'.
Type 'undefined' is not assignable to type 'HydrateDocumentOptions'.

Argument of type 'MockNode | null' is not assignable to parameter of type 'MockNode'.
Type 'null' is not assignable to type 'MockNode'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | null'.
Type 'undefined' is not assignable to type 'string | null'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | null'.

Argument of type 'ShadowRoot | null' is not assignable to parameter of type 'Node'.
Type 'null' is not assignable to type 'Node'.

Argument of type 'MockElement' is not assignable to parameter of type 'MockHTMLElement'.
Types of property 'namespaceURI' are incompatible.
Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Argument of type 'null' is not assignable to parameter of type 'string | Node'.

Argument of type 'null' is not assignable to parameter of type 'string | boolean | undefined'.

Argument of type '(Window & typeof globalThis) | null' is not assignable to parameter of type 'Window'.
Type 'null' is not assignable to type 'Window'.

Argument of type 'string | null | undefined' is not assignable to parameter of type 'string | undefined'.

Argument of type 'HostRef | undefined' is not assignable to parameter of type 'HostRef'.
Type 'undefined' is not assignable to type 'HostRef'.

Argument of type 'this' is not assignable to parameter of type 'HostElement'.
Type 'HostElement' is not assignable to type 'import("/home/runner/work/stencil/stencil/src/declarations/stencil-private").HostElement'.
The types returned by 'componentOnReady()' are incompatible between these types.
Type 'Promise | undefined' is not assignable to type 'Promise'.
Type 'undefined' is not assignable to type 'Promise'.

Argument of type 'this' is not assignable to parameter of type 'HostElement'.

Argument of type 'string | null' is not assignable to parameter of type 'string | undefined'.

Argument of type 'PropertyDescriptor | undefined' is not assignable to parameter of type 'PropertyDescriptor & ThisType'.
Type 'undefined' is not assignable to type 'PropertyDescriptor & ThisType'.
Type 'undefined' is not assignable to type 'PropertyDescriptor'.

Argument of type 'HTMLElement | undefined' is not assignable to parameter of type 'HTMLElement'.
Type 'undefined' is not assignable to type 'HTMLElement'.

Argument of type 'HostElement | undefined' is not assignable to parameter of type 'HostElement'.
Type 'undefined' is not assignable to type 'HostElement'.

Argument of type 'HostElement | undefined' is not assignable to parameter of type 'EventTarget'.
Type 'undefined' is not assignable to type 'EventTarget'.

Argument of type 'HostElement | undefined' is not assignable to parameter of type 'Element'.
Type 'undefined' is not assignable to type 'Element'.

Argument of type 'VNode[] | undefined' is not assignable to parameter of type 'VNode[]'.
Type 'undefined' is not assignable to type 'VNode[]'.

Argument of type 'null | undefined' is not assignable to parameter of type 'ChildType'.
Type 'undefined' is not assignable to type 'ChildType'.

Argument of type 'VNode | null' is not assignable to parameter of type 'ChildType'.
Type 'null' is not assignable to type 'ChildType'.

Argument of type 'VNode | undefined' is not assignable to parameter of type 'ChildType'.
Type 'undefined' is not assignable to type 'ChildType'.

Argument of type 'VNode | null | undefined' is not assignable to parameter of type 'ChildType'.
Type 'undefined' is not assignable to type 'ChildType'.

Argument of type 'null | undefined' is not assignable to parameter of type 'ChildType'.

Argument of type 'RenderNode | undefined' is not assignable to parameter of type 'Node | null'.
Type 'undefined' is not assignable to type 'Node | null'.

Argument of type 'VNode | undefined' is not assignable to parameter of type 'VNode'.
Type 'undefined' is not assignable to type 'VNode'.

Argument of type 'null' is not assignable to parameter of type 'VNode'.

Argument of type 'RenderNode | undefined' is not assignable to parameter of type 'Node'.
Type 'undefined' is not assignable to type 'Node'.

Argument of type 'null' is not assignable to parameter of type 'RenderNode'.

Argument of type 'ScreenshotDiff | undefined' is not assignable to parameter of type 'ScreenshotDiff'.
Type 'undefined' is not assignable to type 'ScreenshotDiff'.

Argument of type 'string | undefined' is not assignable to parameter of type 'PathLike'.
Type 'undefined' is not assignable to type 'PathLike'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | SemVer'.
Type 'undefined' is not assignable to type 'string | SemVer'.

Argument of type 'string | undefined' is not assignable to parameter of type 'string | SemVer'.

Argument of type 'null' is not assignable to parameter of type 'number | undefined'.

Argument of type 'null' is not assignable to parameter of type 'number | PromiseLike'.

Argument of type 'CompilerWorkerTask | undefined' is not assignable to parameter of type 'CompilerWorkerTask'.
Type 'undefined' is not assignable to type 'CompilerWorkerTask'.

Argument of type 'null' is not assignable to parameter of type 'CompilerWorkerTask'.

Argument of type 'undefined' is not assignable to parameter of type 'ErrorHandler'.

Argument of type 'string | undefined' is not assignable to parameter of type 'MockRequestInfo'.
Type 'undefined' is not assignable to type 'MockRequestInfo'.

Argument of type 'string | undefined' is not assignable to parameter of type 'MockRequestInfo'.

Argument of type 'MockResponse | undefined' is not assignable to parameter of type 'MockResponse'.
Type 'undefined' is not assignable to type 'MockResponse'.

Argument of type 'RuntimeRef | undefined' is not assignable to parameter of type 'RuntimeRef'.
Type 'undefined' is not assignable to type 'RuntimeRef'.

Argument of type 'this' is not assignable to parameter of type 'E2EElementInternal'.
Type 'E2EElement' is not assignable to type 'E2EElementInternal'.
Types of property 'find' are incompatible.
Type '(selector: string) => Promise<E2EElement | null>' is not assignable to type '(selector: FindSelector) => Promise'.

Argument of type 'string | null | undefined' is not assignable to parameter of type 'SerializableOrJSHandle'.
Type 'undefined' is not assignable to type 'SerializableOrJSHandle'.

Argument of type 'this' is not assignable to parameter of type 'E2EElementInternal'.

Argument of type 'ElementHandle | null' is not assignable to parameter of type 'ElementHandle'.
Type 'null' is not assignable to type 'ElementHandle'.

Argument of type '{ viewport: EmulateViewport | undefined; userAgent: string | undefined; }' is not assignable to parameter of type '{ viewport: Viewport; userAgent: string; }'.
Types of property 'viewport' are incompatible.
Type 'EmulateViewport | undefined' is not assignable to type 'Viewport'.
Type 'undefined' is not assignable to type 'Viewport'.

Argument of type 'null' is not assignable to parameter of type 'SourceMap | undefined'.
TS2322 539
Error messages Type 'string | null | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'null' is not assignable to type 'DevServerConfig | undefined'.

Type 'boolean | null | undefined' is not assignable to type 'boolean | undefined'.
Type 'null' is not assignable to type 'boolean | undefined'.

Type 'null' is not assignable to type 'DevServer'.

Type 'undefined' is not assignable to type 'ComponentConstructor | Promise'.

Type 'HTMLElement | null' is not assignable to type 'HTMLElement'.
Type 'null' is not assignable to type 'HTMLElement'.

Type '({ type: "chunk"; fileName: string; map: SourceMap | undefined; code: string; moduleFormat: ModuleFormat | undefined; entryKey: string; imports: string[]; isEntry: boolean; ... 4 more ...; content?: undefined; } | { ...; })[]' is not assignable to type 'RollupResult[]'.
Type '{ type: "chunk"; fileName: string; map: SourceMap | undefined; code: string; moduleFormat: ModuleFormat | undefined; entryKey: string; imports: string[]; isEntry: boolean; ... 4 more ...; content?: undefined; } | { ...; }' is not assignable to type 'RollupResult'.
Type '{ type: "chunk"; fileName: string; map: SourceMap | undefined; code: string; moduleFormat: ModuleFormat | undefined; entryKey: string; imports: string[]; isEntry: boolean; ... 4 more ...; content?: undefined; }' is not assignable to type 'RollupResult'.
Type '{ type: "chunk"; fileName: string; map: SourceMap | undefined; code: string; moduleFormat: ModuleFormat | undefined; entryKey: string; imports: string[]; isEntry: boolean; ... 4 more ...; content?: undefined; }' is not assignable to type 'RollupChunkResult'.
Types of property 'moduleFormat' are incompatible.
Type 'ModuleFormat | undefined' is not assignable to type 'ModuleFormat'.
Type 'undefined' is not assignable to type 'ModuleFormat'.

Type 'null' is not assignable to type 'CompilerBuildResults'.

Type 'undefined' is not assignable to type 'string'.

Type 'null' is not assignable to type 'string'.

Type 'undefined' is not assignable to type 'Document'.

Type 'null' is not assignable to type 'Promise'.

Type 'null' is not assignable to type 'LoggerTimeSpan'.

Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'boolean | undefined' is not assignable to type 'boolean'.

Type 'number | undefined' is not assignable to type 'number'.
Type 'undefined' is not assignable to type 'number'.

Type 'boolean | "prod" | undefined' is not assignable to type 'boolean | "prod"'.
Type 'undefined' is not assignable to type 'boolean | "prod"'.

Type 'BuildResultsComponentGraph | undefined' is not assignable to type 'BuildResultsComponentGraph'.
Type 'undefined' is not assignable to type 'BuildResultsComponentGraph'.

Type 'RollupResults | undefined' is not assignable to type 'RollupResults'.
Type 'undefined' is not assignable to type 'RollupResults'.

Type '{ name: string | undefined; source: string; tags: any[]; }[]' is not assignable to type '{ name: string; source: string; tags: string[]; }[]'.
Type '{ name: string | undefined; source: string; tags: any[]; }' is not assignable to type '{ name: string; source: string; tags: string[]; }'.
Types of property 'name' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'null' is not assignable to type 'CompilerWorkerContext'.

Type 'null' is not assignable to type 'WatchOfConfigFile'.

Type 'null' is not assignable to type 'Promise<(void | void[])[]>'.

Type 'Promise<(void | void[] | null)[]>' is not assignable to type 'Promise<(void | void[])[]>'.
Type '(void | void[] | null)[]' is not assignable to type '(void | void[])[]'.
Type 'void | void[] | null' is not assignable to type 'void | void[]'.
Type 'null' is not assignable to type 'void | void[]'.

Type 'null' is not assignable to type '{ program: WatchOfConfigFile; rebuild: () => void; }'.

Type '(this: PluginContext, importee: string, importer: string) => Promise' is not assignable to type 'ResolveIdHook'.
Types of parameters 'importer' and 'importer' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type '(this: PluginContext, importee: string, importer: string) => Promise' is not assignable to type 'ResolveIdHook'.
Types of parameters 'importer' and 'importer' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Type 'null' is not assignable to type 'string | undefined'.

Type 'string' is not assignable to type 'never'.

Type 'null' is not assignable to type 'never'.

Type 'never[]' is not assignable to type 'never'.

Type '{ code: string; exports: string[]; workerMsgId: string; dependencies: string[]; } | null' is not assignable to type 'WorkerMeta | undefined'.
Type 'null' is not assignable to type 'WorkerMeta | undefined'.

Type 'WorkerMeta | undefined' is not assignable to type 'WorkerMeta'.
Type 'undefined' is not assignable to type 'WorkerMeta'.

Type 'CompilerSystem | undefined' is not assignable to type 'CompilerSystem'.
Type 'undefined' is not assignable to type 'CompilerSystem'.

Type 'Logger | undefined' is not assignable to type 'Logger'.
Type 'undefined' is not assignable to type 'Logger'.

Type 'import("/home/runner/work/stencil/stencil/src/compiler/cache").Cache' is not assignable to type 'import("/home/runner/work/stencil/stencil/src/declarations/stencil-private").Cache'.
The types returned by 'get(...)' are incompatible between these types.
Type 'Promise<string | null>' is not assignable to type 'Promise'.

Type 'null' is not assignable to type 'ValidatedConfig'.
Type 'null' is not assignable to type 'Config'.

Type 'null' is not assignable to type 'string[]'.

Type '{ dir: string; buildDir: string; collectionDir: string | null; typesDir: string; esmLoaderPath: string; copy: d.CopyTask[]; polyfills: boolean | undefined; empty: boolean; transformAliasedImportPathsInCollection: boolean; type: "dist"; }' is not assignable to type 'Required'.
Types of property 'polyfills' are incompatible.
Type 'boolean | undefined' is not assignable to type 'boolean'.

Type 'string | undefined' is not assignable to type 'never'.
Type 'undefined' is not assignable to type 'never'.

Type 'boolean | undefined' is not assignable to type 'never'.
Type 'undefined' is not assignable to type 'never'.

Type 'boolean' is not assignable to type 'never'.

Type 'CopyTask[]' is not assignable to type 'never'.

Type 'null' is not assignable to type 'number | undefined'.

Type 'null' is not assignable to type 'HistoryApiFallback | undefined'.

Type 'null' is not assignable to type 'CopyTask[] | undefined'.

Type '{ dirPath: string; filePath: string; fileName: string; readmePath: string; usagesDir: string; tag: string; readme: string | undefined; overview: string; usage: JsonDocsUsage; ... 13 more ...; listeners: JsonDocsListener[]; }[]' is not assignable to type 'JsonDocsComponent[]'.
Type '{ dirPath: string; filePath: string; fileName: string; readmePath: string; usagesDir: string; tag: string; readme: string | undefined; overview: string; usage: d.JsonDocsUsage; docs: string; ... 12 more ...; listeners: d.JsonDocsListener[]; }' is not assignable to type 'JsonDocsComponent'.
Types of property 'readme' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'string[] | undefined' is not assignable to type 'string[]'.
Type 'undefined' is not assignable to type 'string[]'.

Type '{ name: string; type: string; mutable: boolean; attr: string | undefined; reflectToAttr: boolean; docs: string; docsTags: CompilerJsDocTagInfo[]; default: string | undefined; deprecation: string | undefined; values: JsonDocsValue[]; optional: boolean; required: boolean; }[]' is not assignable to type 'JsonDocsProp[]'.
Type '{ name: string; type: string; mutable: boolean; attr: string | undefined; reflectToAttr: boolean; docs: string; docsTags: d.CompilerJsDocTagInfo[]; default: string | undefined; deprecation: string | undefined; values: d.JsonDocsValue[]; optional: boolean; required: boolean; }' is not assignable to type 'JsonDocsProp'.
Types of property 'default' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type '{ name: string; type: string; mutable: false; attr: string; reflectToAttr: false; docs: string; docsTags: never[]; default: undefined; deprecation: undefined; values: JsonDocsValue[]; optional: true; required: false; }[]' is not assignable to type 'JsonDocsProp[]'.
Type '{ name: string; type: string; mutable: false; attr: string; reflectToAttr: false; docs: string; docsTags: never[]; default: undefined; deprecation: undefined; values: d.JsonDocsValue[]; optional: true; required: false; }' is not assignable to type 'JsonDocsProp'.
Types of property 'default' are incompatible.
Type 'undefined' is not assignable to type 'string'.

Type '(ComponentCompilerMeta | undefined)[]' is not assignable to type 'ComponentCompilerMeta[]'.
Type 'ComponentCompilerMeta | undefined' is not assignable to type 'ComponentCompilerMeta'.
Type 'undefined' is not assignable to type 'ComponentCompilerMeta'.

Type '(ComponentCompilerMeta | undefined)[][]' is not assignable to type 'readonly ComponentCompilerMeta[][]'.
Type '(ComponentCompilerMeta | undefined)[]' is not assignable to type 'ComponentCompilerMeta[]'.

Type 'null' is not assignable to type 'boolean | SourceMapOptions | undefined'.

Type 'null' is not assignable to type 'SourceMap | undefined'.

Type '{ name: string | undefined; tags: string[]; }[]' is not assignable to type 'CollectionDependencyData[]'.
Type '{ name: string | undefined; tags: string[]; }' is not assignable to type 'CollectionDependencyData'.
Types of property 'name' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'string | undefined' is not assignable to type 'string | number'.
Type 'undefined' is not assignable to type 'string | number'.

Type 'SourceMap | undefined' is not assignable to type 'SourceMap'.
Type 'undefined' is not assignable to type 'SourceMap'.

Type 'string | null' is not assignable to type 'string | undefined'.
Type 'null' is not assignable to type 'string | undefined'.

Type 'null' is not assignable to type 'Function'.

Type 'null' is not assignable to type 'Map<string, string[]>'.

Type 'null' is not assignable to type 'PrerenderConfig'.

Type 'null' is not assignable to type 'Set'.

Type 'null' is not assignable to type 'HydrateAnchorElement[]'.

Type 'null' is not assignable to type '(prerenderRequest: PrerenderUrlRequest) => Promise'.

Type 'CssNode | null' is not assignable to type 'void | CssNode'.
Type 'null' is not assignable to type 'void | CssNode'.

Type 'CssNode | null' is not assignable to type 'CssNode'.
Type 'null' is not assignable to type 'CssNode'.

Type 'CssNode | null' is not assignable to type 'void | CssNode'.

Type 'RegExpExecArray | null' is not assignable to type 'RegExpExecArray'.
Type 'null' is not assignable to type 'RegExpExecArray'.

Type 'null' is not assignable to type '() => string'.

Type 'null' is not assignable to type 'number'.

Type 'null' is not assignable to type 'boolean'.

Type 'null' is not assignable to type 'boolean | undefined'.

Type 'FsItem | undefined' is not assignable to type 'FsItem'.
Type 'undefined' is not assignable to type 'FsItem'.

Type 'null' is not assignable to type 'CompilerFileWatcherCallback[]'.

Type '(key: string) => string | undefined' is not assignable to type '(key: string) => string'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type '(p: string) => Promise<string | undefined>' is not assignable to type '{ (p: string): Promise; (p: string, encoding: "utf8"): Promise; (p: string, encoding: "binary"): Promise; }'.
Type 'Promise<string | undefined>' is not assignable to type 'Promise'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type '(p: string) => string | undefined' is not assignable to type '(p: string, encoding?: string | undefined) => string'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type '((maxConcurrentWorkers: number) => WorkerMainController) | null' is not assignable to type '((maxConcurrentWorkers: number) => WorkerMainController) | undefined'.
Type 'null' is not assignable to type '((maxConcurrentWorkers: number) => WorkerMainController) | undefined'.

Type 'null' is not assignable to type 'ResolvedModuleWithFailedLookupLocations'.

Type 'null' is not assignable to type 'Worker'.

Type 'CollectionCompilerMeta | undefined' is not assignable to type 'CollectionCompilerMeta'.
Type 'undefined' is not assignable to type 'CollectionCompilerMeta'.

Type 'DeclarationName | undefined' is not assignable to type 'DeclarationName'.
Type 'undefined' is not assignable to type 'DeclarationName'.

Type 'null' is not assignable to type 'PropertyAssignment'.

Type 'boolean | null' is not assignable to type 'boolean'.
Type 'null' is not assignable to type 'boolean'.

Type 'undefined' is not assignable to type 'ComponentCompilerVirtualProperty'.

Type '{ name: string; method: string; bubbles: boolean; cancelable: boolean; composed: boolean; docs: CompilerJsDoc; complexType: ComponentCompilerEventComplexType; internal: boolean | undefined; }[]' is not assignable to type 'ComponentCompilerEvent[]'.
Type '{ name: string; method: string; bubbles: boolean; cancelable: boolean; composed: boolean; docs: d.CompilerJsDoc; complexType: d.ComponentCompilerEventComplexType; internal: boolean | undefined; }' is not assignable to type 'ComponentCompilerEvent'.
Types of property 'internal' are incompatible.
Type 'boolean | undefined' is not assignable to type 'boolean'.

Type '{ name: string; docs: CompilerJsDoc; complexType: ComponentCompilerMethodComplexType; internal: boolean | undefined; }[]' is not assignable to type 'ComponentCompilerMethod[]'.
Type '{ name: string; docs: d.CompilerJsDoc; complexType: d.ComponentCompilerMethodComplexType; internal: boolean | undefined; }' is not assignable to type 'ComponentCompilerMethod'.
Types of property 'internal' are incompatible.
Type 'boolean | undefined' is not assignable to type 'boolean'.

Type '{ name: string; type: ComponentCompilerPropertyType; attribute: string | undefined; reflect: boolean; mutable: boolean; required: boolean; optional: boolean; defaultValue: string | undefined; complexType: ComponentCompilerPropertyComplexType; docs: CompilerJsDoc; internal: boolean | undefined; }[]' is not assignable to type 'ComponentCompilerProperty[]'.
Type '{ name: string; type: d.ComponentCompilerPropertyType; attribute: string | undefined; reflect: boolean; mutable: boolean; required: boolean; optional: boolean; defaultValue: string | undefined; complexType: d.ComponentCompilerPropertyComplexType; docs: d.CompilerJsDoc; internal: boolean | undefined; }' is not assignable to type 'ComponentCompilerProperty'.
Types of property 'internal' are incompatible.
Type 'boolean | undefined' is not assignable to type 'boolean'.

Type 'null' is not assignable to type 'ImportData'.

Type 'undefined' is not assignable to type '"queryparams" | null'.

Type 'null' is not assignable to type 'WeakSet'.

Type 'null' is not assignable to type 'Module'.

Type 'Module | undefined' is not assignable to type 'Module'.
Type 'undefined' is not assignable to type 'Module'.

Type 'undefined' is not assignable to type 'CompilerJsDoc'.

Type '{ [x: string]: d.TypesMemberNameData[] | undefined; }' is not assignable to type 'TypesImportData'.
'string' index signatures are incompatible.
Type 'TypesMemberNameData[] | undefined' is not assignable to type 'TypesMemberNameData[]'.
Type 'undefined' is not assignable to type 'TypesMemberNameData[]'.

Type 'string | undefined' is not assignable to type 'string | null'.

Type '((data: { file: string; line: number; column: number; }) => void) | null' is not assignable to type 'OpenInEditorCallback | undefined'.
Type 'null' is not assignable to type 'OpenInEditorCallback | undefined'.

Type 'null' is not assignable to type 'WebSocket'.

Type 'null' is not assignable to type 'BuildOnEventRemove'.

Type 'null' is not assignable to type '() => void'.

Type 'null' is not assignable to type '(msg: DevServerMessage) => void'.

Type 'DevServerConfig | undefined' is not assignable to type 'DevServerConfig'.
Type 'undefined' is not assignable to type 'DevServerConfig'.

Type 'null' is not assignable to type '{ open(openId: string): Promise; }'.

Type 'string | null' is not assignable to type 'string | undefined'.

Type 'null' is not assignable to type 'Promise<DevServerEditor[]>'.

Type 'null' is not assignable to type '(req: IncomingMessage, res: ServerResponse, next: () => void) => void'.

Type 'null' is not assignable to type 'URL'.

Type 'null' is not assignable to type 'URLSearchParams'.

Type 'PageReloadStrategy | undefined' is not assignable to type 'PageReloadStrategy'.
Type 'undefined' is not assignable to type 'PageReloadStrategy'.

Type 'null' is not assignable to type 'Server<typeof IncomingMessage, typeof ServerResponse>'.

Type 'null' is not assignable to type 'DevWebSocket'.

Type 'null' is not assignable to type 'DevServerContext'.

Type 'DevWebSocket | null' is not assignable to type 'DevWebSocket'.
Type 'null' is not assignable to type 'DevWebSocket'.

Type 'null' is not assignable to type 'ChildProcess'.

Type 'null' is not assignable to type 'HydrateApp'.

Type '() => undefined' is not assignable to type '{ (cb?: (() => void) | undefined): TestServerResponse; (chunk: any, cb?: (() => void) | undefined): TestServerResponse; (chunk: any, encoding: BufferEncoding, cb?: (() => void) | undefined): TestServerResponse; }'.
Type 'undefined' is not assignable to type 'TestServerResponse'.

Type 'null' is not assignable to type 'ComponentConstructor'.

Type '(this: HostElement) => Promise | undefined' is not assignable to type '() => Promise'.
Type 'Promise | undefined' is not assignable to type 'Promise'.
Type 'undefined' is not assignable to type 'Promise'.

Type 'null' is not assignable to type 'Window & typeof globalThis'.
Type 'null' is not assignable to type 'Window'.

Type 'null' is not assignable to type 'Window & typeof globalThis'.

Type 'null' is not assignable to type 'MockCSSStyleSheet'.

Type 'null' is not assignable to type 'string | boolean'.

Type 'null' is not assignable to type 'Location'.

Type 'null' is not assignable to type 'MockElement'.

Type 'null' is not assignable to type 'SVGSVGElement'.

Type 'null' is not assignable to type 'SVGElement'.

Type 'null' is not assignable to type 'EventTarget'.

Type 'null' is not assignable to type 'MockEventListener[]'.

Type 'null' is not assignable to type 'MockDocument'.

Type '{ name: string; value: string; namespace: string | null; prefix: null; }[]' is not assignable to type 'Attribute[]'.
Type '{ name: string; value: string; namespace: string | null; prefix: null; }' is not assignable to type 'Attribute'.
Types of property 'namespace' are incompatible.
Type 'string | null' is not assignable to type 'string | undefined'.

Type '(element: MockElement) => string | null' is not assignable to type '(element: unknown) => string'.
Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Type 'ParentNode | null' is not assignable to type 'Node'.
Type 'null' is not assignable to type 'Node'.

Type 'null' is not assignable to type 'Document'.

Type 'string | null | undefined' is not assignable to type 'string | undefined'.

Type 'never[] | null' is not assignable to type 'RenderNode[]'.
Type 'null' is not assignable to type 'RenderNode[]'.

Type 'null' is not assignable to type 'VNode[]'.

Type 'null' is not assignable to type 'string | number | undefined'.

Type 'null' is not assignable to type 'string | number | Function'.

Type 'RenderNode | null' is not assignable to type 'RenderNode'.
Type 'null' is not assignable to type 'RenderNode'.

Type 'HTMLElement | null' is not assignable to type 'EventTarget'.
Type 'null' is not assignable to type 'EventTarget'.

Type 'string | null | undefined' is not assignable to type 'string | undefined'.
Type 'null' is not assignable to type 'string | undefined'.

Type 'null' is not assignable to type 'Set | undefined'.

Type 'null' is not assignable to type '[string, any][] | undefined'.

Type 'VNode[] | undefined' is not assignable to type 'VNode[]'.
Type 'undefined' is not assignable to type 'VNode[]'.

Type '(props: {}, children: VNode[], utils: FunctionalUtilities) => null' is not assignable to type 'FunctionalComponent<{}>'.
Type 'null' is not assignable to type 'VNode | VNode[]'.

Type '() => null' is not assignable to type 'FunctionalComponent<{}>'.
Type 'null' is not assignable to type 'VNode | VNode[]'.

Type 'null' is not assignable to type 'VNode'.

Type 'undefined' is not assignable to type 'VNode'.

Type 'RenderNode | undefined' is not assignable to type 'RenderNode'.
Type 'undefined' is not assignable to type 'RenderNode'.

Type 'RelocateNodeData | undefined' is not assignable to type 'RelocateNodeData'.
Type 'undefined' is not assignable to type 'RelocateNodeData'.

Type 'ChildNode | null' is not assignable to type 'Node'.
Type 'null' is not assignable to type 'Node'.

Type 'null' is not assignable to type 'ScreenshotBuild'.

Type 'null' is not assignable to type 'ScreenshotCache'.

Type 'null' is not assignable to type 'Buffer'.

Type 'null' is not assignable to type 'Screenshot'.

Type '(() => void) | undefined' is not assignable to type '() => any'.
Type 'undefined' is not assignable to type '() => any'.

Type '() => null' is not assignable to type '(opts: { rootDir: string; moduleId: string; path: string; }) => string'.
Type 'null' is not assignable to type 'string'.

Type '() => null' is not assignable to type '(opts: { moduleId: string; path?: string | undefined; version?: string | undefined; }) => string'.
Type 'null' is not assignable to type 'string'.

Type '() => string | undefined' is not assignable to type '() => string'.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

Type 'number | null' is not assignable to type 'number'.
Type 'null' is not assignable to type 'number'.

Type 'any[] | undefined' is not assignable to type 'any[]'.
Type 'undefined' is not assignable to type 'any[]'.

Type 'boolean | null | undefined' is not assignable to type 'boolean | undefined'.

Type 'null' is not assignable to type 'ConfigBundle[] | undefined'.

Type 'null' is not assignable to type 'OutputTarget[] | undefined'.

Type 'null' is not assignable to type 'TestingConfig | undefined'.

Type 'null' is not assignable to type 'Cache'.

Type 'null' is not assignable to type '{ access: (filePath: string) => Promise; accessSync: (filePath: string) => boolean; cancelDeleteDirectoriesFromDisk: (dirPaths: string[]) => void; cancelDeleteFilesFromDisk: (filePaths: string[]) => void; ... 17 more ...; writeFiles: (files: { ...; } | Map<...>, opts?: FsWriteOptions | undefined) => Promise...'.

Type 'HostRef | undefined' is not assignable to type 'HostRef'.
Type 'undefined' is not assignable to type 'HostRef'.

Type 'Function | undefined' is not assignable to type 'Function'.
Type 'undefined' is not assignable to type 'Function'.

Type 'RafCallback | undefined' is not assignable to type 'Function'.
Type 'undefined' is not assignable to type 'Function'.

Type 'QueuedLoadModule | undefined' is not assignable to type 'QueuedLoadModule'.
Type 'undefined' is not assignable to type 'QueuedLoadModule'.

Type 'null' is not assignable to type 'ElementHandle'.

Type 'null' is not assignable to type 'E2EPageInternal'.

Type 'ElementHandle | null' is not assignable to type 'ElementHandle'.
Type 'null' is not assignable to type 'ElementHandle'.

Type 'null' is not assignable to type 'Promise<JSHandle>'.

Type '(a?: any, b?: any) => Promise<ScreenshotDiff | undefined>' is not assignable to type '{ (): Promise; (description: string): Promise; (opts: ScreenshotOptions): Promise<...>; (description: string, opts: ScreenshotOptions): Promise<...>; }'.
Type 'Promise<ScreenshotDiff | undefined>' is not assignable to type 'Promise'.
Type 'ScreenshotDiff | undefined' is not assignable to type 'ScreenshotDiff'.
Type 'undefined' is not assignable to type 'ScreenshotDiff'.

Type 'null' is not assignable to type 'CompilerWatcher'.

Type 'null' is not assignable to type 'Promise'.

Type 'Browser | null' is not assignable to type 'Browser'.
Type 'null' is not assignable to type 'Browser'.

Type 'null' is not assignable to type 'ValidatedConfig'.

Type 'null' is not assignable to type 'Browser'.

Type 'null' is not assignable to type 'SourceMap'.

Type 'null' is not assignable to type 'PackageJsonData'.

Type 'null' is not assignable to type '{ [moduleId: string]: string; } | undefined'.
TS2531 203
Error messages Object is possibly 'null'.
TS2454 46
Error messages Variable 'pkgJsonData' is used before being assigned.

Variable 'minifyOpts' is used before being assigned.

Variable 'workerCtrl' is used before being assigned.

Variable 'timespan' is used before being assigned.

Variable 'content' is used before being assigned.

Variable 'compilerExe' is used before being assigned.

Variable 'outputText' is used before being assigned.

Variable 'importResolvedFile' is used before being assigned.

Variable 'win' is used before being assigned.

Variable 'attrName' is used before being assigned.

Variable 'oldValue' is used before being assigned.

Variable 'newValue' is used before being assigned.

Variable 'hostId' is used before being assigned.

Variable 'promise' is used before being assigned.

Variable 'textContent' is used before being assigned.

Variable 'resolve' is used before being assigned.

Variable 'opts' is used before being assigned.
TS2722 41
Error messages Cannot invoke an object which is possibly 'undefined'.
TS2352 19
Error messages Conversion of type 'null' to type 'CompilerSystem' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.

Conversion of type 'null' to type 'string' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.

Conversion of type 'null' to type 'string[]' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.

Conversion of type '{ cmps: never[]; }' to type 'Module' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
Type '{ cmps: never[]; }' is missing the following properties from type 'Module': coreRuntimeApis, collectionName, dtsFilePath, excludeFromCollection, and 27 more.

Conversion of type 'null' to type 'ScreenshotCache' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
TS2769 12
Error messages No overload matches this call.
Overload 1 of 2, '(type: keyof DocumentEventMap, listener: (this: Document, ev: PointerEvent | MouseEvent | UIEvent | Event | ErrorEvent | ... 13 more ... | WheelEvent) => any, options?: boolean | ... 1 more ... | undefined): void', gave the following error.
Argument of type '"e"' is not assignable to parameter of type 'keyof DocumentEventMap'.
Overload 2 of 2, '(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions | undefined): void', gave the following error.
Argument of type 'null' is not assignable to parameter of type 'EventListenerOrEventListenerObject'.

No overload matches this call.
Overload 1 of 2, '(o: {}): string[]', gave the following error.
Argument of type 'BuildResultsComponentGraph | undefined' is not assignable to parameter of type '{}'.
Type 'undefined' is not assignable to type '{}'.
Overload 2 of 2, '(o: object): string[]', gave the following error.
Argument of type 'BuildResultsComponentGraph | undefined' is not assignable to parameter of type 'object'.
Type 'undefined' is not assignable to type 'object'.

No overload matches this call.
Overload 1 of 4, '(object: {}, method: never): SpyInstance<never, never>', gave the following error.
Argument of type 'CompilerWorkerContext | undefined' is not assignable to parameter of type '{}'.
Type 'undefined' is not assignable to type '{}'.
Overload 2 of 4, '(object: {}, method: never): SpyInstance<never, never>', gave the following error.
Argument of type 'CompilerWorkerContext | undefined' is not assignable to parameter of type '{}'.
Type 'undefined' is not assignable to type '{}'.

No overload matches this call.
The last overload gave the following error.
Argument of type 'string | null' is not assignable to parameter of type '(substring: string, ...args: any[]) => string'.
Type 'null' is not assignable to type '(substring: string, ...args: any[]) => string'.

No overload matches this call.
The last overload gave the following error.
Argument of type 'string | undefined' is not assignable to parameter of type 'string | RegExp'.
Type 'undefined' is not assignable to type 'string | RegExp'.

No overload matches this call.
Overload 1 of 2, '(configFileName: string, optionsToExtend: CompilerOptions | undefined, system: System, createProgram?: CreateProgram | undefined, reportDiagnostic?: DiagnosticReporter | undefined, reportWatchStatus?: WatchStatusReporter | undefined, watchOptionsToExtend?: WatchOptions | undefined, extraFileExtensions?: readonly FileExtensionInfo[] | undefined): WatchCompilerHostOfConfigFile<...>', gave the following error.
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.
Overload 2 of 2, '(rootFiles: string[], options: CompilerOptions, system: System, createProgram?: CreateProgram | undefined, reportDiagnostic?: DiagnosticReporter | undefined, reportWatchStatus?: WatchStatusReporter | undefined, projectReferences?: readonly ProjectReference[] | undefined, watchOptions?: WatchOptions | undefined): WatchCompilerHostOfFilesAndCompilerOptions<...>', gave the following error.
Argument of type 'string | undefined' is not assignable to parameter of type 'string[]'.
Type 'undefined' is not assignable to type 'string[]'.

No overload matches this call.
Overload 1 of 2, '(timeoutId: string | number | Timeout | undefined): void', gave the following error.
Argument of type 'Timeout | null' is not assignable to parameter of type 'string | number | Timeout | undefined'.
Type 'null' is not assignable to type 'string | number | Timeout | undefined'.
Overload 2 of 2, '(id?: number | undefined): void', gave the following error.
Argument of type 'Timeout | null' is not assignable to parameter of type 'number | undefined'.
Type 'null' is not assignable to type 'number | undefined'.

No overload matches this call.
Overload 1 of 2, '(timeoutId: string | number | Timeout | undefined): void', gave the following error.
Argument of type 'Timeout | null' is not assignable to parameter of type 'string | number | Timeout | undefined'.
Overload 2 of 2, '(id?: number | undefined): void', gave the following error.
Argument of type 'Timeout | null' is not assignable to parameter of type 'number | undefined'.

No overload matches this call.
Overload 1 of 3, '(p: string, encoding: "utf8"): Promise', gave the following error.
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.
Overload 2 of 3, '(p: string, encoding: "binary"): Promise', gave the following error.
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.

No overload matches this call.
Overload 1 of 2, '(values: [] | readonly unknown[]): Promise<unknown[] | []>', gave the following error.
Argument of type 'Promise[] | undefined' is not assignable to parameter of type '[] | readonly unknown[]'.
Type 'undefined' is not assignable to type '[] | readonly unknown[]'.
Overload 2 of 2, '(values: Iterable<void | PromiseLike>): Promise<void[]>', gave the following error.
Argument of type 'Promise[] | undefined' is not assignable to parameter of type 'Iterable<void | PromiseLike>'.
Type 'undefined' is not assignable to type 'Iterable<void | PromiseLike>'.
TS2790 10
Error messages The operand of a 'delete' operator must be optional.
TS2538 8
Error messages Type 'undefined' cannot be used as an index type.

Type 'null' cannot be used as an index type.
TS2416 4
Error messages Property 'get' in type 'Cache' is not assignable to the same property in base type 'Cache'.
Type '(key: string) => Promise<string | null>' is not assignable to type '(key: string) => Promise'.
Type 'Promise<string | null>' is not assignable to type 'Promise'.
Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Property 'getMemoryStats' in type 'Cache' is not assignable to the same property in base type 'Cache'.
Type '() => string | null' is not assignable to type '() => string'.
Type 'string | null' is not assignable to type 'string'.
Type 'null' is not assignable to type 'string'.

Property 'find' in type 'E2EElement' is not assignable to the same property in base type 'E2EElementInternal'.
Type '(selector: string) => Promise<E2EElement | null>' is not assignable to type '(selector: FindSelector) => Promise'.
Type 'Promise<E2EElement | null>' is not assignable to type 'Promise'.
Type 'E2EElement | null' is not assignable to type 'E2EElement'.
Type 'null' is not assignable to type 'E2EElement'.

Property 'findAll' in type 'E2EElement' is not assignable to the same property in base type 'E2EElementInternal'.
Type '(selector: string) => Promise<E2EElement[]>' is not assignable to type '(selector: FindSelector) => Promise<E2EElement[]>'.
Type 'Promise<import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-element").E2EElement[]>' is not assignable to type 'Promise<import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-declarations").E2EElement[]>'.
Type 'import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-element").E2EElement[]' is not assignable to type 'import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-declarations").E2EElement[]'.
Type 'import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-element").E2EElement' is not assignable to type 'import("/home/runner/work/stencil/stencil/src/testing/puppeteer/puppeteer-declarations").E2EElement'.
The types returned by 'find(...)' are incompatible between these types.
Type 'Promise<E2EElement | null>' is not assignable to type 'Promise'.
TS2533 3
Error messages Object is possibly 'null' or 'undefined'.
TS2493 3
Error messages Tuple type '[]' of length '0' has no element at index '0'.
TS2488 2
Error messages Type 'Diagnostic[] | undefined' must have a 'Symbol.iterator' method that returns an iterator.

Type 'string[] | undefined' must have a 'Symbol.iterator' method that returns an iterator.
TS2774 2
Error messages This condition will always return true since this function is always defined. Did you mean to call it instead?
TS2684 1
Error messages The 'this' context of type 'ResolveIdHook | undefined' is not assignable to method's 'this' of type 'Function'.
Type 'undefined' is not assignable to type 'Function'.
TS2464 1
Error messages A computed property name must be of type 'string', 'number', 'symbol', or 'any'.
TS2430 1
Error messages Interface 'SerializeOpts' incorrectly extends interface 'SerializeCssOptions'.
Types of property 'usedSelectors' are incompatible.
Type 'UsedSelectors | null' is not assignable to type 'UsedSelectors | undefined'.
Type 'null' is not assignable to type 'UsedSelectors | undefined'.

Unused exports report

There are 9 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports
File Line Identifier
src/screenshot/screenshot-fs.ts 18 readScreenshotData
src/testing/testing-utils.ts 186 withSilentWarn
src/compiler/app-core/app-data.ts 3 BUILD
src/compiler/app-core/app-data.ts 88 Env
src/compiler/app-core/app-data.ts 90 NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts 111 updateCacheFromRebuild
src/testing/platform/testing-platform.ts 29 cssVarShim
src/testing/puppeteer/puppeteer-declarations.ts 513 WaitForEventOptions
src/client/polyfills/css-shim/utils.ts 1 GLOBAL_SCOPE

Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs left a comment

Choose a reason for hiding this comment

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

just noting a few things

@@ -3,7 +3,7 @@ import type { LogLevel, TaskCommand } from '@stencil/core/declarations';
/**
* All the Boolean options supported by the Stencil CLI
*/
export const BOOLEAN_CLI_ARGS = [
export const BOOLEAN_CLI_FLAGS = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a bit of a 🤔 and thought these arrays should be arrays of flags rather than args, so I renamed...if we decide the new name isn't an improvement happy to back the change out!

Copy link
Member

Choose a reason for hiding this comment

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

I didn't comment on this the first time around, but I do think this is an improvement & am in favor of keeping the change 👍

c: 'config',
h: 'help',
p: 'port',
v: 'version',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reversed this object to make it easier to get the full config flag name from the alias.

* A regular expression which can be used to match a CLI flag for one of our
* short aliases.
*/
export const CLI_FLAG_REGEX = new RegExp(`^-[${Object.keys(CLI_FLAG_ALIASES).map((k) => k)}]{1}$`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since there's only four of these little CLI flags I figure it's not so hard to create an exhaustive regex to match them

const argsCopy = args.concat();
while (argsCopy.length > 0) {
// there are still unprocessed args to deal with
parseCLITerm(flags, argsCopy);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in parseCLITerm we use Array.prototype.shift to iterate through the arguments, so we make a copy here w/ .concat() so as not to rudely mutate something we're just borrowing

}

// we shouldn't get here if the input is well formed! so if we do we should
// raise an error with the malformed argument
Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs Oct 25, 2022

Choose a reason for hiding this comment

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

I have here (as well as below) some points noted where we'd want to raise an error since the parser has encountered something unexpected - however, at present we don't have any error reporting capability in this module, so wanted to raise it for discussion before I added it. It might be nice to not have this module depend on other parts of the codebase and be a little less coupled, so we could throw to signal an error. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine. What did you have in mind? An error message like:

Error - command line argument 'idkSomeBadFlag' could not be parsed.
Acceptable formats are:
-- list of what's allowed 

Or something different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah basically something like that, to just let the user that they're trying to use a flag which isn't supported

Copy link
Member

Choose a reason for hiding this comment

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

I think throw would be okay. I'm all for keeping the CLI decoupled from other bits of the code 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just pushed a change to add error reporting by throwing errors, lmk what you think!

const parseNumberArg = (flags: ConfigFlags, args: string[], configCaseName: NumberCLIArg) => {
if (typeof flags[configCaseName] !== 'number') {
flags[configCaseName] = null;
const setCLIArg = (flags: ConfigFlags, rawArg: string, normalizedArg: string, value: CLIValueResult) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is kind of a big function, I don't have a problem with that but these bits for handling each kind of argument could be pulled into separate functions if we want (doesn't make much difference I think).

Right now I have basically code for handling each case, points noted where we'd like to raise some sort of error, and then a void return for each case. Should this be a single big if / else if / else if instead?

the way this is set up currently doesn't lend itself super well to a switch statement, but we could add a classifyFlagName function which returned a value we could switch on if we wanted to go that route.

Copy link
Member

Choose a reason for hiding this comment

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

Right now I have basically code for handling each case, points noted where we'd like to raise some sort of error, and then a void return for each case. Should this be a single big if / else if / else if instead?

Yeah, I like this!

Copy link
Member

Choose a reason for hiding this comment

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

Usually I'd lean toward the path of small helper functions, but I think this file is already so big that just adds noise and makes the code harder to step through. So, the conditional logic (whatever shape it takes) works for me here

@@ -201,87 +320,41 @@ const parseStringNumberArg = (flags: ConfigFlags, args: string[], configCaseName
*/
const CLI_ARG_STRING_REGEX = /[^\d\.Ee\+\-]+/g;

export const Empty = Symbol('Empty');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially was bundling up two return values from the parseCLIValue function so I wanted to have something that was a little Option-ey, we could just use string | null as well I suppose.

Would we benefit in general from adding an Option-like type to use throughout the codebase? It's not terribly hard to get some of the key functionality together I think. It would be nice to have in situations like this.

Copy link
Member

Choose a reason for hiding this comment

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

I've never used a symbol manually defined like this, can you elaborate on the benefits (for my own curiosity)?

Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs Nov 3, 2022

Choose a reason for hiding this comment

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

I think I see a few advantages in it over just using string | null or something. For one, the value at runtime is not falsy, so a missed = in a check like if (value != Empty) won't ever return false as long as value is a string. It also explicitly conveys intent, like if I intend to convey an empty value I should be using something which is distinguished at the type level from a non-empty value — put another way, using "" to signal an empty string value is ambiguous, because we don't have any way to distinguish between an explicit input of an empty string and an actually missing value. If we use something like this symbol then we can keep an actually fully missing value separate, at both the type- and runtime-level, from any kind of existing value, whether it's an empty string or "foo" or "/path/to/file.foobar" or whatever.

Does that all make sense?

This is basically me just missing having access to Option which exists in Rust (and is very similar to Maybe from Haskell and other ML family languages)

Copy link
Member

@rwaskiewicz rwaskiewicz left a comment

Choose a reason for hiding this comment

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

All in all looks good! Mostly nits

* A regular expression which can be used to match a CLI flag for one of our
* short aliases.
*/
export const CLI_FLAG_REGEX = new RegExp(`^-[chpv]{1}$`);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could programmatically grab the keys from CLI_FLAG_ALIASES here so that we don't have to update this regex when we update the object above. Would something like this work?

Suggested change
export const CLI_FLAG_REGEX = new RegExp(`^-[chpv]{1}$`);
export const CLI_FLAG_REGEX = new RegExp(`^-[${Object.keys(CLI_FLAG_ALIASES).join('')}]{1}$`);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually had basically exactly that originally but I ended up having to change it because it caused a tree shaking issue, I think because then CLI_FLAG_REGEX had a dependency on CLI_FLAG_ALIASES so the treeshaking check that we do for the build verification broke 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which does introduce a bit of an unfortunate situation where we can't automatically keep the regex in sync with the defined aliases. If we have some precedent for adding an exception for the tree-shaking build verification thingy I'd be open to doing that

Copy link
Member

Choose a reason for hiding this comment

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

Gahh that's shame :-( We don't (have a precedent). I feel like we should put something on the books to document the treeshaking check a bit better. Sometimes I think about it as a black box that we always abide by. Stubbed out STENCIL-627 to do just that, but I don't think that should block this effort

@@ -28,6 +28,17 @@ export const dashToPascalCase = (str: string): string =>
.map((segment) => segment.charAt(0).toUpperCase() + segment.slice(1))
.join('');

/**
* Convert a string to 'configCase', which is basically PascalCase but with a lowercase first character.
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't that be camelCase? 😆

Can we use that jargon instead of creating our own?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, we already had this in here for some reason (I might have introduced it before though haha)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah "config case" is a new one for me haha

*
* This parses the following grammar:
*
* CLIArgments → ""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* CLIArgments ""
* CLIArguments ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙃

* SimpleArg → "--" ArgName ( " " CLIValue )? ;
* ArgName → /^[a-zA-Z-]+$/ ;
* AliasName → /^[a-z]{1}$/ ;
* CLIValue → '"' /^[a-zA-Z0-9]+$/ ('"')? ;
Copy link
Member

Choose a reason for hiding this comment

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

Should we update this to state it supports '-' for the case of --reporters="jest-junit"? Is there any actual limitation as to what characters we can put between the parens?

EDIT: You call that out in the line immediately after that there are additional constraints 👍

* SimpleArg → "--" ArgName ( " " CLIValue )? ;
* ArgName → /^[a-zA-Z-]+$/ ;
* AliasName → /^[a-z]{1}$/ ;
* CLIValue → '"' /^[a-zA-Z0-9]+$/ ('"')? ;
Copy link
Member

Choose a reason for hiding this comment

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

Does the question mark at the end imply that a key w/o a closing double quote is valid?

--reporters="something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mmmm I will disambiguate that

}

// we shouldn't get here if the input is well formed! so if we do we should
// raise an error with the malformed argument
Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine. What did you have in mind? An error message like:

Error - command line argument 'idkSomeBadFlag' could not be parsed.
Acceptable formats are:
-- list of what's allowed 

Or something different?

* @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
* @param flagName the flagname to normalize
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param flagName the flagname to normalize
* @param flagName the flag name to normalize


// AliasEqualsArg → "-" AliasName "=" CLIValue ;
if (arg.startsWith('-') && arg.includes('=')) {
// we're dealing with an EqualsArg, we have a special helper for that
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// we're dealing with an EqualsArg, we have a special helper for that
// we're dealing with an AliasEqualsArg, we have a special helper for that

if (arg.startsWith('-') && arg.includes('=')) {
// we're dealing with an EqualsArg, we have a special helper for that
const [originalArg, value] = parseEqualsArg(arg);
setCLIArg(flags, arg.split('=')[0], originalArg.replace(/^-/, ''), value);
Copy link
Member

Choose a reason for hiding this comment

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

Between here, https://github.com/ionic-team/stencil/pull/3765/files#diff-e6a96bc13cd5c047a16a1888b27e88af52903e89848de68b0072d32ea11b1208R134, and https://github.com/ionic-team/stencil/pull/3765/files#diff-e6a96bc13cd5c047a16a1888b27e88af52903e89848de68b0072d32ea11b1208R201 we have similar-ish leading hyphen removal code. I wonder if there's any value in trying to extract that out into a small helper function like:

const stripLeadingDashes(str: string): string => str.replace(/^-+/, '');

thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Can't those all just run through normalizeFlagName()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right you are! I updated things to just use normalizeFlagName and normalizeNegativeFlagName throughout

const parseNumberArg = (flags: ConfigFlags, args: string[], configCaseName: NumberCLIArg) => {
if (typeof flags[configCaseName] !== 'number') {
flags[configCaseName] = null;
const setCLIArg = (flags: ConfigFlags, rawArg: string, normalizedArg: string, value: CLIValueResult) => {
Copy link
Member

Choose a reason for hiding this comment

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

Right now I have basically code for handling each case, points noted where we'd like to raise some sort of error, and then a void return for each case. Should this be a single big if / else if / else if instead?

Yeah, I like this!

Copy link
Member

@tanner-reits tanner-reits left a comment

Choose a reason for hiding this comment

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

Overall really like the change to have a minimal parser! Started playing w/ this idea a while back in JEDI days (but using packages)

@@ -28,6 +28,17 @@ export const dashToPascalCase = (str: string): string =>
.map((segment) => segment.charAt(0).toUpperCase() + segment.slice(1))
.join('');

/**
* Convert a string to 'configCase', which is basically PascalCase but with a lowercase first character.
Copy link
Member

Choose a reason for hiding this comment

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

Yeah "config case" is a new one for me haha

if (arg.startsWith('-') && arg.includes('=')) {
// we're dealing with an EqualsArg, we have a special helper for that
const [originalArg, value] = parseEqualsArg(arg);
setCLIArg(flags, arg.split('=')[0], originalArg.replace(/^-/, ''), value);
Copy link
Member

Choose a reason for hiding this comment

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

Can't those all just run through normalizeFlagName()?

}

// we shouldn't get here if the input is well formed! so if we do we should
// raise an error with the malformed argument
Copy link
Member

Choose a reason for hiding this comment

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

I think throw would be okay. I'm all for keeping the CLI decoupled from other bits of the code 👍

const parseNumberArg = (flags: ConfigFlags, args: string[], configCaseName: NumberCLIArg) => {
if (typeof flags[configCaseName] !== 'number') {
flags[configCaseName] = null;
const setCLIArg = (flags: ConfigFlags, rawArg: string, normalizedArg: string, value: CLIValueResult) => {
Copy link
Member

Choose a reason for hiding this comment

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

Usually I'd lean toward the path of small helper functions, but I think this file is already so big that just adds noise and makes the code harder to step through. So, the conditional logic (whatever shape it takes) works for me here

@@ -201,87 +320,41 @@ const parseStringNumberArg = (flags: ConfigFlags, args: string[], configCaseName
*/
const CLI_ARG_STRING_REGEX = /[^\d\.Ee\+\-]+/g;

export const Empty = Symbol('Empty');
Copy link
Member

Choose a reason for hiding this comment

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

I've never used a symbol manually defined like this, can you elaborate on the benefits (for my own curiosity)?

@alicewriteswrongs alicewriteswrongs force-pushed the ap/reporters-flag branch 2 times, most recently from 7b44cb3 to 15c372d Compare November 3, 2022 13:02
@alicewriteswrongs
Copy link
Contributor Author

alicewriteswrongs commented Nov 3, 2022

ok I believe I addressed all feedback, should be ready for another look!

Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs left a comment

Choose a reason for hiding this comment

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

one question

// if it matches the regex we treat it like a string
flags[normalizedArg] = value;
} else {
const parsed = Number(value);
Copy link
Contributor Author

@alicewriteswrongs alicewriteswrongs Nov 3, 2022

Choose a reason for hiding this comment

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

I want to point out an inconsistency in number handling here. Does it make sense to just use Number on the raw string, or to try to use parseInt ? We did have (when I first went in to this module) some idea of supporting scientific notation like 40e4. That rules out using parseInt, which will return 40 for that. Just Number on a string, however, does do some other stuff which is maybe undesirable, like Number("") is 0. Anywhoo - I have it one way for a NUMBER_CLI_FLAG and another way for this STRING_NUMBER_CLI_FLAG, interested in hearing thoughts about which way we should go (another alternative would be carving out a parseCLINumber helper function that has some logic to figure out which of the two, parseInt or just Number, is suitable)

Copy link
Member

Choose a reason for hiding this comment

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

I think so (that this makes sense) 👍

@alicewriteswrongs alicewriteswrongs changed the title fix(cli): handle 'array' arguments correctly fix(cli): refactor CLI argument parser Nov 4, 2022
Copy link
Member

@rwaskiewicz rwaskiewicz left a comment

Choose a reason for hiding this comment

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

All in all looks good! I tested this against the issue that prompted this work and a few older issues:

I wanna do a bit more testing, and had a few minor asks that I think can be done in parallel

@@ -3,7 +3,7 @@ import type { LogLevel, TaskCommand } from '@stencil/core/declarations';
/**
* All the Boolean options supported by the Stencil CLI
*/
export const BOOLEAN_CLI_ARGS = [
export const BOOLEAN_CLI_FLAGS = [
Copy link
Member

Choose a reason for hiding this comment

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

I didn't comment on this the first time around, but I do think this is an improvement & am in favor of keeping the change 👍

* A regular expression which can be used to match a CLI flag for one of our
* short aliases.
*/
export const CLI_FLAG_REGEX = new RegExp(`^-[chpv]{1}$`);
Copy link
Member

Choose a reason for hiding this comment

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

Gahh that's shame :-( We don't (have a precedent). I feel like we should put something on the books to document the treeshaking check a bit better. Sometimes I think about it as a black box that we always abide by. Stubbed out STENCIL-627 to do just that, but I don't think that should block this effort

@@ -136,11 +141,10 @@ const parseCLITerm = (flags: ConfigFlags, args: string[]) => {
}

// NegativeDashArg → "--no-" ArgName ;
if (arg.startsWith('--no-')) {
if (arg.startsWith('--no-') && arg.length > 5) {
Copy link
Member

Choose a reason for hiding this comment

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

It took me a moment to realize what the '5' meant here. It may be easier to track by using '--no-'.length:

  if (arg.startsWith('--no-') && arg.length > '--no-'.length) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah that's a lot less magical 👍

@@ -161,13 +165,10 @@ const parseCLITerm = (flags: ConfigFlags, args: string[]) => {
}

// SimpleArg → "--" ArgName ( " " CLIValue )? ;
if (arg.startsWith('--')) {
if (arg.startsWith('--') && arg.length > 2) {
Copy link
Member

Choose a reason for hiding this comment

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

It took me a moment to realize what the '2' meant here. It may be easier to track by using '--'.length:

  if (arg.startsWith('--no-') && arg.length > '--'.length) {

@@ -1,4 +1,4 @@
import { readOnlyArrayHasStringMember, toConfigCase } from '@utils';
import { readOnlyArrayHasStringMember, toCamelCase as toCamelCase } from '@utils';
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the alias here? Could this just be an import of toCamelCase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed we do not! I think this was an artifact of using the TypeScript language server's rename service

@@ -45,6 +45,16 @@ describe('util helpers', () => {
});
});

describe('toConfigCase', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
describe('toConfigCase', () => {
describe('toCamelCase', () => {

parseArgs(flags, args.slice(1));
} else {
// we didn't find a leading flag, so we should just parse them all
parseArgs(flags, flags.args);
Copy link
Member

Choose a reason for hiding this comment

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

Fun, so we were just parsing things like npx stencil --logLevel error test before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think so...

Comment on lines 272 to 273
// parseInt went alright, so here we go
// (duck typing?)
Copy link
Member

Choose a reason for hiding this comment

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

we can probably remove this comment - I have a feeling it has the potential to lead to confusion in the future (particularly the second line - e.g. "did we have a typing issue in the past?")

// if it matches the regex we treat it like a string
flags[normalizedArg] = value;
} else {
const parsed = Number(value);
Copy link
Member

Choose a reason for hiding this comment

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

I think so (that this makes sense) 👍

@rwaskiewicz
Copy link
Member

With additional testing, I think this looks good. There are a handful of aliases in Jest we don't handle, but I don't think that's in scope here

This commit refactors the argument parser we use for our CLI module.
Doing so fixes an issue with certain flags supported by Jest which can
be passed multiple times. For instance, in the following example:

```
jest --coverage --reporters="default" --reporters="jest-junit"
```

all of the values for the `--reporters` flag ("default" and
"jest-junit") should be collected into an array of values, instead of
simply recording whichever value is farther to the right (Stencil's
behavior before this commit).

To support passing such arguments to the `stencil test` subcommand this
commit adds a new recursive-descent parser in `src/cli/parse-flags.ts`
to replace the somewhat ad-hoc approach that was there previously. It
parses the following grammar:

```
CLIArguments    → ""
                | CLITerm ( " " CLITerm )* ;
CLITerm         → EqualsArg
                | AliasEqualsArg
                | AliasArg
                | NegativeDashArg
                | NegativeArg
                | SimpleArg ;
EqualsArg       → "--" ArgName "=" CLIValue ;
AliasEqualsArg  → "-" AliasName "=" CLIValue ;
AliasArg        → "-" AliasName ( " " CLIValue )? ;
NegativeDashArg → "--no-" ArgName ;
NegativeArg     → "--no" ArgName ;
SimpleArg       → "--" ArgName ( " " CLIValue )? ;
ArgName         → /^[a-zA-Z-]+$/ ;
AliasName       → /^[a-z]{1}$/ ;
CLIValue        → '"' /^[a-zA-Z0-9]+$/ '"'
                | /^[a-zA-Z0-9]+$/ ;
```

The regexes are a little fuzzy, but this is sort of an informal
presentation, and additionally there are other constraints implemented
in the code which handles all of these different terms.

Refactoring this to use a proper parser (albeit a pretty simple one)
allows our implementation to much more clearly conform to this defined
grammar, and should hopefully both help us avoid other bugs in the
future and be easier to maintain.

See #3712 for more details on the issues with the `--reporters` flag in
particular.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants