-
-
Notifications
You must be signed in to change notification settings - Fork 242
[WIP]: add toSnakeCase
& toSnakeCaseKeys
actions
#1030
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
commit: |
Refactor and rename (if required) types, then move them to the right file based on the usage
@fabian-hiller I'll have to change the algorithm to transform a string to its snake case version, as I have missed some important edge cases. I'll need your feedback on the new algorithm. |
I will try to review it next week. |
There are some tricky problems that might have broken the type safety. To make sure the type safety is maintained and to make sure the TS type checker doesn't get overwhelmed, I have loosened the types a bit while reimplementing the action, and I think it is the correct path to take. I'll reply with a detailed message tomorrow which explains the decisions taken. Once we agree on the new algorithm and the implementation, adding the action to our website will be the only pending work. |
Sounds good. I can try to review it on the weekend and give feedback on your code. |
Ideally, when dealing with duplicate keys, this is what I expected: const Schema = v.pipe(
v.object({FooBar: v.string(), fooBar: v.boolean()}),
v.toSnakeCase()
);
const input = {FooBar: "", fooBar: false}; // type: {FooBar: string, fooBar: boolean}
const output = v.parse(Schema, input); // type: {foo_bar: string} While working on the action, I realized that this expectation was incorrect because:
Additionally, the Since it is difficult to sync the type and runtime key order, we avoid it. This is where we add a bit of inconvenience to maintain type safety. At runtime, the last used duplicate key's value will be in the output. At the type level, the duplicate key values will form a union. const Schema = v.pipe(
v.object({ FooBar: v.string(), fooBar: v.boolean() }),
v.toSnakeCase(),
);
const output = v.parse(Schema, {
FooBar: '',
fooBar: false,
}); // value: {foo_bar: false}, type: {foo_bar: string | boolean} |
The other issue is related to the key select list. When we have two keys, the number of possible select list tuples is four. const Schema = v.pipe(
v.object({ FooBar: v.string(), fooBar: v.boolean() }),
// union of 4 tuples:
// ["fooBar"] | ["FooBar"] | ["fooBar", "FooBar"] | ["FooBar", "fooBar"]
v.toSnakeCase(["fooBar", "FooBar"]),
); The number of possible tuples increases very rapidly as the number of keys in the object increases. This slows down the type checker. In some cases, the type checker never finishes calculating the possible types. I recommend making the select list type loose. type SelectedStringKeys<T extends ObjectInput> = (keyof T & string)[]; |
Do you think it is a good idea to expose an additional action related to this PR? |
Sorry that I don't have a lot of time right now. I will try to review more PRs in about two weeks. Yes, we could provide a |
toSnakeCase
actiontoSnakeCase
& toSnakeCaseKeys
action
toSnakeCase
& toSnakeCaseKeys
actiontoSnakeCase
& toSnakeCaseKeys
actions
While transforming a string to a case such as snake case, the runtime and type system have to find whitespace characters. To keep the runtime & related types in sync, create and use a whitespace list.
SummarySince there is a lot to discuss, I have summarized everything I know until now. API usage
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces two new actions—toSnakeCase and toSnakeCaseKeys—to convert strings and object keys to snake case. Key changes include the addition of utility functions for snake case transformations, corresponding type definitions/tests, and the integration of these actions into the actions index.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
library/src/utils/index.ts | Re-exports new snake case utilities |
library/src/utils/_snakeCase/* | Implements the snake case transformation logic |
library/src/utils/_caseTransform/* | Adds supporting type definitions and helper functions |
library/src/types/utils.ts | Introduces additional type helpers (e.g. IsNever, OptionalKeys) |
library/src/actions/toSnakeCase* | Implements the toSnakeCase action and its tests |
library/src/actions/toSnakeCaseKeys* | Implements the toSnakeCaseKeys action and its tests |
library/src/actions/index.ts | Updates the actions index to re-export the new actions |
Comments suppressed due to low confidence (1)
library/src/utils/_snakeCase/_snakeCase.ts:10
- [nitpick] Consider whether the leading underscore in _snakeCase is necessary for a publicly re-exported function. If it is not intended to denote a private member, renaming it to snakeCase could improve clarity.
export const _snakeCase: (input: string) => string = _createToTargetCase(
No description provided.