-
Notifications
You must be signed in to change notification settings - Fork 73
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
Type fixes needed to make matrix-appservice-irc build #196
Changes from all commits
ebe8128
3280097
01e0aa1
66a37bd
06d59ef
596e0ea
e027f07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Bump matrix-js-sdk to 8.0.1 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ import PQueue from "p-queue"; | |
interface StateLookupOpts { | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
client: any; //TODO: Needs to be MatrixClient (once that becomes TypeScript) | ||
stateLookupConcurrency: number; | ||
stateLookupConcurrency?: number; | ||
eventTypes?: string[]; | ||
retryStateInMs?: number; | ||
} | ||
|
@@ -33,14 +33,15 @@ interface StateLookupRoom { | |
}; | ||
} | ||
|
||
interface StateLookupEvent { | ||
export interface StateLookupEvent { | ||
// eslint-disable-next-line camelcase | ||
room_id: string; | ||
// eslint-disable-next-line camelcase | ||
state_key: string; | ||
type: string; | ||
// eslint-disable-next-line camelcase | ||
event_id: string; | ||
content: unknown; | ||
} | ||
|
||
const RETRY_STATE_IN_MS = 300; | ||
|
@@ -96,7 +97,7 @@ export class StateLookup { | |
* array of events, which may be empty. | ||
* @return {?Object|Object[]} | ||
*/ | ||
public getState(roomId: string, eventType: string, stateKey?: string): unknown|unknown[] { | ||
public getState(roomId: string, eventType: string, stateKey?: string): null|StateLookupEvent|StateLookupEvent[] { | ||
const r = this.dict[roomId]; | ||
if (!r) { | ||
return stateKey === undefined ? [] : null; | ||
|
@@ -121,12 +122,7 @@ export class StateLookup { | |
() => this._client.roomState(roomId) | ||
); | ||
events.forEach((ev) => { | ||
if (this.eventTypes[ev.type]) { | ||
if (!r.events[ev.type]) { | ||
r.events[ev.type] = {}; | ||
} | ||
r.events[ev.type][ev.state_key] = ev; | ||
} | ||
this.insertEvent(r, ev); | ||
}); | ||
return r; | ||
} | ||
|
@@ -191,9 +187,22 @@ export class StateLookup { | |
} | ||
|
||
// blunt update | ||
if (!r.events[event.type]) { | ||
r.events[event.type] = {}; | ||
this.insertEvent(r, event); | ||
} | ||
|
||
private insertEvent(roomSet: StateLookupRoom, event: StateLookupEvent) { | ||
if (typeof event.content !== "object") { | ||
// Reject - unexpected content type | ||
return; | ||
} | ||
if (!event.type || !event.state_key) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for learning something: This broke the tests. It's not testing if these keys are missing (as claimed by the line below), but if they are truethy. The tests use empty strings, which aren't truethy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in deb2f38 |
||
// Reject - missing keys | ||
return; | ||
} | ||
// blunt update | ||
if (!roomSet.events[event.type]) { | ||
roomSet.events[event.type] = {}; | ||
} | ||
r.events[event.type][event.state_key] = event; | ||
roomSet.events[event.type][event.state_key] = event; | ||
} | ||
} |
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.
Is content guaranteed to be an
Object
or claimed as hereunknown
? That would require some more type checks in my PR.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.
We could probably call it a
Record<string,unknown>
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.
But if that's not guaranteed by the client lib, we'll get errors like the one #155 fixes, where a presumed object may be a string or a number.
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.
Content should really never be anything other than a object, if it is, we should reject it before insertion?
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.
e027f07