support for configs in jsonc or json#54
Conversation
|
Hi @mrhut10, thanks for contributing this! I'll test and merge in the next few days. |
|
cheers @zpg6 look forward to the feedback, let me know if can help any more |
|
@mrhut10 If you can conduct e2e testing, even exploratory and leave me a few notes, that would speed us along :) |
yep test evidence. CLI under test: done via a worktree for each configs
let me know if anything else needs testing with it |
|
Have you had a chance to review @zpg6 ? |
commit: |
|
let me know if can help at all @zpg6 |
There was a problem hiding this comment.
Pull request overview
Adds support for the CLI (notably migrate) to detect and parse Cloudflare Wrangler configuration from wrangler.json, wrangler.jsonc, or wrangler.toml, addressing requests in Issues #20 and #53.
Changes:
- Implement wrangler config auto-detection + parsing for JSON/JSONC/TOML, and add
--configoverride for the migrate workflow. - Add unit tests covering TOML/JSON/JSONC parsing behavior.
- Update docs/CLI help text to describe multi-format wrangler config support.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/configuration.md | Updates configuration docs to describe supported wrangler config formats. |
| cli/tests/wrangler-config.test.ts | Adds unit tests for parsing wrangler config across TOML/JSON/JSONC. |
| cli/src/lib/helpers.ts | Adds config discovery + unified parsing logic for JSON/JSONC/TOML. |
| cli/src/index.ts | Updates migrate flow to use auto-detected (or explicit) wrangler config + adds --config help text. |
| cli/README.md | Updates CLI documentation wording around wrangler config support. |
| cli/package.json | Adds JSONC parsing dependency. |
Comments suppressed due to low confidence (1)
cli/package.json:51
- The CLI now
require()sjsonc-simple-parserat runtime (and also@iarna/tomlwhen parsing TOML), but these are listed underdevDependencies. When this package is installed from npm, devDependencies are not installed, so the CLI will fail withCannot find module .... Move these runtime dependencies intodependencies(or avoid the runtime require).
"dependencies": {
"ora": "^9.0.0",
"picocolors": "^1.0.0",
"prompts": "^2.4.2"
},
"devDependencies": {
"@iarna/toml": "^2.2.5",
"jsonc-simple-parser": "^1.0.0",
"@types/node": "^22.7.5",
"@types/prompts": "^2.4.9",
"bun-types": "^1.1.33",
"postgres": "^3.4.4",
"typescript": "^5.5.4"
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function parseWranglerConfigObject(parsed: Record<string, unknown>): { | ||
| databases: DatabaseConfig[]; | ||
| hasMultipleDatabases: boolean; | ||
| } { | ||
| const databases: DatabaseConfig[] = []; | ||
|
|
||
| const d1Databases = parsed.d1_databases as Array<Record<string, unknown>> | undefined; | ||
| if (d1Databases && Array.isArray(d1Databases)) { |
There was a problem hiding this comment.
I prefer something more typesafe, but obviously let's not concern ourselves with irrelevant wrangler fields
| const databases: DatabaseConfig[] = []; | ||
|
|
| // Check for explicit config path from --config flag | ||
| const explicitConfigPath = cliArgs?.config as string | undefined; | ||
|
|
||
| // Read and parse wrangler.toml to detect database configurations | ||
| debugLog(`Reading wrangler.toml from: ${wranglerPath}`); | ||
| let wranglerContent: string; | ||
| try { | ||
| wranglerContent = readFileSync(wranglerPath, "utf8"); | ||
| } catch (e) { | ||
| fatal("Failed to read wrangler.toml"); | ||
| return; | ||
| // Find wrangler config file (supports wrangler.json, wrangler.jsonc, wrangler.toml) | ||
| const config = explicitConfigPath | ||
| ? findWranglerConfigWithExplicitPath(explicitConfigPath) | ||
| : findWranglerConfig(process.cwd()); | ||
|
|
| ☁️ Handles Cloudflare resource creation: | ||
|
|
||
| - Runs `wrangler d1/kv/r2 create` commands and configures `wrangler.toml` | ||
| - Runs `wrangler d1/kv/r2 create` commands and configures your wrangler config file |
There was a problem hiding this comment.
Yes, and I prefer the CLI generate use toml by default still
|
@mrhut10 - lets make these fixes, then we merge and I will patch the CLI on npm |
|
hay @zpg6 sorry had lost interest as had been using my patched version successfully for a while. i'll have a look at the comments above over the weekend |
hoping this allows the CLI to work with other config file types. as identified in
#20 and in #53 I believe