-
-
Notifications
You must be signed in to change notification settings - Fork 580
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 filters to reflect MSC3440 (threads) #2065
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.
The code overall seems fine, though does rely on unspecced behaviour. We need to figure out a way forward on that before continuing here, I think.
As a gauge of difficulty, the spec has previously rejected the change to allow optional from
. As a reference SDK we cannot (or rather, shouldn't) let ourselves become bound to Synapse behaviour.
Blocked, waiting on |
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.
This seems fine, though if matrix-org/matrix-spec-proposals#3567 fails for some reason then we'll need to backpedal on this change, so I would not recommend merging until the MSC has at least passed FCP. At that point it's stable enough for usage and we don't need to do awkward steering here in the js-sdk. Before FCP finishes, we have to do quite a few things that honestly aren't worth the effort.
Technically still unstable https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3roomsroomidmessages matrix-org/matrix-spec#1002 Synapse has supported this for over 2 years and Element web depends on it for threads. matrix-org/matrix-js-sdk#2065 Given that redactions are super heavy in Mjolnir already and have been reported as barely functional on matrix.org I believe we should also adopt this approach as if for some reason Synapse did change before the next release (extremely unlikely) we can revert this commit.
Technically still unstable https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3roomsroomidmessages matrix-org/matrix-spec#1002 Synapse has supported this for over 2 years and Element web depends on it for threads. matrix-org/matrix-js-sdk#2065 Given that redactions are super heavy in Mjolnir already and have been reported as barely functional on matrix.org I believe we should also adopt this approach as if for some reason Synapse did change before the next release (extremely unlikely) we can revert this commit.
Technically still unstable https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3roomsroomidmessages matrix-org/matrix-spec#1002 Synapse has supported this for over 2 years and Element web depends on it for threads. matrix-org/matrix-js-sdk#2065 Given that redactions are super heavy in Mjolnir already and have been reported as barely functional on matrix.org I believe we should also adopt this approach as if for some reason the spec did change before the next release (1.3) (extremely unlikely) we can revert this commit.
At the moment we call `/initialSync` to give a `from` token to `/messages`. In this PR we instead do not provide a `from` token when calling `/messages`, which has recently been permitted in the spec Technically this is still unstable in the spec https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3roomsroomidmessages matrix-org/matrix-spec#1002 Synapse has supported this for over 2 years and Element web depends on it for threads. matrix-org/matrix-js-sdk#2065 Given that redactions are super heavy in Mjolnir already and have been reported as barely functional on matrix.org I believe we should also adopt this approach as if for some reason the spec did change before the next release (1.3) (extremely unlikely) we can revert this commit.
* Remove the need to call `/initialSync` in `getMessagesByUserIn`. At the moment we call `/initialSync` to give a `from` token to `/messages`. In this PR we instead do not provide a `from` token when calling `/messages`, which has recently been permitted in the spec Technically this is still unstable in the spec https://spec.matrix.org/unstable/client-server-api/#get_matrixclientv3roomsroomidmessages matrix-org/matrix-spec#1002 Synapse has supported this for over 2 years and Element web depends on it for threads. matrix-org/matrix-js-sdk#2065 Given that redactions are super heavy in Mjolnir already and have been reported as barely functional on matrix.org I believe we should also adopt this approach as if for some reason the spec did change before the next release (1.3) (extremely unlikely) we can revert this commit.
This PR currently has no changelog labels, so will not be included in changelogs.
Add one of:
T-Deprecation
,T-Enhancement
,T-Defect
,T-Task
to indicate what type of change this is plusX-Breaking-Change
if it's a breaking change.