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

fast-text-encoding usage breaks compatibility with Emscripten based libaries #947

Closed
curvedriver opened this issue Jan 19, 2021 · 4 comments · Fixed by #951 or #957
Closed

fast-text-encoding usage breaks compatibility with Emscripten based libaries #947

curvedriver opened this issue Jan 19, 2021 · 4 comments · Fixed by #951 or #957
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@curvedriver
Copy link

curvedriver commented Jan 19, 2021

Environment details:

OS: Linux
node: 10.23.1
npm: 6.14.10

  • @google-cloud/translate: 6.1.0
    which requires version ^2.9.2 of google-gax

Description:

google-gax 2.10.0 introduced fast-text-encoding, which breaks the compatibility with Emscripten based libaries.

if (typeof TextEncoder === 'undefined' || typeof TextDecoder === 'undefined') {

Error

RangeError: Failed to construct 'TextDecoder': The encoding label provided ('utf-16le') is invalid.

Caused by

https://github.com/emscripten-core/emscripten/blob/d7c7aedf45aafb72a8ca617fdfca98311f76525a/src/runtime_strings_extra.js#L38

Similar to:
googleapis/google-auth-library-nodejs#626

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Jan 20, 2021
@alexander-fenster
Copy link
Contributor

Hi @curvedriver,

Thank you for your report. Our main target is Node.js (which has both TextEncoder and TextDecoder starting from v11.0.0). I will try to fix it just as in googleapis/google-auth-library-nodejs#627 but since we must support Node v10 (which has no global TextDecoder), the logic will probably be a little bit uglier than that.

As soon as Node v10 goes EOL and we drop its support, the code will be exactly the same as in google-auth-library now. But for the time being, I'll try to make it work.

@alexander-fenster alexander-fenster self-assigned this Jan 29, 2021
@alexander-fenster alexander-fenster added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Jan 29, 2021
@alexander-fenster
Copy link
Contributor

@curvedriver The latest released version (v2.10.2) should have this problem fixed. Could you double check if it works for you? Thank you!

@curvedriver
Copy link
Author

Hi @alexander-fenster
Thanks for processing this issue so quickly!
Unfortunately, the reported error still happens in my case because I'm using nodejs 10.23.1.

What do you think about mapping the util.TextDecoder and util.TextEncoder to the global scope?

 const util = require('util');
  Object.assign(global, {
    TextDecoder: util.TextDecoder,
    TextEncoder: util.TextEncoder
  });

I didn't found any API changes of the TextEncoder & TextDecoder between node 10 & 11.

Do you think that there would any side-effects?

@alexander-fenster
Copy link
Contributor

@curvedriver I thought of doing it, and the only side effect here is that the browser bundlers (the main audience of src/fallback.ts) will probably start bundling Node.js util polyfill even if the require is within the conditional statement. I just checked it with webpack, the bundle size increased from 399K to 407K when I added require('util').

Well, maybe these 8K is not a big deal though, given that we'll be able to drop it back after April 30th (when we stop supporting Node.js v10). PR coming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
3 participants