search

matrix-org / matrix-js-sdk

Matrix Client-Server SDK for JavaScript
Apache License 2.0
1.54k stars 580 forks source link

Avoiding base64-js library usage appears questionable #4341

Open turt2live opened 1 month ago

turt2live commented 1 month ago

See https://github.com/matrix-org/matrix-js-sdk/pull/4338 and in particular https://github.com/matrix-org/matrix-js-sdk/pull/4338#pullrequestreview-2222292684

dbkr commented 1 month ago

I'm still a bit confused by this: the point is not so much to avoid using base64-js if Webpack or whatever happens to be providing it, the point is that the js-sdk should not require it in order to work.

richvdh commented 1 month ago

I'm still a bit confused by this: the point is not so much to avoid using base64-js if Webpack or whatever happens to be providing it, the point is that the js-sdk should not require it in order to work.

well, why not? Isn't depending on base64-js better than requiring a polyfill of Buffer?

dbkr commented 1 month ago

Well, yes, but that's certainly not the intention. If that's what it's doing then in practice it's broken.

richvdh commented 1 month ago

well, then I think we're aligned.

TBH though, my feeling is that the attempt to avoid a dependency on base64-js has given us some complicated code that we don't really understand, and the cost isn't worth the benefit.

(aside: it might be nice if base64-js had an alternative entrypoint for nodejs which did something like Buffer.from(input).toString('base64') instead of the pure js impl. But that's a sidebar.)