-
Notifications
You must be signed in to change notification settings - Fork 397
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: remove ts-strict-ignore for env files and add typing #2683
fix: remove ts-strict-ignore for env files and add typing #2683
Conversation
@@ -21,7 +21,7 @@ export interface Environment { | |||
readonly docsUrlPrefix: string; | |||
readonly links: { | |||
|
|||
readonly COMMON_STORAGE: string, | |||
readonly COMMON_STORAGE: string | null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default value is null, so I added null.
readonly SETTINGS_NETWORK_CONFIGURATION: string, | ||
readonly SETTINGS_ALERTING: string | null, | ||
readonly SETTINGS_NETWORK_CONFIGURATION: string | null, | ||
readonly EVCS_CLUSTER: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is this attribute on theme.ts. but in this file this attribute is missing. so I added it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great 👍
// cf. | ||
// - tools/docker/ui/root/etc/s6-overlay/s6-rc.d/init-nginx/run | ||
// - tools/docker/ui/assets/env.template.js | ||
const window_env = (window as any).env as { [key: string]: string}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
window is exsiting variable, but window.env is added by project dependent code. typing is needed.
readonly SETTINGS_NETWORK_CONFIGURATION: string, | ||
readonly SETTINGS_ALERTING: string | null, | ||
readonly SETTINGS_NETWORK_CONFIGURATION: string | null, | ||
readonly EVCS_CLUSTER: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great 👍
For upstream, is this your way with really strict with lint and typing? You want to drop all ts-strict-ignore? Then it is same as our will. |
I share one script. // I only checked this script work in my mac.
and it is sorted by strict mode error number, line numbers for file.
I(and We) will mainly focus on the head files of this list initially. Because small changes are easy to review and can be improved little by little but surely. as of 2024.6 0876dce
|
No description provided.