feat: add TypeScript TOML config loader and consolidate types#3655
feat: add TypeScript TOML config loader and consolidate types#3655christierney wants to merge 3 commits intomainfrom
Conversation
| export type ConfigurationLocation = { | ||
| configurationName: string; | ||
| configurationPath: string; | ||
| configurationRelPath: string; |
There was a problem hiding this comment.
nobody was using this. We can always add it back if we want it but I think it's confusing
| @@ -0,0 +1,253 @@ | |||
| // Copyright (C) 2026 by Posit Software, PBC. | |||
There was a problem hiding this comment.
this is based on a similar test on the Go side---it's basically checking that we have defined our schema correctly. (It's probably not comprehensive).
There was a problem hiding this comment.
should we make a follow-up ticket to try to make this more comprehensive? These seem like good tests.
There was a problem hiding this comment.
Maybe? I'm not sure about the approach which feels a little haphazard. But it is nice to prove the schema does what we think.
| "target": "ES2023", | ||
| "outDir": "out", | ||
| "lib": ["ES2023", "DOM"], | ||
| "resolveJsonModule": true, |
There was a problem hiding this comment.
required for loading the JSON schema file
| Description: cfg.Description, | ||
| } | ||
| if cfg.Connect != nil { | ||
| if cfg.Connect.AccessControl != nil { |
There was a problem hiding this comment.
due to validation, it was impossible for this to ever be not nil
| }); | ||
|
|
||
| it("passes through already-camelCase keys unchanged", () => { | ||
| const result = convertKeysToCamelCase({ |
There was a problem hiding this comment.
does this test actually validate camelCase keys?
There was a problem hiding this comment.
it's validating that keys which are already camelCase (authType, loadFactor, defaultImageName) get passed through unchanged
There was a problem hiding this comment.
ohhh - yes, ok. Keys, not values. sorry, disregard
| }); | ||
|
|
||
| it("preserves environment keys (user-defined)", () => { | ||
| const result = convertKeysToCamelCase({ |
There was a problem hiding this comment.
does this actually test that we aren't camelCasing things? Should we include some snake_casing in this test to make the fact that things aren't changing more clear?
There was a problem hiding this comment.
MY_API_KEY and DATABASE_URL are there, or are you thinking there should be lower_case fields?
There was a problem hiding this comment.
I got mixed up on keys vs values, disregard!
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
| expect(isConfigurationError(result)).toBe(false); | ||
| const cfg = result as Configuration; | ||
|
|
||
| // validate defaults to true (matches Go New()) |
There was a problem hiding this comment.
I think these code comments are helpful for now - eventually, we will want to probably want to make a sweep-through and delete the go references once the go implementation is gone.
| // Copyright (C) 2026 by Posit Software, PBC. | ||
|
|
||
| import { describe, expect, it } from "vitest"; | ||
| import Ajv2020 from "ajv/dist/2020"; |
There was a problem hiding this comment.
is it correct that we are pulling something that seems to be versioned for 2020?
There was a problem hiding this comment.
that's the version of JSON-schema we use:
There was a problem hiding this comment.
also the most current version
|
this all makes sense to me - looks good. I don't know why that arm build step is failing - its failing everywhere, currently investigating. |
dotNomad
left a comment
There was a problem hiding this comment.
Thanks for that change 🎉
Add a TypeScript-native TOML configuration loader that matches Go's config.FromFile behavior, as a building block for migrating away from the Go backend. - Add smol-toml, ajv, ajv-formats dependencies for TOML parsing and JSON Schema validation - Create src/toml/ module: loader, key converter, error factories, and JSON schema copy - Consolidate TS types to match Go and schema (remove dead types/fields, add missing ones) - Remove dead Go types (Schedule, AccessType, ConnectAccessControl) and unreachable code in content.go - Fix schema: auth_type in integration_requests (was camelCase), add additionalProperties: false to integration_requests items Fixes #3651 Fixes #3652 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Jordan Jensen <jordan.jensen@posit.co>
e35e6a3 to
250556d
Compare
|
#3662 includes these changes |
Intent
Add a TypeScript-native TOML configuration loader that matches Go's config.FromFile behavior, as a building block for migrating away from the Go backend.
Fixes #3651
Fixes #3652
Type of Change
Approach
User Impact
Minimal. Schema now accurately reflects what the code produces. The schema is also stricter now, so in theory if somebody had hand-edited their integration requests TOML and introduced errors, they will get better error messaging.
Automated Tests
New tests for new code. I used the existing Go config loading tests as a guide for coverage.
Directions for Reviewers
The type cleanup is hopefully fairly easy to follow. Unused stuff that got deleted should be easy to prove as unused since little to no code referenced it. It's a little hard to read the JSON schema manually and see that the TS types now align more closely with it, but the code for parsing and validating TOML helps prove that.
Nothing is calling the TS toml code yet (other than tests) but I tried to design it to be a clean building block for replacing the APIs, and there are some "downstream compatibility" tests to help demo that.
A note on the schema change: this could be considered breaking, since I'm renaming an existing field, but I think it's ok for a couple reasons:
The schema now lives in two places (under the Go code and under TS). I think this is acceptable for now as we're in flux. We should clean this up as we get rid of the Go side.
Checklist
Mentioned the schema change since that is potentially user-visible. The rest is all internal changes.