Skip to content
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

Add support for to-device messages via OLM #303

Merged
merged 5 commits into from
May 17, 2022
Merged

Add support for to-device messages via OLM #303

merged 5 commits into from
May 17, 2022

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