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

TypeScript + CommonJS support #3

Closed
2 of 3 tasks
Jackman3005 opened this issue Dec 22, 2022 · 6 comments
Closed
2 of 3 tasks

TypeScript + CommonJS support #3

Jackman3005 opened this issue Dec 22, 2022 · 6 comments
Labels
question Further information is requested

Comments

@Jackman3005
Copy link

Jackman3005 commented Dec 22, 2022

Guidelines

  • Please search other issues to make sure this bug has not already been reported.
  • If this is related to a typo or the documentation being unclear, please click on the relevant page's Edit button (pencil icon) and suggest a correction instead.

Describe the bug

I am unable to import this library into my codebase. I think it is because the published version is not compiled to module system I can use (?).

It's quite possible there is something on my side that is causing the issue, or some configuration I could change to get it to work, but we already have > 80 other dependencies in our API that are not having any issues importing. (It's an express API)

Thanks in advance for any help, I'm quite excited to give this library a try!

Steps to reproduce

  1. Import and use the library
import ModernError from "modern-errors";

const BaseError = ModernError.subclass("BaseError");
throw new BaseError("test");
  1. Start app with ts-node-dev
  2. See following stack trace:
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/jack/workspace/my-project/api/node_modules/modern-errors/build/src/main.js from /Users/jack/workspace/my-project/api/src/index.ts not supported.
Instead change the require of main.js in /Users/jack/workspace/my-project/api/src/index.ts to a dynamic import() which is available in all CommonJS modules.
    at require.extensions..jsx.require.extensions..js (/private/var/folders/8n/dqgv9b4j797cqd34zb62ythw0000gn/T/ts-node-dev-hook-0371657800851346.js:114:20)
    at Object.nodeDevHook [as .js] (/Users/jack/workspace/my-project/api/node_modules/ts-node-dev/lib/hook.js:63:13)
    at Object.<anonymous> (/Users/jack/workspace/my-project/api/src/index.ts:68:41)
    at Module._compile (/Users/jack/workspace/my-project/api/node_modules/source-map-support/source-map-support.js:547:25)
    at Module.m._compile (/private/var/folders/8n/dqgv9b4j797cqd34zb62ythw0000gn/T/ts-node-dev-hook-0371657800851346.js:69:33)
    at require.extensions..jsx.require.extensions..js (/private/var/folders/8n/dqgv9b4j797cqd34zb62ythw0000gn/T/ts-node-dev-hook-0371657800851346.js:114:20)
    at require.extensions.<computed> (/private/var/folders/8n/dqgv9b4j797cqd34zb62ythw0000gn/T/ts-node-dev-hook-0371657800851346.js:71:20)
    at Object.nodeDevHook [as .ts] (/Users/jack/workspace/my-project/api/node_modules/ts-node-dev/lib/hook.js:63:13)
    at Object.<anonymous> (/Users/jack/workspace/my-project/api/node_modules/ts-node-dev/lib/wrap.js:104:1)
    at Module._compile (/Users/jack/workspace/my-project/api/node_modules/source-map-support/source-map-support.js:547:25)
    at Object.require.extensions..jsx.require.extensions..js (/private/var/folders/8n/dqgv9b4j797cqd34zb62ythw0000gn/T/ts-node-dev-hook-0371657800851346.js:95:24)

Configuration

tsconfig.json

{
  "compilerOptions": {
    "lib": ["es2021"],
    "target": "es2019",
    "module": "commonjs",
    "moduleResolution": "node",
    "esModuleInterop": true,
    "strict": true,
    "rootDir": "./",
    "outDir": ".build",
    "allowJs": true,
    "sourceMap": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "typeRoots": ["node_modules/@types", "generated"],
  },
  "exclude": ["node_modules"]
}

Environment

 System:
   OS: macOS 13.0.1
   CPU: (10) arm64 Apple M1 Pro
   Memory: 158.23 MB / 16.00 GB
   Shell: 5.8.1 - /bin/zsh
 Binaries:
   Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
   Yarn: 1.22.17 - /opt/homebrew/bin/yarn
   npm: 8.18.0 - ~/.nvm/versions/node/v16.14.0/bin/npm
 Browsers:
   Brave Browser: 100.1.37.113
   Chrome: 108.0.5359.124
   Firefox: 107.0.1
   Safari: 16.1

Pull request (optional)

  • I can submit a pull request.
@Jackman3005 Jackman3005 added the bug Something isn't working label Dec 22, 2022
@Jackman3005 Jackman3005 changed the title Runtime Error in Typescript Node environment Import causes runtime error in Typescript Node environment Dec 22, 2022
@ehmicky
Copy link
Owner

ehmicky commented Dec 22, 2022

Hi @Jackman3005,

Thanks a lot for reaching out!

Problem

The core problem is that modern-errors is using ES modules while your configuration is using CommonJS. Your current TypeScript configuration is using "module": "commonjs" and"moduleResolution": "node" which transpiles import ModernError from "modern-errors" to const ModernError = require("modern-errors"). However, Node.js does not allow using require() on ES modules.

Solution 1

The best solution to this is to configure TypeScript to support ES modules, i.e. use "module": "nodenext" and "moduleResolution": "nodenext".

I understand this is a big change and might not be worth it for you when trying to add a single library. However, this is a general problem you will encounter not only with modern-errors but with any dependencies using ES modules. The >80 dependencies you are currently importing are not using ES modules, but the number of libraries that have migrated to ES modules is rapidly growing right now.

CommonJS is deprecated by Node.js. Node 14 (the oldest Node.js version still officially supported) works natively with ES modules. For this reason, modern-errors should remain ES modules only.

Solution 2 (details)

The other option is to use a dynamic import(): const ModernError = await import("modern-errors"). Node.js allows dynamic imports of ES modules from CommonJS.

This solution has a problem though: modern-errors is meant to be imported on the top-level scope, because it is much simpler to create error classes in the top-level scope instead of inside a function. However, top-level await is not supported by TypeScript with "module": "commonjs". Instead, "module": "nodenext" should be used. The main difference is: instead of outputting only CommonJS, TypeScript will output either CommonJS or ES modules, depending on which one it detects. In particular, it will output ES modules if your package.json is using "type": "module" or a file has a .mjs file extension. If you're currently using a CommonJS configuration, you are probably not doing either, i.e. I believe this change might work without any issues for you.

The other problem with dynamic imports is that they are missing type information. This can be solved by using the following type assertion: as unknown as typeof import('modern-errors').default.

Solution 2 (summary)

"module": "nodenext" instead of "module": "commonjs" in tsconfig.json then:

const ModernError = (await import('modern-errors')) as unknown as typeof import('modern-errors').default

I have just added some documentation for the above here.

Please let me know what you think.

@ehmicky ehmicky added question Further information is requested and removed bug Something isn't working labels Dec 22, 2022
@ehmicky ehmicky changed the title Import causes runtime error in Typescript Node environment TypeScript + CommonJS support Dec 22, 2022
@Jackman3005
Copy link
Author

Hey @ehmicky thanks for getting back to me so quickly. I'll take a look at the options and see what works for us. Cheers!

@Jackman3005
Copy link
Author

I guess part of the issue is I was not aware that "commonjs" is deprecated in Node 14. I got confused by the following in the Official Typescript Docs for Module field:

Sets the module system for the program. See the Modules reference page for more information. You very likely want "CommonJS" for node projects.

@Jackman3005
Copy link
Author

I notice other libraries I use actually provide both commonjs and esm files. What would the problem be for modern-errors to provide multiple module systems in the NPM package? I imagine I'm not the only one that isn't on ESM yet..

@ehmicky
Copy link
Owner

ehmicky commented Dec 22, 2022

Please note I was referring to Node.js (not TypeScript) deprecating CommonJS. However the term "deprecating" was rather incorrect: Node.js is not deprecating nor planning to ever remove support for CommonJS. A more accurate way to put it would be: they are planning to focus primarily on ES modules for the present and future (see discussion about this).

Double packaging

I have tried double packaging in the past, and it turned out to be quite complicated for bigger projects like modern-errors.

All my repositories use the same build logic. Changes in how one project is built impacts the other ones, so I tend to be cautious about major changes.

I am currently supporting two different build steps: one for my JavaScript repositories (like modern-errors) that uses Babel, and one for my TypeScript repositories that uses tsc. This would requiring configuring four instead.

This would also double the npm package size. While some users might elevate this with tree-shaking, many of them do not have a build setup that supports that.

Beyond the complication of the build logic, supporting both CommonJS and ES modules in a bigger codebase can get quite tricky. The two module systems have subtle differences beyond import/export vs require()/module.exports, such as:

  • CommonJS does not allow top-level await
  • CommonJS require()'s argument is a file path string, not a file: URI string. For example, escaping of non-ASCII works differently.
  • CommonJS uses require.cache for cache busting, while ES modules use query strings
  • CommonJS uses __filename/__dirname instead of import.meta.url
  • CommonJS uses require.resolve() instead of import.meta.resolve()
  • And so on

Some of the above differences have workarounds, but some are harder to fix.

Another quite important issue would be: using dependencies which are pure ES modules, such as all libraries from Sindre Sorhus (which I rely on a lot). To be able to import those while still outputting CommonJS, dynamic import() would be needed instead of static import statements. However, this does not work in a project where the main exports are synchronous (like modern-errors).

CommonJS support

On one hand, I do agree with you. Not everyone has migrated to ES modules yet, and I do want to support those users. It does bother me that some users might not be able to use my libraries due to using CommonJS.

On the other hand, this would be too complicated to support. It also would prevent from using dependencies that are pure ES modules themselves (although some of those are critical to modern-errors). Overall, CommonJS is legacy and its usage will decrease over time.

Updates on the solution

The workarounds I was suggesting above have some problems unfortunately.

First, they require using a top-level await, which is not supported by Node.js when using CommonJS.

Then, they require tsc to not transpile import() to require(). The only way to do this unfortunately is to use "module": "es2020" (or above), or both "module": "node16" (or above) and "moduleResolution": "node16" (or above). However, that configuration will produce ES modules as output, not CommonJS.

Which means, for TypeScript users, it seems like there is no workaround, besides switching to ES modules. I updated the documentation accordingly.

Thanks again for your interest in this project, and I'm sorry I could not find a solution for your setup.

If switching to ES modules is an option for you, please check the following TypeScript guide which is quite thorough. This other guide is also very helpful. Doing that migration would not only allow you to use modern-errors, but also any other Node modules that are pure ES modules. For example, any package from Sindre Sorhus. There are quite many of them now, and that number is expected to keep increasing.

@ehmicky ehmicky closed this as completed Dec 23, 2022
@Jackman3005
Copy link
Author

Jackman3005 commented Jan 2, 2023

Unfortunately it looks like we will not be able to use your error framework for now due to the lack of double packaging/support for the CommonJS module system out of the box. It'd be great if we could more readily support the new module system in our code base or if the hacks to use it while we still are using CommonJS were less hacky.

It looks great in concept and seems like something that would really clean up and improve our error handling system. One day I hope to be able to use it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants