Skip to content

Add support for to-device messages via OLM#303

Merged
dbkr merged 5 commits intomainfrom
to-device-olm2
May 17, 2022
Merged

Add support for to-device messages via OLM#303
dbkr merged 5 commits intomainfrom
to-device-olm2

Conversation

@robertlong
Copy link
Contributor

@robertlong robertlong commented Apr 26, 2022

}

export async function initClient(clientOptions) {
await addScript(olmJsPath);
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary as well as the hardcoded script addition in index.html? I think you should be able to import Olm as a module to avoid both (https://github.com/vector-im/element-web/blob/develop/src/vector/init.tsx#L87).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've made a change to import it that way. One gotcha I found is that @matrix-org/olm expects OLM_OPTIONS to exist in the global context. Not sure why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added an issue to the Olm repository here https://gitlab.matrix.org/matrix-org/olm/-/issues/10

Copy link
Member

Choose a reason for hiding this comment

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

Thanks - needing OLM_OPTIONS defined is a bit weird - we don't seem to need it in element-web - not sure why though.


const storeOpts = {};

if (indexedDB && localStorage && !import.meta.env.DEV) {
Copy link
Member

Choose a reason for hiding this comment

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

Fwiw I think the bit you need is the crypto store below - this shouldn't really be necessary to get crypto working (although it will speed up your subsequent loads).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah Matthew and I discussed adding this to Element Call so I figured it was good to do it here in addition to the crypto store.

@robertlong
Copy link
Contributor Author

So we have a staging environment setup on main now. Should we merge this and test it out?

@dbkr dbkr merged commit 6913fdd into main May 17, 2022
@daniel-abramov daniel-abramov deleted the to-device-olm2 branch July 19, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants