matrix-org / matrix-react-sdk-module-api

API surface for writing Modules for the react-sdk
Apache License 2.0
3 stars 8 forks source link

Add extension mechanism, and extension points to replace deprecated Security customisations. #31

Closed thoraj closed 8 months ago

thoraj commented 9 months ago

Background

The purpose of this PR is to provide a replacement for the deprecated Security customisations. See #24

The PR brings two main contributions:

Method call extension mechanism

This will be in addition to the already existing mechanism to raise lifecycle events, so (if this PR is accepted), there will be two main ways to extension modules to "interact" with react-sdk:

The reason both are needed is that the semantics of raising events, and calling methods is fundamenally different. Raising events is fire and forget, and the calling code is indifferent to whether anything is listening or not. Therefore raising events is not an appropriate mechanism for returning data, and possibly changing program flow.

The method call semantics are appropriate if e.g. program flow should be modified based on the results of the call (which is how the deprecated Security customisation works).

Introducing an extension

Extensions are introduced by adding code in this repo and in matrix-react-sdk. This of course means that new extensions are introduced by the Element team either by merging constributions (like this one), or by authoring new extensions themselves. regardless, to create a new method call one must:

Implementing an extension in an extension module

Extension modules are created by developers wishing to extend EW, by deriving from the RuntimeModule base class. This PR adds a property to expose extension methods. An extension module may or may not provide an implementation of the available extension method interface. However, only one extension module can provide an implementation of an extension method interface. This is enforced in react-sdk when the extension modules are registered (and thus not part of this PR)

Changes to the matrix-react-sdk (separate PR)

For everything to come together, there will changes needed in matrix-react-sdk as well. These will be proposed in a separate PR which will show the details, but in short the PR will:

Future extensions

Although the scope of this PR is to address #24, it would also provide an easy way to port other customisations such as e.g. ComponentVisibility.ts

Signed-off-by: Thor A. Johansen taj@verji.com

t3chguy commented 9 months ago

Don't forget to request a re-review when it is ready for such.

image

thoraj commented 9 months ago

Not sure how to request another review. On my side it looks like this: image

(review from @dbkr already requested, and no button to re-request review from @t3chguy)

thoraj commented 8 months ago

I hope I requested a re-review correctly?

I understand you are very busy, but it would be great to know what to expect wrt lead time, and when this can/will happen? If the PR is approved (and released), it will be much simpler to create accompanying PR for matrix-react-sdk, and also to prepare porting other customisations.

thoraj commented 8 months ago

@t3chguy ping ☝️

thoraj commented 8 months ago

We are eager to continue our work and make contributions to the EW ecosystem.

If what is missing in the PR is cleaning up formatting and code style, we can go ahead and make additional PRs for finishing the extension, as well as porting more customisations. If you have issues with the design and implementation in this PR, we will definitely wait for your re-review before spending more time.

Either way it would be great if we could nudge this PR along, so we kindly ask if we can get a re-review (@t3chguy @dbkr )

t3chguy commented 8 months ago

Sorry I was away sick @thoraj. I will try get to it today but have a significant amount to catch up on.