A TS fix for PPR #86
A TS fix for PPR #86
Conversation
PPR uses strict typescript checking. This fix is required for PPR to use these common components.
Plus, fix large number of unit test failures
Updating the test package resolves numerous unit test errors that have the form
console.error node_modules/vue/dist/vue.runtime.common.dev.js:1884
TypeError: Cannot read property '_transitionClasses' of undefined
|
@severinbeauvais plz review |
| @@ -63,7 +63,8 @@ export default class SbcHeader extends Vue { | |||
|
|
|||
| get showLogin () : boolean { | |||
| let featureHide: any | |||
| const authApiConfig = JSON.parse(sessionStorage.getItem('AUTH_API_CONFIG')) | |||
| let config: string = sessionStorage.getItem('AUTH_API_CONFIG') || '{}' | |||
There was a problem hiding this comment.
Explicitly typing config is not necessary here. getItem returns a string, so the type is implied. authApiConfig in the next line with still have type any.
As a side note, if I do enable strict and transpile the common-components, I get many more errors that just the one addressed here.
There was a problem hiding this comment.
Actually getItem can also return null when the key does not exist. TS was not happy passing a possible null into the JSON.parse which requires a string.
https://developer.mozilla.org/en-US/docs/Web/API/Storage/getItem
There was a problem hiding this comment.
I'm not disputing that getItem returns string | null as a type. You already handle that with the || '{}' part so config is always going to be of type string. What I'm saying is you shouldn't explicitly declare types when it's not necessary. i.e let TypeScript do that for you as it will be less likely to make a mistake.
Ideally, it should be const config = sessionStorage.getItem('AUTH_API_CONFIG) || '{}'
I was incorrect by pointing out that authApiConfig would be any, as technically this will transpile in strict mode. If you wanted to be truly type-safe (which I assume is what you're going for by pushing for strict mode), you may want to read up on type safe JSON parsing: https://www.sparkbit.pl/type-safe-json-parsing/
There was a problem hiding this comment.
please review update. thanks
PPR uses strict typescript checking. This fix is required for PPR to use these common components.
Plus, fix large number of unit test failures
Updating the test package resolves numerous unit test errors that have the form
console.error node_modules/vue/dist/vue.runtime.common.dev.js:1884
TypeError: Cannot read property '_transitionClasses' of undefined