Declaring a function with name ''then" makes vitest runner to halt

pbalenzuela commented 2 weeks ago

Describe the bug

It seems that ~~some special names~~ some special function names cause vitest to halt the test run. In this case the problem is a function called then ~~but I think that the problem also happens with names like it and others that I can't remember. I also think that the problem happens with any identifier: variables, classes, functions, etc~~

Reproduction

Go ahead and clone https://github.com/pbalenzuela/vitest-problem/commits/main/
install dependencies using pnpm
1. run pnpm vitest
2. test runs and passes
reset to the commit that shows the problem
1. git reset --hard 7d4a9d002e754e0ee7ffea6f2625afb623d75a0f
2. run pnpm vitest
3. test runners does't provide any feedback, gets stuck and doesn't finish

System Info

System:
    OS: macOS 15.1
    CPU: (11) arm64 Apple M3 Pro
    Memory: 67.91 MB / 18.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.16.0 - ~/.nvm/versions/node/v20.16.0/bin/node
    npm: 10.8.1 - ~/.nvm/versions/node/v20.16.0/bin/npm
    pnpm: 9.12.3 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 130.0.6723.91
    Safari: 18.1
  npmPackages:
    vitest: ^2.1.4 => 2.1.4

Used Package Manager

pnpm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

AriPerkkio commented 2 weeks ago

Duplicate of https://github.com/vitest-dev/vitest/issues/5122, see https://github.com/vitest-dev/vitest/issues/5122#issuecomment-2406488169.

sensiblejay commented 2 weeks ago

do you have no way to do anything but an infinite loop at this point? a test suite that silently hangs on an internal error is failing at its one job

edit: it may be important to note that it works perfectly fine in non-vitest scenarios

AriPerkkio commented 2 weeks ago

it may be important to note that it works perfectly fine in non-vitest scenarios

Vitest transforms the static imports into dynamic ones when run on Node. When you run script below on NodeJS, you can see how weirdly even Node works when you have named export then:

import { writeFileSync } from "node:fs";

writeFileSync("./first.mjs", `export const example = "first"`);
await import("./first.mjs").then((mod) => console.log("First:", mod));

writeFileSync(
  "./second.mjs",
  `
export const example = "second";
export async function then() {};
`
);

await import("./second.mjs").then((mod) => console.log("Second:", mod));

$ node index.mjs 
First: [Module: null prototype] { example: 'first' }
# Where's second?

It's recommended to follow MDN's advice and not export then from modules: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import#module_namespace_object

pbalenzuela commented 2 weeks ago

In the case of jest it is not a problem because it doesn't use ES Modules. As today, working with ES Modules is at experimental stage v29.7

AriPerkkio commented 2 weeks ago

do you have no way to do anything but an infinite loop at this point?

Not sure about that, maybe not. But at least on Node v22 you should see following warning:

$ node index.mjs 
First: [Module: null prototype] { example: 'first' }

Warning: Detected unsettled top-level await at file:///x/x/x/repros/then/index.mjs:23
const second = await import("./second.mjs");

pbalenzuela commented 2 weeks ago

We used to use https://www.npmjs.com/package/unplugin-auto-import plugin and then we decided to remove it because it caused similar problems to this one. This fact worried us because thinking about how you name things is a problem.

It seems that the only identifier that could cause a problem is then so I think that is not a big of a deal and, as @AriPerkkio said, it is a MDN recommendation not to use it.

I this branch there's a proof that other vitest identifiers won't cause a problem unless you use globals:

export const afterAll = () => undefined;
export const afterEach = () => undefined;
export const assert = () => undefined;
export const beforeAll = () => undefined;
export const beforeEach = () => undefined;
export const describe = () => undefined;
export const expect = () => undefined;
export const it = () => undefined;
export const suite = () => undefined;
export const test = () => undefined;
export const vi = () => undefined;
export const vitest = () => undefined;

vitest-dev / vitest