vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
12.75k stars 1.15k forks source link

0.34.3 doens't work with `Uint8Array` and `TextEncoder` #4043

Open wxiaoguang opened 1 year ago

wxiaoguang commented 1 year ago

Describe the bug

The test works in 0.34.2, but doesn't work in 0.34.3

Reproduction

test('toEqual', () => {
  expect([1]).toEqual([1]); // OK
  expect(new TextEncoder().encode('a')).toEqual(new TextEncoder().encode('a')); // OK
  expect(Uint8Array.from([97])).toEqual(new TextEncoder().encode('a')); // FAIL: AssertionError: expected Uint8Array[ 97 ] to deeply equal Uint8Array[ 97 ]
});

System Info

node: v20.5.0, macOS

Used Package Manager

npm

Validations

paul-sachs commented 1 year ago

I believe this also effects esbuild directly. Using vitest with a utility that calls esbuild results in:

Error: Invariant violation: "new TextEncoder().encode("") instanceof Uint8Array" is incorrectly false

This indicates that your JavaScript environment is broken. You cannot use
esbuild in this environment because esbuild relies on this invariant. This
is not a problem with esbuild. You need to fix your environment instead.
wxiaoguang commented 1 year ago

This change seems quite suspicious https://github.com/vitest-dev/vitest/pull/3203

And this one: https://github.com/vitest-dev/vitest/pull/3998 (b42cf)

silverwind commented 1 year ago

I think https://github.com/vitest-dev/vitest/pull/3998 is likely the culprit. Something about the Uint8Array returned by TextEncoder makes it fail, even thought those values are definitely structurally equal.

silverwind commented 1 year ago

The cause is this removal, specifically:

TextEncoder (and TextDecoder) is a Node API and Uint8Array is a jsdom API. The Uint8Array that both produce are not comparable, even while structurally ident (I guess some prototype stuff differs which breaks toEqual).

https://github.com/jestjs/jest/issues/9983 has more context and https://github.com/jsdom/jsdom/issues/2524 would be the proper fix, but it's stale.

How does vitest want to proceed? I see two options:

Both are fine with me, but the current situation where only TextEncoder and TextDecoder are from node is problematic.

sheremet-va commented 1 year ago

The cause is this removal

The issue doesn't specify what mode it is running, so I assume it's not experimental-vm (TextEncoder should not even be available there). This line only affects --experimental-vm-threads.

But yeah, this one is quite hard to fix in general because jsdom uses its internal Uint8Array for its APIs, and Node.js uses its own implementation so they clash in different APIs, just like with URL.

silverwind commented 1 year ago

Config is pretty much default configuration. TextEncoder is global in node since v11, so unless vitest deletes these globals (maybe it should), they will be present in non-node environments. I've worked around the issue now by converting to array for comparison.

vighnesh153 commented 1 year ago

I've worked around the issue now by converting to array for comparison.

I am still seeing this issue. Is the workaround published as part of 0.34.4 or is it still pending to be published?

deot commented 1 year ago

Temporarily fix for node environment:

// @vitest-environment node
shunjizhan commented 1 year ago

got the same issue with vitest 0.34.3, prev versions worked as expected.

For the below function (source), isU8a(someUint8Array) returns false, while it should return ture

export function isU8a (value?: unknown): value is Uint8Array {
  // here we defer the instanceof check which is actually slightly
  // slower than just checking the constrctor (direct instances)
  return (
    ((value && (value as Uint8Array).constructor) === Uint8Array) ||
    value instanceof Uint8Array
  );
}
vighnesh153 commented 1 year ago

Is it not covered in the specs? u8a.spec.ts

shunjizhan commented 1 year ago

Is it not covered in the specs? u8a.spec.ts

it is covered, but vitest 0.34.3 has this bug that causes it to fail, the isU8a method itself is fine

7rulnik commented 1 year ago

@sheremet-va btw what is the correct way to use AbortController, TextEncoder and others within experimentalVmThreads and jsdom/happy-dom environment?

sheremet-va commented 1 year ago

btw what is the correct way to use AbortController, TextEncoder and others within experimentalVmThreads and jsdom/happy-dom environment?

Environments (jsdom/happy-dom) should provide these APIs.

7rulnik commented 1 year ago

@sheremet-va and as I understand it's impossible to transfer (assign it to window/global in setup file) classes like AbortController or TextEncoder to VM threads. So they should be polyfilled with things that exist in VM threads. Am I right?

sheremet-va commented 1 year ago

and as I understand it's impossible to transfer (assign it to window/global in setup file) classes like AbortController or TextEncoder to VM threads

It is possible to pass down implementations to VM threads, but some APIs will not accept them, while others will.

For example, if we use node.Uint8Array, then its .buffer will be node.ArrayBuffer, but we already define global ArrayBuffer from jsdom, so there will be a mismatch. Ok, let's then assign global ArrayBuffer to Node's. In this case JSDOM API that returns a buffer, will return JSDOM buffer and instanceof checks will fail:

