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

Avoiding base64-js library usage appears questionable #4341

Open
turt2live opened this issue Aug 7, 2024 · 4 comments
Open

Avoiding base64-js library usage appears questionable #4341

turt2live opened this issue Aug 7, 2024 · 4 comments
Labels
T-Task Tasks for the team like planning

Comments

@turt2live
Copy link
Member

See #4338 and in particular #4338 (review)

@richvdh richvdh changed the title Avoiding base64 library usage appears questionable Avoiding base64-js library usage appears questionable Aug 7, 2024
@dbkr
Copy link
Member

dbkr commented Aug 8, 2024

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
Copy link
Member

richvdh commented Aug 8, 2024

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
Copy link
Member

dbkr commented Aug 8, 2024

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

@richvdh
Copy link
Member

richvdh commented Aug 8, 2024

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.)

@dbkr dbkr added the T-Task Tasks for the team like planning label Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

No branches or pull requests

3 participants