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

Update to Typescript 5 #10535

Merged
merged 12 commits into from
Sep 12, 2023
Merged

Update to Typescript 5 #10535

merged 12 commits into from
Sep 12, 2023

Conversation

notbakaneko
Copy link
Collaborator

Shouldn't break anything, hopefully 🤔

@@ -295,7 +295,9 @@ export default class Channel {

runInAction(() => {
// gap in messages, just clear all messages instead of dealing with the gap.
const minMessageId = minBy(messages, 'messageId')?.messageId ?? -1;
const maybeMinMessageId = minBy(messages, 'messageId')?.messageId ?? -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

?? -1 not needed here?

@@ -50,7 +50,11 @@ export default class ConversationView extends React.Component<Props> {
each(channel.messages, (message: Message, key: number) => {
// check if the last read indicator needs to be shown
// when messageId is a uuid, comparison will always be false.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this comment can use an update

@@ -295,7 +295,9 @@ export default class Channel {

runInAction(() => {
// gap in messages, just clear all messages instead of dealing with the gap.
const minMessageId = minBy(messages, 'messageId')?.messageId ?? -1;
const maybeMinMessageId = minBy(messages, 'messageId')?.messageId ?? -1;
// for typing; realistically, a uuid won't be min unless it's the only value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

or is it 🤔

> _.minBy([{a:'100asd'},{a:100}], 'a')
Object { a: "100asd" }

first item being a string turned it into a string comparison
Comment on lines 18 to 20
return typeof message.messageId === 'number'
? message.messageId
: NaN;
Copy link
Collaborator

Choose a reason for hiding this comment

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

might as well make a normal loop?

let ret: number|undefined;
for (const m of messages) {
  if (typeof m.messageId === 'number' && (ret == null || m.messageId < ret)) {
    ret = m.messageId;
  }
}
return ret ?? -1;

also add note about fixing/checking getter and variable names
@nanaya
Copy link
Collaborator

nanaya commented Sep 12, 2023

conflicts

@nanaya nanaya merged commit 0b602e3 into ppy:master Sep 12, 2023
3 checks passed
@notbakaneko notbakaneko deleted the feature/ts5 branch September 13, 2023 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants