Skip to content
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

BREAKING(front_matter): deprecate Format enum, use union type instead #3641

Merged
merged 5 commits into from
Oct 6, 2023

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Sep 15, 2023

part of #3601 #3489

This PR deprecates Format enum, and use the union type "yaml" | "toml" | "json" | "unknown" instead for format parameters.

Copy link
Contributor

@lino-levan lino-levan left a comment

Choose a reason for hiding this comment

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

LGTM. Makes a lot of sense.

MAP_FORMAT_TO_EXTRACTOR_RX,
MAP_FORMAT_TO_RECOGNIZER_RX,
} from "./_formats.ts";

type Format = "yaml" | "toml" | "json" | "unknown";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be exported?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch, I would think so.

Copy link
Member Author

Choose a reason for hiding this comment

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

The name 'Format' is currently exported as enum value, which this PR deprecates. I think exporting this 'Format' in this PR could break typing of the user's code.

How about using literal type ("yaml" | "toml" | "json" | "unknown") directly in public interfaces?

@kt3k kt3k merged commit 39c551d into denoland:main Oct 6, 2023
9 checks passed
@kt3k kt3k deleted the fronte-matter-use-union-for-format branch October 6, 2023 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants