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

Chore: Increase strength of Settings types #10605

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Jun 15, 2024

Summary

This pull request demonstrates a way to strengthen types returned by Setting.value('some-key-here') and appState.settings['some-key-here'].

Examples: Setting.value

Return type of Setting.value('editor.usePlainText'):

  • Before: any
  • After: boolean

Return type of Setting.value('folders.sortOrder.field'):

  • Before: any
  • After: string

Return type of Setting.value('someCustomSetting'):

  • Before: any
  • After: any

Example: Setting.setValue

Setting.setValue('showMenuBar', 123456);:

  • Before: Passes typechecking.
  • After: Argument of type 'number' is not assignable to parameter of type 'boolean'.

Testing plan

  • To-do.
    • Should include manual tests for installing/disabling/updating plugins.
    • Should include manual tests for desktop app layout.

Comment on lines +63 to +64
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Partial refactor of old code before rule was applied
Setting.setValue('notes.perFieldReverse', { ...perFieldReverse } as any);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To-do -- better type.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review June 17, 2024 14:47
@laurent22 laurent22 merged commit a44412a into laurent22:dev Jun 25, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants