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

🐛 Bug: Hit an error with CJS named exports #5156

Closed
3 of 4 tasks
khteh opened this issue Jun 4, 2024 · 8 comments
Closed
3 of 4 tasks

🐛 Bug: Hit an error with CJS named exports #5156

khteh opened this issue Jun 4, 2024 · 8 comments
Labels
status: waiting for author waiting on response from OP - more information needed type: bug a defect, confirmed by a maintainer

Comments

@khteh
Copy link

khteh commented Jun 4, 2024

Bug Report Checklist

  • I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
  • I have searched for related issues and issues with the faq label, but none matched my issue.
  • I have 'smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, my usage of Mocha, or Mocha itself.
  • I want to provide a PR to resolve this

Expected

No runtime error

Actual

I have the following src/infrastructure/db.ts file:

@injectable()
export class Database {
}

and src/infrastructure/index.ts:

import config from 'config';
export * from "./db"

This file is referenced in src/webapi/routes/api.ts and hit the following error using rollup:

 Exception during run: file:///usr/src/Node.JSRestAPI/src/webapi/routes/api.ts:13
import { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } from "infrastructure";
                                                                          ^^^^^^^^
SyntaxError: Named export 'Database' not found. The requested module 'infrastructure' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'infrastructure';
const { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async formattedImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run.js:370:5)

Minimal, Reproducible Example

https://github.com/khteh/Node.JSRestAPI

Versions

Mocha: 10.4.0
Node: 22.2.0

Additional Info

npm run start does not hit this error.

@khteh khteh added status: in triage a maintainer should (re-)triage (review) this issue type: bug a defect, confirmed by a maintainer labels Jun 4, 2024
musanmaz added a commit to musanmaz/mocha that referenced this issue Jun 19, 2024
Fix CommonJS named export error in esm-utils (Issue mochajs#5156)
@JoshuaKGoldberg
Copy link
Member

I can't figure out how to get these tests to run in the repro. After npm i:

 $ npm run test

> [email protected] test
> export NODE_ENV=development&& ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --timeout 10000 --recursive --check-leaks --exit

(node:63540) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)

 Exception during run: tests/IntegrationTests/add_students_to_teacher_tests.ts(61,14): error TS2349: This expression is not callable.
  Type 'ChaiHttpRequest' has no call signatures.
tests/IntegrationTests/add_students_to_teacher_tests.ts(64,19): error TS7006: Parameter 'err' implicitly has an 'any' type.
tests/IntegrationTests/add_students_to_teacher_tests.ts(64,24): error TS7006: Parameter 'res' implicitly has an 'any' type.
tests/IntegrationTests/add_students_to_teacher_tests.ts(103,14): error TS2349: This expression is not callable.
  Type 'ChaiHttpRequest' has no call signatures.
tests/IntegrationTests/add_students_to_teacher_tests.ts(106,19): error TS7006: Parameter 'err' implicitly has an 'any' type.
tests/IntegrationTests/add_students_to_teacher_tests.ts(106,24): error TS7006: Parameter 'res' implicitly has an 'any' type.

...then if I add // @ts-ignores to those failing lines, it gives a different error:

$ npm run test

> [email protected] test
> export NODE_ENV=development&& ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --timeout 10000 --recursive --check-leaks --exit

(node:66735) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)

 Exception during run: Error: Cannot find module '/Users/josh/repos/Node.JSRestAPI/node_modules/webapi.core/build/index.js' imported from /Users/josh/repos/Node.JSRestAPI/src/webapi/routes/healthchecks.ts
    at finalizeResolution (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/dist-raw/node-internal-modules-esm-resolve.js:352:11)
    at moduleResolve (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/dist-raw/node-internal-modules-esm-resolve.js:801:10)
    at Object.defaultResolve (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/dist-raw/node-internal-modules-esm-resolve.js:912:11)
    at /Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/src/esm.ts:218:35
    at entrypointFallback (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/src/esm.ts:168:34)
    at /Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/src/esm.ts:217:14
    at addShortCircuitFlag (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/src/esm.ts:409:21)
    at resolve (/Users/josh/repos/Node.JSRestAPI/node_modules/ts-node/src/esm.ts:197:12)
    at nextResolve (node:internal/modules/esm/hooks:865:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:303:30)

Could you please post an isolated reproduction that I can quickly get running on my end, please?

@JoshuaKGoldberg JoshuaKGoldberg added status: waiting for author waiting on response from OP - more information needed and removed status: in triage a maintainer should (re-)triage (review) this issue labels Jul 3, 2024
@khteh
Copy link
Author

khteh commented Jul 4, 2024

You should run:

$ npm run build_all
$ npm run ts_test

However, I see the same errors with chai types recently and have created an issue here: chaijs/chai-http#334

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Jul 4, 2024

👍 that worked for me, thanks.

Interestingly, those specific errors go away if we switch the absolute "infrastructure" imports to relative ones:

- import { Database } from "infrastructure"
+ import { Database } from "../../infrastructure"

...but are replaced with equivalents from chai:

$ npm run test

> [email protected] test
> export NODE_ENV=development&& ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --timeout 10000 --recursive --check-leaks --exit

(node:53317) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)

 Exception during run: file:///Users/josh/repos/Node.JSRestAPI/tests/IntegrationTests/add_students_to_teacher_tests.ts:4
import chai from 'chai';
       ^^^^
SyntaxError: The requested module 'chai' does not provide an export named 'default'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:132:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:214:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:431:15)
    at async formattedImport (/Users/josh/repos/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/Users/josh/repos/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/Users/josh/repos/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/Users/josh/repos/Node.JSRestAPI/node_modules/mocha/lib/cli/run-helpers.js:158:3)
    at async Object.exports.handler (/Users/josh/repos/Node.JSRestAPI/node_modules/mocha/lib/cli/run.js:370:5)

These still happen if I rm mocharc.json and simplify the test command to:

npx mocha -r ts-node/register

The change in #5160 does not fix the chai or infrastructure issues for me (cc @musanmaz).


Anyway, for the "infrastructure" import issues, I believe this to be a misconfiguration in how your TypeScript / ts-node setup is working together. I see in your tsconfig.json you have "moduleResolution": "node", which is an alias for the deprecated "node10".

If you can make a minimal isolated reproduction (https://antfu.me/posts/why-reproductions-are-required#reproducible-projects-or-playgrounds) that shows an issue in Mocha, that'd be helpful. But given that we have multiple working examples in https://github.com/mochajs/mocha-examples of TypeScript code, I'm going to close this for now as userland misconfiguration.

Thanks for filing! ☕

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Jul 4, 2024
@khteh
Copy link
Author

khteh commented Jul 5, 2024

I changed the chai import before I replied you yesterday and chaijs/chai-http#334 is the list of the latest errors I observe. Thanks for the heads-up about the moduleResolution. I didn't follow the development of Node.JS closely enough. I have made the changes and pending chai errors resolved. Have a great weekend!

@khteh
Copy link
Author

khteh commented Jul 6, 2024

The original problem reported in this post surfaces with chaijs/chai-http#334 resolved. infrastructure is a workspace. There are 3 workspaces in the repo as defined in the top-level package.json. The symbols are exported in the individual workspaces. So, I can't import { Database } from "../../infrastructure" with module: "NodeNext" as it requres importing using file extension which doesn't work in this context. So, the original problem persists:

$ n run ts_test

> [email protected] ts_test
> export NODE_ENV=development && ./node_modules/.bin/nyc --timeout 10000 --recursive --check-leaks --reporter=lcov --reporter=text --reporter=text-summary --reporter=html ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_reports/mocha/test-results.xml --exit

(node:49284) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)
(node:49284) [DEP0180] DeprecationWarning: fs.Stats constructor is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)

 Exception during run: file:///usr/src/Node.JSRestAPI/src/webapi/routes/api.ts:13
import { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } from "infrastructure";
                                                                          ^^^^^^^^
SyntaxError: Named export 'Database' not found. The requested module 'infrastructure' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'infrastructure';
const { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async formattedImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run-helpers.js:158:3)
    at async Object.exports.handler (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run.js:370:5)

Attempting to import using ../../infrastructure results in:

routes/api.ts:14:91 - error TS2834: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

14 import { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } from "../../infrastructure"
                                                                                             ~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in routes/api.ts:14

npm error Lifecycle script `build` failed with error:
npm error code 1
npm error path /usr/src/Node.JSRestAPI/src/webapi
npm error workspace [email protected]
npm error location /usr/src/Node.JSRestAPI/src/webapi
npm error command failed
npm error command sh -c tsc --project tsconfig.json && npm run rollup
ERROR: "build_webapi" exited with 1.

I guess the question is how to import non-default exported symbols from another workspace using NodeNext module type?

@JoshuaKGoldberg
Copy link
Member

At this point, it looks like this is more of a help request / general question, not an issue in Mocha to be fixed. So I'll say this isn't the right medium to ask. You'd be better off on somewhere like StackOverflow or our Discord.

@khteh
Copy link
Author

khteh commented Jul 8, 2024

I still believe it is a problem with mocha. This error does not happen in npm run build_all and npm run start. Only in ./node_modules/.bin/mocha:

$ ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_reports/mocha/test-results.xml --exit
(node:20122) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)
(node:20122) [DEP0180] DeprecationWarning: fs.Stats constructor is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)

 Exception during run: file:///usr/src/Node.JSRestAPI/src/webapi/routes/api.ts:13
import { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } from "infrastructure";
                                                                          ^^^^^^^^
SyntaxError: Named export 'Database' not found. The requested module 'infrastructure' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'infrastructure';
const { StudentRepository, TeacherRepository, LoggerImpl, DatabaseTypes, Database } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async formattedImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run-helpers.js:158:3)
    at async Object.exports.handler (/usr/src/Node.JSRestAPI/node_modules/mocha/lib/cli/run.js:370:5)

@khteh
Copy link
Author

khteh commented Jul 8, 2024

W.r.t https://github.com/mochajs/mocha-examples/tree/main/packages/typescript I tried env TS_NODE_COMPILER_OPTIONS='{\"module\": \"commonjs\" }' and now the error is the following:

$ n run ts_test

> [email protected] ts_test
> export NODE_ENV=development && env TS_NODE_COMPILER_OPTIONS='{"module": "commonjs" }' ./node_modules/.bin/nyc --timeout 10000 --recursive --check-leaks --reporter=lcov --reporter=text --reporter=text-summary --reporter=html ./node_modules/.bin/mocha -r ts-node/register tests/**/*.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_reports/mocha/test-results.xml --exit

(node:150779) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)
(node:150779) [DEP0180] DeprecationWarning: fs.Stats constructor is deprecated.
(Use `node --trace-deprecation ...` to show where the warning was created)

 Exception during run: error TS5110: Option 'module' must be set to 'NodeNext' when option 'moduleResolution' is set to 'NodeNext'.

No previous error but no test is run at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for author waiting on response from OP - more information needed type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

2 participants