search

Vortec4800 / homebridge-ecobee-away

Homebridge plugin to access or set the Ecobee's home and away status
Apache License 2.0
6 stars 5 forks source link

Address 'unknown' and lint errors #15

Closed powellcj12 closed 2 years ago

powellcj12 commented 2 years ago

There was change in Typescript last year that instead of defaulting to any, errors in catch blocks now default to unknown - see here.

Commit da3f4090bce74c7b8a5670079e87088a84893577 updated the package lock file such that it seems like now various CI tools are pulling in the new version of typescript. I've updated the nom package file to also use at least the version of TS that introduced these issues so that it's easier to reproduce locally.

For auth-refresh-token, I opted to check for an axios error first since that was done in refresh-token.

For awaySwitchAccessory, I was a bit lazy and just treated the type as Error since that seemed to be what we basically assumed the type would have been when invoking the callback.

For refresh-token, there's a type-safe way of checking for an axios error so I switched to that (also what was used in auth-refresh-token).

While here I addressed a couple lint issues, namely specifying a type in updateHomebridgeConfig and checking the response property on axios errors before using it.

Lastly, since I build on macOS, I added .DS_Store to be ignored by git.

powellcj12 commented 2 years ago

Something else that might be worth considering that I can update here as well - validating against NodeJS v10.x doesn't seem worth it anymore since Homebridge stopped supporting it as of April 2021. Seems like we might also want to add more recent versions (14.x, 16.x)? Selfishly since I'm on an ARM macOS machine and don't want to install Rosetta, I need at least 15.x to build locally. 😄

Vortec4800 commented 2 years ago

Thanks for putting this PR (and the other one) together. I work a lot in Javascript but almost never in Typescript, so I'm thankful you went through and got this fixed. I'm also working on an M1 Mac so I think supporting Node 14 and 16 makes sense, with 16 being the primary target given we can't even use 14.