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

design discussion: toSnakeCase, toCamelCase & related actions #1010

Open
EltonLobo07 opened this issue Jan 9, 2025 · 11 comments
Open

design discussion: toSnakeCase, toCamelCase & related actions #1010

EltonLobo07 opened this issue Jan 9, 2025 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@EltonLobo07
Copy link
Contributor

As concluded in #981, adding toSnakeCase-like transformation actions is a good idea. I have created this issue to discuss the design of the actions. Once the design is finalized, I'll work on adding the actions. The initial requirements that affect the design of the actions are:

  1. Should be flexible enough to exclude or include keys of the object
    Example 1: Without any key (transforms all of the keys)

    const Schema = v.pipe(
      v.object({ productCount: v.number(), someKey: v.string() }),
      v.toSnakeCase()
    );
    
    /*
      Output type:
        {
          product_count: number;
          some_key: string;
        }
    */

    Example 2: With one or more keys (transforms only the listed keys)

    const Schema = v.pipe(
      v.object({ productCount: v.number(), someKey: v.string() }),
      v.toSnakeCase(["someKey"])
    );
    
    /*
      Output type:
        {
          productCount: number;
          some_key: string;
        }
    */
  2. Should not affect the keys in any nested object

    const Schema = v.pipe(
      v.object({
        someK1: v.object({ someNestedKey: v.string() }),
        someK2: v.string(),
      }),
      v.toSnakeCase()
    );
    
    /*
      Output type:
        {
          some_k1: { someNestedKey: string };
          some_k2: string;
        }
    */

    This requirement adds more control over which keys should be transformed. Every nested object will be associated with a Valibot schema. If a user wants to transform the keys of a nested object, the user can use the nested object's schema as shown in the example below.

    const Schema = v.pipe(
      v.object({
        someK1: v.pipe(
          v.object({ someNestedKey: v.string() }),
          v.toSnakeCase(),
        ),
        someK2: v.string(),
      }),
      v.toSnakeCase(),
    );
    
    /*
      Output type:
        {
          some_k1: { some_nested_key: string };
          some_k2: string;
        }
    */

I created the initial requirements based on what I felt was sensible. Feel free to discuss so that we can change the requirements (if required) and finalize the design.

@EltonLobo07 EltonLobo07 changed the title design discussion: toSnakeCase, toCamelCase and related transformation actions design discussion: toSnakeCase, toCamelCase & related actions Jan 9, 2025
@EltonLobo07
Copy link
Contributor Author

@krokyze Feel free to share your opnion.

@krokyze
Copy link

krokyze commented Jan 10, 2025

To be honest, I’d be happier to see something like #163 implemented, as that aligns more closely with my experience using Kotlin and Dart data serialization libraries. In my view, it also offers better performance in several scenarios.

That said, my TypeScript experience is somewhat limited, so I could be mistaken.

  1. While this approach is the simplest in terms of usability, it might be the least efficient performance-wise. For instance, if an object contains 10+ values but only one lacks snake_case support, doesn’t processing the entire object through toSnakeCase add extra performance overhead compared to transforming just the specific value that needs it?
const Schema = v.pipe(
  v.object({ product_count: v.number(), someKey: v.string() }),
  v.toSnakeCase()
);

Having control over which specific values should be transformed, like v.toSnakeCase(['someKey']), is definitely better approach. However, it would still require passing the entire object through the pipe.


  1. If solution will not handle nested keys, I'm confident that a new issue will emerge in the coming months requesting support for it. 😄 And once nested key support is implemented, I believe it might introduce performance drawbacks.

For example, after implementing the current recommended solution, nesting is handled automatically, which is advantageous but comes with a caveat. In my project, both Product.ts and User.ts models utilize v.pipe() with toCamelCase functionality. In various scenarios, these models are parsed separately; however, the Product model includes User as a nested value. So toCamelCase is applied twice to the User value within Product, potentially impacting performance.


Also It would be great if it supported "Go to Definition," which is currently broken when using ts-case-convert. Here's an example in this video.

