-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor: Adjust code for some Typescript errors #344
Conversation
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.
Some comments for easier review
ShareType.SHARE_TYPE_LINK, | ||
ShareType.SHARE_TYPE_EMAIL, | ||
ShareType.Link, | ||
ShareType.Email, |
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.
Lets use the non deprecated variant :)
render: h => h(AdminSettings), | ||
}) | ||
instance.$mount('#admin-download-limit') |
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.
Makes no technical difference, but silences ESLint without needing a comment
@@ -74,21 +75,21 @@ export default defineComponent({ | |||
data() { | |||
return { | |||
limitEnabled: false, | |||
initialLimit: null, | |||
initialLimit: null as number | 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.
Otherwise the inferred type is just null
loading: false, | ||
hasError: false, | ||
} | ||
}, | ||
|
||
computed: { | ||
remainingCount() { | ||
return this.initialLimit - this.count | ||
return (this.initialLimit ?? 0) - (this.count ?? 0) |
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.
Math is not possible with null
as null
is an object in JavaScript
}, | ||
|
||
helperText() { | ||
if (this.limit > 0) { | ||
if (this.limit && this.limit > 0) { |
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.
Could be null
, null
can not be compared with number (strictly in TS)
handleUpdateLimit(limit: string) { | ||
this.limit = Number(limit) // emitted <input> value is string so we parse it to number | ||
}, | ||
|
||
async onSave() { | ||
const isValid = typeof this.limit === 'number' && this.limit > 0 | ||
if (!isValid) { | ||
if (typeof this.limit !== 'number' || this.limit <= 0) { |
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.
With that helper Typescript can not type check the limit
property, so directly using it in the if
allows Typescript to narrow down the type further down the function :)
@@ -59,7 +59,7 @@ window.addEventListener('DOMContentLoaded', () => { | |||
} | |||
|
|||
// Adding double-download warning | |||
const downloadButtons = document.querySelectorAll<HTMLAnchorElement>('a[href*="/download/"]') || [] | |||
const downloadButtons = Array.from(document.querySelectorAll<HTMLAnchorElement>('a[href*="/download/"]')) |
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.
querySelectorAll
does not return an array but a NodeList
, so passing it to a Set
works only if the browser likes you 😅
/compile rebase-amend / |
Signed-off-by: Ferdinand Thiessen <[email protected]> Signed-off-by: nextcloud-command <[email protected]>
c63c36b
to
185fc97
Compare
No description provided.