const blob = new Blob(['Hello'], { type: 'text/plain' })

const arraybuffer = await new Promise<ArrayBuffer>((resolve, reject) => {
  const reader = new FileReader()
  reader.onload = () => resolve(reader.result as ArrayBuffer)
  reader.onerror = () => reject(reader.error)
  reader.readAsArrayBuffer(blob)
})

expect(arraybuffer instanceof ArrayBuffer).toBeTruthy() // fails, because ArrayBuffer is node.ArrayBuffer, but FileReader resolves to jsdom.ArrayBuffer

If we use jsdom.Uint8Array, then Node.js API that checks for it will fail (see #4175). So even if JSDOM implements TextEncoder, most Node.js APIs will reject JSDOM's Uint8Array.

I am not sure where the middle ground is, and open to suggestions.

sheremet-va commented 1 year ago

Also, looks like happy-dom doesn't have these issues.

7rulnik commented 1 year ago

Oh, yeah, I remember this infamous jest issue https://github.com/jestjs/jest/issues/2549 Thank you for the detailed description!

And yeah, it feels like the latest happy-dom has many globals. At least, Request, AbortController, AbortSignal. And it's easy to polyfill TextEncoder/TextDecoder via https://github.com/samthor/fast-text-encoding

smcstewart commented 8 months ago

Temporarily fix for node environment:

// @vitest-environment node

Super simple workaround that actually works. Thanks! Put it at the top of your test spec file.

kyledetella commented 8 months ago

I was banging my head against the wall for a while on this one. None of the solutions I encountered here (or in the JSDom, Jest, or ESBuild repos) were working for my use case. Ultimately, what I ended up doing to get this fixed was to introduce a shim of TextEncoder lifted from the text-encoder library.

First, in my vite.config.ts file:

// vite.config.ts
import "./setupTestEnvironmentCompatibility"; // This MUST be the first import
// ...

And then the setupTestEnvironmentCompatibility file looks like this:

// Only overwrite this if we are in test mode. This is an env var we happen to have in our repo
if (process.env.VITE_IS_TEST === "true") {
  class ESBuildAndJSDOMCompatibleTextEncoder extends TextEncoder {
    constructor() {
      super();
    }

    encode(input: string) {
      if (typeof input !== "string") {
        throw new TypeError("`input` must be a string");
      }

      const decodedURI = decodeURIComponent(encodeURIComponent(input));
      const arr = new Uint8Array(decodedURI.length);
      const chars = decodedURI.split("");
      for (let i = 0; i < chars.length; i++) {
        arr[i] = decodedURI[i].charCodeAt(0);
      }
      return arr;
    }
  }

  Object.defineProperty(global, "TextEncoder", {
    value: ESBuildAndJSDOMCompatibleTextEncoder,
    writable: true,
  });
}

Hope this helps others.

JoshuaCWebDeveloper commented 5 months ago

Following @kyledetella's workaround, putting the import:

import "./setupTestEnvironmentCompatibility";

in my vite.config.ts file did not work; however, putting it at the top of every *.spec.ts file that was giving me the error (in my case only 2) did work.

eduardhasanaj commented 2 days ago

@sheremet-va What is the status of this issue? I tried @kyledetella way but could fix it: I got same error.

KholdStare commented 1 day ago

@eduardhasanaj I am not sure why @kyledetella inserted the code into vite.config.ts directly - that may be a problem for you.

Since this issue is with the JSDom environment and vitest, I put a slightly simplified version of @kyledetella's code in my setupTests.ts which gets run by vitest when it starts the tests.

// JSDom + Vitest don't play well with each other. Long story short - default
// TextEncoder produces Uint8Array objects that are _different_ from the global
// Uint8Array objects, so some functions that compare their types explode.
// https://github.com/vitest-dev/vitest/issues/4043#issuecomment-1905172846
class ESBuildAndJSDOMCompatibleTextEncoder extends TextEncoder {
  constructor() {
    super();
  }

  encode(input: string) {
    if (typeof input !== "string") {
      throw new TypeError("`input` must be a string");
    }

    const decodedURI = decodeURIComponent(encodeURIComponent(input));
    const arr = new Uint8Array(decodedURI.length);
    const chars = decodedURI.split("");
    for (let i = 0; i < chars.length; i++) {
      arr[i] = decodedURI[i].charCodeAt(0);
    }
    return arr;
  }
}

global.TextEncoder = ESBuildAndJSDOMCompatibleTextEncoder;

In vite.config.ts config:

{
  // ...
  test: {
    setupFiles: ["src/setupTests.ts"],
  },
}
eduardhasanaj commented 1 day ago

@KholdStare Thank you very much for you time. It worked! I spent a lot of time searching for a callback where we could polyfill correctly but missed setup files option.