Skip to content

feat: Refactor and update CLI package#2761

Open
inferrinizzard wants to merge 6 commits into
masterfrom
release/bin/revamp
Open

feat: Refactor and update CLI package#2761
inferrinizzard wants to merge 6 commits into
masterfrom
release/bin/revamp

Conversation

@inferrinizzard

Copy link
Copy Markdown
Contributor

Description

Related Issue

Motivation and Context

Previous Behaviour / Output

New Behaviour / Output

How Has This Been Tested?

Screenshots (if appropriate):

* fix lint errors

* remove gratuitous lodash from test.ts

* resolve lint issues in index
inferrinizzard and others added 3 commits May 26, 2025 12:21
* fix lint errors

* remove gratuitous lodash from test.ts

* resolve lint issues in index

* split CLIOptions interface to own file

* split cli options, inference, option definitions, utils to own files

* split sources to own file

* move input, usage to own file

* move quicktype options to own file

* ♻️
@schani

schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

Thanks for this — splitting src/index.ts into focused modules (cli.options.ts, quicktype.options.ts, sources.ts, etc.) is a welcome direction. A few things need to be resolved before this can merge:

Hard requirements

  • src/utils.ts's negatedInferenceFlagName now uses lodash's _.capitalize, which (unlike quicktype-core's capitalize in packages/quicktype-core/src/support/Strings.ts) lowercases the rest of the string, not just the first character. Since the negated flag name feeds dashedFromCamelCase, this silently renames CLI flags: --no-date-times--no-datetimes, --no-integer-strings--no-integerstrings, --no-boolean-strings--no-booleanstrings (both in parsing and in --help output). No fixture/unit test covers these flags, so CI stayed green. Please keep quicktype-core's capitalize.
  • The new src/index.ts only exports writeOutput, main, and the CLIOptions type. parseCLIOptions (now in cli.options.ts), makeQuicktypeOptions (quicktype.options.ts), and jsonInputForTargetLanguage (input.ts) are no longer re-exported from the package entry point (package.json's main/types still point at dist/index.js/dist/index.d.ts). That's a silent removal of public API for programmatic consumers of the quicktype npm package. Since we're planning a major version, dropping these deliberately is negotiable — dropping them silently isn't. Please either re-export them or call out the removal explicitly.

Please also address

  • The branch has real conflicts with current master in src/GraphQLIntrospection.ts, src/index.ts, test/languages.ts, test/lib/deepEquals.ts, test/lib/multicore.ts, and test/test.ts — master has since moved on (e.g. the boolean --help fix, integer-range inference, the Biome formatting pass, and test/lib/multicore.ts now using process.exit instead of the removed exit package). Because this PR relocates index.ts wholesale, resolving these amounts to redoing the split against current master.
  • New file names (cli.options.ts, quicktype.options.ts, CLIOptions.types.ts, and friends) don't match the repo's existing module convention (TypeSource.ts, URLGrammar.ts, GraphQLIntrospection.ts) — please align on PascalCase.
  • This PR's file set is a subset of your own chore: Add unit tests for options functions and cli #2792, which touches the same files plus packages/quicktype-core and adds unit tests. We won't merge both — let's consolidate into a single PR, redone on current master, with the two hard requirements above addressed.

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.

2 participants