Skip to content

Better TOML types #6656

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Better TOML types #6656

wants to merge 4 commits into from

Conversation

Gouvernathor
Copy link

No description provided.

these are the types that may come out of a toml parser, not the types that may be put in a toml serializer
inspired by type-fest's Json types
@Gouvernathor Gouvernathor requested a review from kt3k as a code owner May 18, 2025 17:40
@CLAassistant
Copy link

CLAassistant commented May 18, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the toml label May 18, 2025
@@ -0,0 +1,4 @@
export type TOMLTable = {[Key in string]: TOMLValue} & {[Key in string]: TOMLValue|undefined};
Copy link
Member

Choose a reason for hiding this comment

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

We'd prefer the capitalization of TomlTable. See https://docs.deno.com/runtime/contributing/style_guide/#naming-convention

Copy link
Member

Choose a reason for hiding this comment

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

The second part {[Key in string]: TOMLValue|undefined} looks strange to me. Is it possible for key to be undefined? Is that spec compliant?

@@ -131,18 +132,21 @@ export class Scanner {
// Utilities
// -----------------------

function success<T>(body: T): Success<T> {
function success<T extends TOMLValue|void>(body: T): Success<T> {
Copy link
Member

Choose a reason for hiding this comment

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

Why |void part necessary?

@kt3k
Copy link
Member

kt3k commented May 20, 2025

Run deno fmt to format the source code

@kt3k
Copy link
Member

kt3k commented May 20, 2025

We currently don't accept a new feature directly, but we'd like to accept it as unstable feature first (See https://github.com/denoland/std/blob/main/.github/CONTRIBUTING.md#suggesting-a-new-feature )

Let's keep the return type of parse() to Record<string, unknown>, and create another entrypoint unstable_parse.ts which exports unstable parse function that returns TomlValue type.

for (const parse of parsers) {
const result = parse(scanner);
if (result.ok) return result;
}
return failure();
};
}) as unknown as any;
Copy link
Member

Choose a reason for hiding this comment

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

This needs // deno-lint-ignore no-explicit-any directive before this line to pass lint check. See the deno lint output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants