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

Error when: node v20, --test --loader, await import #47614

Closed
iambumblehead opened this issue Apr 19, 2023 · 11 comments
Closed

Error when: node v20, --test --loader, await import #47614

iambumblehead opened this issue Apr 19, 2023 · 11 comments
Labels
confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders test_runner Issues and PRs related to the test runner subsystem. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.

Comments

@iambumblehead
Copy link

iambumblehead commented Apr 19, 2023

Version

v20.0.0

Platform

Darwin Bumbles-MBP.home 21.6.0 Darwin Kernel Version 21.6.0: Thu Mar 9 20:08:59 PST 2023; root:xnu-8020.240.18.700.8~1/RELEASE_X86_64 x86_64

What steps will reproduce the bug?

  • clone this repo or download this package
  • run npm test from the cloned or unpacked package
  • see the error

Essentially, using --loader and await import('./path/to/module.js') from inside any imported module causes the test to fail for no reason related to the test or any of the files used.

A copy of the error seen in the test process is attached here
testnode$ npm test    

> test
> node --experimental-loader=./anyloader.js --test

ℹ (node:55506) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
ℹ (Use `node --trace-warnings ...` to show where the warning was created)
✖ should not show promise resolution error (3.975453ms)
  Error: Promise resolution is still pending but the event loop has already resolved
      at process.emit (node:events:523:35)

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 0
ℹ cancelled 1
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 8.432653
ℹ THIS CONSOLE LOG IS PREINTED
ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 0
ℹ cancelled 1
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 150.112342

✖ failing tests:

✖ should not show promise resolution error (3.975453ms)
  Error: Promise resolution is still pending but the event loop has already resolved
      at process.emit (node:events:523:35)

How often does it reproduce? Is there a required condition?

The issue is reproduced every time on this machine, using the package attached above

What is the expected behavior? Why is that the expected behavior?

The test in the attached package should pass without any error, following the behaviour of previous versions of node from v12 onward.

What do you see instead?

Error: Promise resolution is still pending but the event loop has already resolved

In the example failing test, the test process reaches the passing condition but the test fails regardless

@panva
Copy link
Member

panva commented Apr 19, 2023

I've experienced the same problem in one my projects which uses ava test runner. Great job with a no-dependency reproduction.

Not sure if test_runner Issues and PRs related to the test runner subsystem. or loaders Issues and PRs related to ES module loaders . Better ping both.

cc @nodejs/loaders, @nodejs/test_runner

@panva panva added loaders Issues and PRs related to ES module loaders test_runner Issues and PRs related to the test runner subsystem. confirmed-bug Issues with confirmed bugs. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. labels Apr 19, 2023
@aduh95
Copy link
Contributor

aduh95 commented Apr 19, 2023

I suspect this is a duplicate of #47566.

@iambumblehead iambumblehead changed the title Error when: node v20, --loader, --test, await import Error when: node v20, --loader, await import Apr 22, 2023
@GeoffreyBooth
Copy link
Member

Can you please test now against latest main or nightly?

@aladdin-add
Copy link

@GeoffreyBooth I have tried the nightly, and it's still failing: https://github.com/iambumblehead/esmock/actions/runs/4804121738/jobs/8549299803?pr=199

@aduh95
Copy link
Contributor

aduh95 commented Apr 26, 2023

It seems to be the same thing as nodejs/help#3902 (comment)

@aduh95 aduh95 changed the title Error when: node v20, --loader, await import Error when: node v20, --test --loader, await import Apr 26, 2023
@MoLow
Copy link
Member

MoLow commented Apr 27, 2023

I have performed some testing, and it seems like the real issue is the combination of --test --loader, and --require.
on my machine this only reproduces inside vscode where NODE_OPTIONS is set to --require the bootloader file from https://github.com/microsoft/vscode-js-debug/blob/main/src/targets/node/bootloader.ts

@iambumblehead can you please compare your repro when running through vscode and through an external cli interface?

@MoLow
Copy link
Member

MoLow commented Apr 27, 2023

see this repro for example:
27e08d4

@iambumblehead
Copy link
Author

@MoLow politely, neither --require nor vscode were used to repro the issue reported here

@MoLow
Copy link
Member

MoLow commented Apr 27, 2023

@nodejs/loaders I have checked the repro provided by @iambumblehead, and the root cause is process.on('beforeExit' is called multiple times (node:test uses beforeExit to cancel all ongoing tests),

see this repro

// file.js
process.on('beforeExit', () => {
  console.log('beforeExit')
})
import('./anyfile.js')
console.log('THIS CONSOLE LOG IS PREINTED')
./node  file.js                                                                                              ✔
THIS CONSOLE LOG IS PREINTED
beforeExit
./node --experimental-loader="data:text/javascript,export default true"  file.js                             ✔
(node:62415) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
THIS CONSOLE LOG IS PREINTED
beforeExit
beforeExit
beforeExit

@koshic
Copy link

koshic commented May 4, 2023

Shorter example (20.1.0, macOS):

test.mjs

import { test } from "node:test";

import "fs"; // any other module

test("test", async (t) => {
  await import("data:text/javascript,");
});
node --experimental-loader 'data:text/javascript,' test.mjs

image

@MoLow
Copy link
Member

MoLow commented May 14, 2023

I have verified - this has been fixed via #47964

@MoLow MoLow closed this as completed May 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders test_runner Issues and PRs related to the test runner subsystem. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

No branches or pull requests

7 participants