Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(jest-mock): simplify PropertyLikeKeys utility type #13020

Merged
merged 2 commits into from
Jul 13, 2022
Merged

refactor(jest-mock): simplify PropertyLikeKeys utility type #13020

merged 2 commits into from
Jul 13, 2022

Conversation

mrazauskas
Copy link
Contributor

Summary

The shape of PropertyLikeKeys utility type looks somewhat redundant after #13013. I was not happy with it, hence decided to go for a refactor.

Test plan

PropertyLikeKeys is covered by type test. These and all other type tests should pass.

@@ -374,8 +374,6 @@ expectType<SpyInstance<typeof indexSpiedObject.methodE>>(
spyOn(indexSpiedObject, 'methodE'),
);

expectError(spyOn(indexSpiedObject, 'propertyA'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same test is repeating few lines bellow.

@@ -142,6 +142,6 @@ declare const objectProperties: PropertyLikeKeys<SomeObject>;
declare const indexObjectProperties: PropertyLikeKeys<IndexObject>;

expectType<'propertyA' | 'propertyB' | 'propertyC'>(classProperties);
expectType<string>(indexClassProperties);
expectType<string | number>(indexClassProperties);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fells like string | number is more correct or at least more TypeScriptish.

A note from TS docs: keyof {[k: string]: any} is of type string | number, "because JavaScript object keys are always coerced to a string".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked, TypeScript allows this:

type IndexObject = {
  [key: string]: Record<string, any>;
};

const indexObject: IndexObject = {
  123: {a: 123},

  prop: {b: 'abc'},
};

So it feels like this is correct (or?):

type IndexObject = {
  [key: string]: Record<string, any>;
};

type PropKeys = PropertyLikeKeys<IndexObject>; // `string | number`

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

works for me!

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants