nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.52k stars 29.03k forks source link

type stripping + module mocking causes test runner to throw #54428

Open JakobJingleheimer opened 3 weeks ago

JakobJingleheimer commented 3 weeks ago

Version

22.6.0

Platform

Irrelevent

Subsystem

module

What steps will reproduce the bug?

  1. Create a typescript file (ex logger.ts)
  2. Create a test file (ex replace-js-ext-with-ts-ext.test.ts)
  3. Within the test file, set up a module mock

    const logger = mock.fn();
    
    before(async () => {
     mock.module('./logger.ts', {
       namedExports: { logger }
     });
    
     await import('./replace-js-ext-with-ts-ext.ts');
    });
  4. Run the test (ex node --experimental-test-module-mocks --experimental-strip-types --test ./replace-js-ext-with-ts-ext.test.ts)

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

100%

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

It should behave the same as when the module mock target is a node builtin or a javascript file.

What do you see instead?

  TypeError [ERR_INVALID_ARG_VALUE]: The argument 'format' must be one of: 'builtin', 'commonjs', 'module'. Received 'module-typescript'
      at MockTracker.module (node:internal/test_runner/mock/mock:517:7)
      at SuiteContext.<anonymous> (file:///[…]/JakobJingleheimer/correct-ts-specifiers/replace-js-ext-with-ts-ext.test.ts:37:23)
      at TestHook.runInAsyncScope (node:async_hooks:206:9)
      at TestHook.run (node:internal/test_runner/test:865:25)
      at TestHook.run (node:internal/test_runner/test:1116:18)
      at TestHook.run (node:internal/util:545:20)
      at node:internal/test_runner/test:785:20
      at async Suite.runHook (node:internal/test_runner/test:783:7)
      at async Suite.run (node:internal/test_runner/test:1220:7)
      at async Test.processPendingSubtests (node:internal/test_runner/test:574:7) {
    code: 'ERR_INVALID_ARG_VALUE'
  }

Additional information

No response

RedYetiDev commented 3 weeks ago

I can't reproduce:

import { before, mock } from 'node:test';

const logger = mock.fn();

before(async () => {
  mock.module('./logger.ts', {
    namedExports: { logger }
  });

  await import('./replace-js-ext-with-ts-ext.ts');
});
(node:745974) ExperimentalWarning: Type Stripping is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:745980) ExperimentalWarning: Type Stripping is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:745980) [MODULE_TYPELESS_PACKAGE_JSON] Warning: file:///index.test.ts parsed as an ES module because module syntax was detected; to avoid the performance penalty of syntax detection, add "type": "module" to /package.json
(node:745980) ExperimentalWarning: Module mocking is an experimental feature and might change at any time
✔ index.test.ts (93.695635ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 100.76966
JakobJingleheimer commented 3 weeks ago

You can see it here: https://github.com/JakobJingleheimer/correct-ts-specifiers/commit/6deda58539058ac0675d60afdd7d8d3cc69921c6

Just change the file extension of logger back to .ts and run npm test. It succeeds as .js and fails as .ts.

kruncher commented 2 weeks ago

I am seeing the same issue when experimenting with these two flags.

marco-ippolito commented 2 weeks ago

I cannot reproduce on main, can you check? I can actually

cjihrig commented 2 weeks ago

I haven't verified this issue, but I would expect it to be valid since the module-typescript (and others) format was introduced, and the module mocking code explicitly checks for one of these formats.

It might be that it's tricky to reproduce when ambiguous files are used?