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

Need clarity on the browser vs non-browser version relationship #2903

Open
NathanC opened this issue Nov 22, 2022 · 8 comments
Open

Need clarity on the browser vs non-browser version relationship #2903

NathanC opened this issue Nov 22, 2022 · 8 comments

Comments

@NathanC
Copy link

NathanC commented Nov 22, 2022

I'm setting up a Vite + React + matrix-js-sdk project (but not using the matrix-react-sdk), and I'm trying to understand how this project is laid out.

To get things to work initially, I had to install the npm events package. I loaded Olm using wasm. Then I tried to use the sdk, and had a lot of issues with it referencing a global object that did not exist, so I do this before loading the sdk:

global = {
     window: window,
     fetch: fetch.bind(window),
     Olm: window.Olm,
     localStorage: window.localStorage
};

I then followed the pattern Element is using of importing matrix-js-sdk/src/browser-index at the root of my project, and importing all the sdk modules from matrix-js-sdk/src (instead of matrix-js-sdk top level). This caused > 350 typescript compilation errors, as it tried to compile the /src directory of the sdk, and there are a lot of errors such as indexing Partial objects and not checking if the value is defined. I tried downgrading my typescript version, but no dice.

I ended up removing the brower-index import, using matrix-js-sdk imports instead of matrix-js-sdk/src imports, and actually entirely removing my importing of the browser version that I downloaded from github.

And it..somewhat works? I can log in and interact with the server, and send encrypted messages.

tl;dr; it seems to be working now even if I don't download the browser version at all or import browser-index. What am I missing? I'm just looking for some clarity on the reason the setup is the way it is and how I should understand/interact with it, as I haven't been able to find much documentation and have just been trying to reverse engineer Element and the react sdk. A quick overview explanation would be very helpful

@t3chguy
Copy link
Member

t3chguy commented Nov 22, 2022

browser-index is predominantly for the browser-matrix browserify bundle that is offered, but is not strictly necessary when using a bundler like Vite. It used to be different before the move to fetch and webcrypto

@NathanC
Copy link
Author

NathanC commented Nov 22, 2022

I only care about supporting more modern browsers. what's a tl;dr; on what the browserify bundle offers that the standard npm package doesn't? It it just events and some polyfills?

I'm new to Vite (this was actually the first project I used it for), but I may try to make some contributions down the line to make the sdk easier to use in Vite once I understand it better. Even just having documentation on what sort of things are expected on the global/window object, or potentially falling back to window so there's no need to create a global implicit var would be useful.

@NathanC
Copy link
Author

NathanC commented Nov 22, 2022

Before I added the global object, it fell to here, which caused Vite to say that crypto and util were not supported and were being externalized.

@t3chguy
Copy link
Member

t3chguy commented Nov 22, 2022

It it just events and some polyfills?

Yes but also doesn't let you bundle your code together, and might be over-polyfilled for your requirements. We don't use or really test the browserify bundle it is just offered for historical reasons.

I haven't used Vite myself ftr

WebCrypto & TextEncoder are available natively in the web - https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/crypto/crypto.ts#L19-L21

@NathanC
Copy link
Author

NathanC commented Nov 22, 2022

Ahh, I thought those were being attached to the window object by libOlm, didn't realize they were native.

However, the code you linked doesn't work for me unless I set up the global object before using the sdk—global doesn't exist by default in the Vite context, just window. How would you feel about me adding some logic in crypto.ts to fall to window if global isn't defined?

@t3chguy
Copy link
Member

t3chguy commented Nov 22, 2022

You could, though it'd need to be careful to not hit an Uncaught ReferenceError

image

@YousefED
Copy link

I also noticed process, buffer and global needs to be polyfilled. Done it like this for Vite: https://github.com/YousefED/Matrix-CRDT/blob/main/examples/todo-simple-react-vite/src/fixMatrixSDK.ts

orbiteleven added a commit to bwllaming/matrix-js-sdk that referenced this issue Sep 30, 2023
Switches use of `global` to `globalThis`, which is better supported when building with modern build tools like Vite.

Refs matrix-org#2903
orbiteleven added a commit to bwllaming/matrix-js-sdk that referenced this issue Sep 30, 2023
Switches use of `global` to `globalThis`, which is better supported when building with modern build tools like Vite.

Refs matrix-org#2903

Signed-off-by: Damon Vestervand <damon@beyondwork.ai>
Signed-off-by: Damon <damon@vestervand.net>
@richvdh
Copy link
Member

richvdh commented Oct 2, 2023

I'm not really clear on what remains to be done on this issue. The summary is "Need clarity..." and some clarity has been given I think, so ... can we close it (please)?

github-merge-queue bot pushed a commit that referenced this issue Oct 2, 2023
Switches use of `global` to `globalThis`, which is better supported when building with modern build tools like Vite.

Refs #2903

Signed-off-by: Damon Vestervand <damon@beyondwork.ai>
Signed-off-by: Damon <damon@vestervand.net>
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

No branches or pull requests

4 participants