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 some typing issues related to TS 4.8 #488

Merged
merged 1 commit into from
Aug 18, 2022
Merged

Fix some typing issues related to TS 4.8 #488

merged 1 commit into from
Aug 18, 2022

Conversation

RyanCavanaugh
Copy link
Contributor

See comments inline

@@ -34,7 +34,7 @@ export interface IGitSpawnExecutionOptions {
* set as environment variables before executing the git
* process.
*/
readonly env?: Object
readonly env?: object
Copy link
Contributor Author

Choose a reason for hiding this comment

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

object is the correct type to use 99% of the time; using Object creates problems because e.g. a deep-partial regular object can't be an Object but is a valid object.

@@ -195,7 +195,7 @@ describe('git-process', () => {
try {
await GitProcess.exec(['show', 'HEAD'], testRepoPath)
} catch (e) {
error = e
error = e as Error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 4.8, e has type unknown under --strict. I added the one type assertion instead of turning that off

Copy link
Member

@niik niik left a comment

Choose a reason for hiding this comment

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

Thanks so much! We should probably make the env type more specific but let's get this in and get a new release out first.

@niik niik merged commit c7008e9 into desktop:master Aug 18, 2022
@RyanCavanaugh RyanCavanaugh deleted the fixObject branch August 18, 2022 17:13
@niik niik mentioned this pull request Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants