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

Remove web-encoding dependency and use Nodejs built-in TextEncoder #1244

Open
aldy505 opened this issue Nov 22, 2023 · 6 comments
Open

Remove web-encoding dependency and use Nodejs built-in TextEncoder #1244

aldy505 opened this issue Nov 22, 2023 · 6 comments

Comments

@aldy505
Copy link
Contributor

aldy505 commented Nov 22, 2023

Since we're only supporting major new LTS versions[1] and we're just using TextEncoder on our web-encoding dependency, we might as well drop the dependency and just use Node.js' built-in TextEncoder. It should serve the same purpose like the one we currently use.

Request for comments.

[1] See discussion here: #1236 (comment) -- Node.js' TextEncoding has been around since Node.js v8 and stable as global object since Node.js v11.

@prakashsvmx
Copy link
Member

This npm package was updated to work both in nodejs and web browser environment. as of now, (broken) it does not work in browser environment, we need to check and fix once migration is complete.

@trim21
Copy link
Contributor

trim21 commented Nov 22, 2023

I think we don't need web-encoding or Nodejs built-in TextEncoder.

#1246 (comment)

@trim21
Copy link
Contributor

trim21 commented Nov 22, 2023

TextEncoder is for encoding utf-8 string to Uint8Array, which can be done by node's Buffer.from

@aldy505
Copy link
Contributor Author

aldy505 commented Nov 22, 2023

TextEncoder is for encoding utf-8 string to Uint8Array, which can be done by node's Buffer.from

@trim21 @prakashsvmx regarding the browser environment support: do we have a specific build (other than regular CJS and ESM) that targets Browser and converts every Node dependency into browser supported objects?

@prakashsvmx
Copy link
Member

prakashsvmx commented Nov 23, 2023

@aldy505 , we need to explore and investigate . because modern bundlers/builds do not export node built-ins for web and not all modules are ported (browserify) for web.

@aldy505
Copy link
Contributor Author

aldy505 commented Nov 24, 2023

@aldy505 , we need to explore and investigate . because modern bundlers/builds do not export node built-ins for web and not all modules are ported (browserify) for web.

Well, on browser, we already have https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder and on Node.js, it's already on the global scope and stable since v11 https://nodejs.org/api/globals.html#textencoder

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

3 participants