Update filters to reflect MSC3440 (threads) #2065
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
turt2live
requested changes
Dec 13, 2021
There was a problem hiding this comment.
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 |
added 2 commits
December 20, 2021 09:32
turt2live
approved these changes
Dec 20, 2021
There was a problem hiding this comment.
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.
Gnuxie
added a commit
to matrix-org/mjolnir
that referenced
this pull request
May 17, 2022
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.
Gnuxie
added a commit
to matrix-org/mjolnir
that referenced
this pull request
May 17, 2022
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.
Gnuxie
added a commit
to matrix-org/mjolnir
that referenced
this pull request
May 17, 2022
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.
Gnuxie
added a commit
to matrix-org/mjolnir
that referenced
this pull request
May 17, 2022
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.
Gnuxie
added a commit
to matrix-org/mjolnir
that referenced
this pull request
May 24, 2022
* 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR currently has no changelog labels, so will not be included in changelogs.
Add one of:
T-Deprecation,T-Enhancement,T-Defect,T-Taskto indicate what type of change this is plusX-Breaking-Changeif it's a breaking change.