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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

TextEncoder issues on 26.0.1 #9983

Closed
jacogr opened this issue May 6, 2020 · 16 comments
Closed

TextEncoder issues on 26.0.1 #9983

jacogr opened this issue May 6, 2020 · 16 comments

Comments

@jacogr
Copy link

jacogr commented May 6, 2020

馃悰 Bug Report

The TextEncoder.encode does not seem to yield a valid U8a, or rather, it cannot be compared. When the output from this is passed into a function expecting a Uint8Array it does indeed fail.

To Reproduce

This is a failing test -

it('encodes string as Uint8array', (): void => {
    expect(
      new TextEncoder().encode('袩褉懈胁械褌, 屑懈褉!')
    ).toEqual(
      new Uint8Array([208, 159, 209, 128, 208, 184, 208, 178, 208, 181, 209, 130, 44, 32, 208, 188, 208, 184, 209, 128, 33])
    );
  });

The output does not match, rather it fails with -

    expect(received).toEqual(expected) // deep equality

    - Expected  - 23
    + Received  + 23

    - Uint8Array [
    -   208,
    -   159,
    -   209,
    -   128,
    -   208,
    -   184,
    -   208,
    -   178,
    -   208,
    -   181,
    -   209,
    -   130,
    -   44,
    -   32,
    -   208,
    -   188,
    -   208,
    -   184,
    -   209,
    -   128,
    -   33,
    - ]
    + Uint8Array {
    +   "0": 208,
    +   "1": 159,
    +   "10": 209,
    +   "11": 130,
    +   "12": 44,
    +   "13": 32,
    +   "14": 208,
    +   "15": 188,
    +   "16": 208,
    +   "17": 184,
    +   "18": 209,
    +   "19": 128,
    +   "2": 209,
    +   "20": 33,
    +   "3": 128,
    +   "4": 208,
    +   "5": 184,
    +   "6": 208,
    +   "7": 178,
    +   "8": 208,
    +   "9": 181,
    + }

Expected behavior

It was still working on the 25.4.x version before I updated.

Link to repl or repo (highly encouraged)

Failing test included above.

envinfo

  System:
    OS: macOS 10.15.2
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 13.8.0 - /usr/local/bin/node
    Yarn: 2.0.0-rc.30 - /usr/local/bin/yarn
    npm: 6.13.7 - /usr/local/bin/npm
@SimenB
Copy link
Member

SimenB commented May 7, 2020

@jacogr Does this happen only in JSDOM environment? If yes, can you try removing this line in your node_modules directory: https://github.com/facebook/jest/blob/4bd3d4a05999170f423f7050d4e0537648499e88/packages/jest-environment-jsdom/src/index.ts#L49

@TrySound
Copy link
Contributor

TrySound commented May 7, 2020

This example still fails.
#9993

@jacogr
Copy link
Author

jacogr commented May 7, 2020

It does indeed fail like @TrySound suggested, even with the line commented.

However, I can make both test cases pass by adding testEnvironment: "node" to the config.

eemeli added a commit to eemeli/yaml that referenced this issue May 9, 2020
Also switch Jest testEnvironment to "node", due to this bug:
jestjs/jest#9983
@SimenB
Copy link
Member

SimenB commented May 10, 2020

Hmm, seems like JSDOM doesn't support TextEncoder at all - jsdom/jsdom#2524. Not sure why it's available in jest-environment-jsdom, or why it worked before.

Is this

Each jsdom Window now exposes all JavaScript-spec-defined globals uniformly. When runScripts is disabled, it exposes them as aliases of the ones from the outer Node.js environment. Whereas when runScripts is enabled, it exposes fresh copies of each global from the new scripting environment. (Previously, a few typed array classes would always be aliased, and with runScripts disabled, the other classes would not be exposed at all.)

from https://github.com/jsdom/jsdom/blob/master/Changelog.md#1600?

Seems to be in progress here: jsdom/whatwg-encoding#11

@jeysal
Copy link
Contributor

jeysal commented May 10, 2020

Node 11+ supports the TextEncoder global as part of their efforts to align further with browser environments

@TrySound
Copy link
Contributor

@SimenB Look at my issue #9993. TextEncoder is not the only problem. Here simple buffer instanceof Uint8Array does not work in jsdom environment.

@TrySound
Copy link
Contributor

Also TextEncoder is available in Node.

@SimenB
Copy link
Member

SimenB commented May 10, 2020

What node supports and what jsdom supports is not the same thing. We usually point to jsdom for missing web APIs, this seems like the same thing?

@jacogr
Copy link
Author

jacogr commented May 11, 2020

I dug, in the case of no TextEncoder it injects a polyfill in the project where this was uncovered, which is this -

if (typeof TextEncoder === 'undefined') {
 global.TextEncoder = require('util').TextEncoder;
}

So the issue is exactly the same as @TrySound reported - since doing the above will no doubt use Buffer internally, so the test-case he has is the simplest and the actual underlying cause -

  it('has Buffer & Uint8array equivalency', () => {
    expect(Buffer.from('') instanceof Uint8Array).toBeTruthy();
  });

Sorry about the confusion, it has been literally 2 years since that polyfill was added. (Since it just worked in the meantime, totally lost track of it)

@chrisfosterelli
Copy link

We also polyfill this and have failing tests on JSDOM 16 with jest:

// Polyfill for encoding which isn't present globally in jsdom
import { TextEncoder, TextDecoder } from 'util'
global.TextEncoder = TextEncoder
global.TextDecoder = TextDecoder

Since Buffer.from('') instanceof Uint8Array evaluates true in node, I'm confused about where the problem actually is. Is JSDOM polyfilling in a slightly different implementation of Uint8Array?

@tgoorden
Copy link

Just adding the fact that this bug also breaks openpgpjs and was incredibly difficult to trace all the way to this unexpected jest peculiarity.

@andrevenancio
Copy link

Is there any progress on this issue?

SetupCoding pushed a commit to porsche-design-system/porsche-design-system that referenced this issue Sep 9, 2021
SetupCoding pushed a commit to porsche-design-system/porsche-design-system that referenced this issue Sep 9, 2021
SetupCoding pushed a commit to porsche-design-system/porsche-design-system that referenced this issue Sep 10, 2021
SetupCoding pushed a commit to porsche-design-system/porsche-design-system that referenced this issue Sep 10, 2021
@johnsmith-gooddollar
Copy link

  1. find the following line inside jest-environment-jsdom's index.js file:
global.ArrayBuffer = ArrayBuffer;
  1. add the following line after:
global.Uint8Array = Uint8Array; 
  1. use https://www.npmjs.com/package/patch-package to keep those changes after each npm i

@johnsmith-gooddollar
Copy link

also if you won't patch packages, you could create your own custom environment extending JSDOMEnvironment, add this override after super() call in the env class constructor and use path to this file as your testEnvironment. Not that you couldn't use ES6 stuff in the env file, so use require('jest-environment-jsdom') and module.exports to export your env class

@SimenB
Copy link
Member

SimenB commented Feb 21, 2022

The example in the OP fails in JSDOM env due to missing TextEncoder (jsdom/jsdom#2524) and passes with the node environment. So I'll consider this fixed

@SimenB SimenB closed this as completed Feb 21, 2022
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants