refactor: move the shared prompt types to core and drop dead exports - #47
Merged
Merged
Conversation
Cleanup the command-factory series exposed, now that every feature's imports are visible in one place instead of buried in `app.ts`. `emulator-ui-prompt-types.ts` moves to `core/ui/core-ui-prompt-types.ts`. Nothing in it was ever emulator-specific — it is the generic clack prompt contracts (`MultiSelectPrompt`, `SelectPrompt`, `TextPrompt`, `PromptDependencies`) plus `resolvePromptCancellation` — and 24 files across five features imported it from emulator. That removes the last `device -> emulator`, `playground -> emulator` and `webshell -> emulator` edges in the import graph; what is left crossing feature boundaries is device and playground reaching into localnet for adb, and emulator reusing device's tweak UI. `emulator-types.ts` no longer re-exports `CommandRunner`, `InteractiveCommandRunner` and `RunCommandOptions` from core. It was the same barrel indirection the series removed elsewhere, and it let files take a core type by way of emulator. Its four consumers now import from `core/data-access/command-types.ts` directly; `RunCommandOptions` turned out to have no consumer at all, and the file's own import block moves to the top where the rest are. `core/ui/core-ui-select-command.ts` is deleted. `selectCommand` and `CommandOption` had no reference anywhere in `src`, `test` or `scripts`. `extractTemplateOptions` and `parsePackageManagerOption` lose their `export`: they were public for `app.ts`, which no longer calls them now that the create wiring lives beside them. `createDoctorReport` loses its own for the same reason. No behaviour change: 487 tests pass with and without a TTY, and the move is type-only apart from `resolvePromptCancellation` changing file.
|
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (32)
Comment |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleanup the command-factory series (#41, #45, #46) exposed. With every feature's imports now visible in its own file instead of buried in a 660-line
app.ts, a few things stood out as living in the wrong place.No behaviour change. 487 tests pass with and without a TTY.
The prompt types were never emulator-specific
emulator/ui/emulator-ui-prompt-types.ts→core/ui/core-ui-prompt-types.ts.Its contents are the generic clack prompt contracts —
MultiSelectPrompt,SelectPrompt,TextPrompt,PromptDependencies— plusresolvePromptCancellation. Nothing about emulators. 24 files across five features imported it from emulator, including everydevice/ui/*selector,webshell-ui-prompts.ts, andplayground-feature-serve.ts.Measured effect on the cross-feature import graph (counting both
../feature/and../../feature/forms;-> coreedges excluded as legitimate):device -> emulatorplayground -> emulatorwebshell -> emulatordevice -> localnetplayground -> localnetemulator -> deviceplayground -> devicedoctor -> emulatorlocalnet -> emulatorTwo edges gone entirely and the third down to a single import, and every one removed existed only to borrow types that had nothing to do with emulators. The
device -> emulatorimport that remains is real:list-connected-devices.tsusesparseRunningEmulatorNameto label emulator serials.What still crosses feature boundaries is mostly device and playground reaching into localnet for adb primitives — 11 of those 15 device imports are just for the
Adb*types inlocalnet-types.ts, anddevice/data-access/device-types.tsis one of them, so device's own type file currently depends on localnet's. That is a follow-up.emulator -> deviceis deliberate and dates from #40.A barrel survived inside a types file
emulator/data-access/emulator-types.tsre-exportedCommandRunner,InteractiveCommandRunnerandRunCommandOptionsfromcore/data-access/command-types.ts. That is the same indirection this series removed seven times over, and it let files take a core type by way of emulator. Its four consumers now import from core directly.RunCommandOptionshad no consumer at all, so that line was serving one live import and two dead ones. The file's own import block also moves to the top, where it was sitting mid-file after several interface declarations.Dead code
core/ui/core-ui-select-command.tsis deleted —selectCommandandCommandOptionhad zero references insrc,testorscripts.extractTemplateOptionsandparsePackageManagerOptionlose theirexport; they were public soapp.tscould call them, and #46 moved the create wiring into the same file.createDoctorReportloses its own for the same reason, from #45.Exports with no consumer outside their own file: 77 → 72. The remaining 72 are pre-existing, mostly dependency-injection types, and want a tool like
kniprather than a hand audit — biome 2.x hasnoUnusedImportsbut no project-wide unused-export rule.Verification
bun run ci— build, lint,tsc -b --noEmit, 487 pass / 0 fail. Also 487/0 under a pseudo-TTY, and the CLI boots.Apart from
resolvePromptCancellationchanging file, the move is type-only. No changeset: thesolana-mobilebin and the./templatesexport are unchanged.