@fabian-hiller fabian-hiller self-assigned this Jan 10, 2025
@fabian-hiller fabian-hiller added the enhancement New feature or request label Jan 10, 2025
@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 10, 2025

Thank you Elton!

My initial thought was to keep it simple and not allow passing arguments as we can always extend it later. After reading Martin's comment, I think we should investigate how such a feature affects the implementation in terms of performance and bundle size.

When it comes to whether we should transform nested keys, I tend to not transform them. This gives our users more control and simplifies the implementation, but I am aware of the pros and cons of both decisions.

We could also mark such actions as @alpha to allow breaking changes until we are more sure about the final implementation.

One last thing we need to investigate is what the action should do in the following cases when a snake case key of a camel case key already exists.

const Schema = v.pipe(
  v.object({ someKey: v.string(), some_key: v.number() }),
  v.toSnakeCase()
);

I think we have two choices. We can overwrite it, or we can skip it. The important thing is that our runtime implementation matches our type implementation. That's probably a rare case. So it is not too important how we decide, and maybe the more practical choice is just to choose the simpler implementation.

@EltonLobo07
Copy link
Contributor Author

@krokyze

I think it would be a bit difficult and not ideal to implement something like #163 for this feature, as Valibot prefers users to define the input schema and then transform.

By Example 1 and Example 2, I meant both cases will be supported. I assume my initial message was not clear. I apologize.

Related to the concern of passing the entire object through the pipe to transform one or more keys (Example 2 of my initial message): I think it is something the pipeline approach favors. One way to think about it is that the pipeline has more information than the individual schemas mapped to a key inside the object. This makes the pipe an ideal place to transform the object safely.

@EltonLobo07
Copy link
Contributor Author

@krokyze and @fabian-hiller thanks for providing your inputs! I'll implement and investigate some of the concerns mentioned in the discussion. Will keep everyone posted.

@EltonLobo07
Copy link
Contributor Author

EltonLobo07 commented Jan 19, 2025

I need everyone's opinion on these questions:

  1. There are different types of cases. Some of the cases are snake, camel, pascal, and kebab. Should we provide built-in support for the popular cases and then discuss adding the rest if there is a need (open issue in the future that wants an unsupported case added)? If yes, which popular cases should we support initially?
  2. The edge case mentioned here will exist for all transformations (toSnakeCase, toCamelCase, etc.). Should we make handling of conflicts customizable? In other words, should we let users control: if the source should override the destination, or should the source be simply removed when there is a conflict?
  3. This is related to an implementation detail of toSnakeCase. But I expect to be in similar situations when implementing other actions, so I feel it is a relevant question for this discussion. What should the snake case conversion of foo_Bar be? foo__bar or foo_bar? The ts-case-convert package transforms it to foo__bar. Should we make implementation-specific behaviors customizable? Example for toSnakeCase: Create an optional parameter allowConsecutiveUnderscores. If the argument passed is true, foo_Bar will be converted to foo__bar. Otherwise to foo_bar.

@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 22, 2025

  1. I think snake and pascal are the most popular. I would only implement them for now.
  2. I wouldn't make it configurable and only implement what is simpler and maybe smaller in bundle size.
  3. Same here. I wouldn't make it configurable and only implement what is simpler and maybe smaller in bundle size.

The most complex part is probably the key transformation in TypeScript.

@EltonLobo07
Copy link
Contributor Author

EltonLobo07 commented Jan 22, 2025

For points 2 and 3, I favor providing customizable actions. But I also don't like the idea of adding complexity to provide customization capabilities that might not be used at all. I think it's sensible to mark such actions as @alpha → get community feedback → make changes (if required).

So I'll follow @fabian-hiller suggestions if there are no additional comments.

@fabian-hiller
Copy link
Owner

Sounds good 👍

@michachen
Copy link

EltonLobo07

as Valibot prefers users to define the input schema and then transform.

Regarding this statement I tend to disagree, because this will probably be impossible in some scenarios or really really hard.

@EltonLobo07
Copy link
Contributor Author

@michachen Can you provide an example that might explain the difficulties faced?

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

No branches or pull requests

4 participants