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

fix: object properties, func, and so on must not convert to camelCase #112

Merged
merged 2 commits into from Aug 16, 2023

Conversation

yutak23
Copy link
Contributor

@yutak23 yutak23 commented Jun 26, 2023

This ought to fix #111. And also fix #77.

I think the reason why even fields/methods of classes and object are converted to camelCase is because of the use of Record<string, any>.
Record<string, any> means { [P in string]: any; }. This type definition uses any, so it will be true as follows.

// type result is 'true'(Date extends Record<string, any>)
type result = Date extends Record<string, any> ? 'true' : 'false';

Therefore, I think the problem can be solved by not using Record<string, any> and using Record<string, unknown> instead.

The rationale is as follows.
https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#-k-string-unknown--is-no-longer-a-wildcard-assignment-target

@@ -64,7 +64,7 @@
"ava": "^5.0.1",
"matcha": "^0.7.0",
"tsd": "^0.24.1",
"xo": "^0.52.4"
"xo": "^0.54.2"
Copy link
Contributor Author

@yutak23 yutak23 Jun 26, 2023

Choose a reason for hiding this comment

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

I have version up because following issue error (on local testing).

xojs/xo#718

@@ -69,10 +68,8 @@ export type CamelCaseKeys<
true,
]
? T[P]
// eslint-disable-next-line @typescript-eslint/ban-types
: {} extends CamelCaseKeys<T[P]>
Copy link
Contributor Author

@yutak23 yutak23 Jun 26, 2023

Choose a reason for hiding this comment

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

This has been added in this PR, which is essentially a Record<string, any>, which would have been {} due to the fact that it is a Record<string, any>. This change to unknow makes this condition unnecessary.

// type result is 'true'
type testType<T> = T extends Record<string, any> ? 'true' : 'false';
type result = testType<() => 123>;

? T[P]
: [Deep] extends [true]
: [Deep] extends [true]
? T[P] extends ObjectOptional | readonly any[]
Copy link
Contributor Author

@yutak23 yutak23 Jun 26, 2023

Choose a reason for hiding this comment

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

To support optional properties.
This change allows the following tests to pass.

expectType<ConvertedDeepObjectDataTypeWithDeepTrue>(
	camelcaseKeys(deepInputData, {deep: true}),
);

@yutak23
Copy link
Contributor Author

yutak23 commented Aug 2, 2023

@sindresorhus

I would appreciate it if you could check the PR to fix the bug when used with TypeScript.

@sindresorhus sindresorhus merged commit d3cf1e2 into sindresorhus:main Aug 16, 2023
3 checks passed
@yutak23 yutak23 deleted the object-not-convert branch August 16, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants