tsx icon indicating copy to clipboard operation
tsx copied to clipboard

Surprising module resolution behavior vs. esbuild and bun

Open andrewbranch opened this issue 1 year ago • 2 comments

Acknowledgements

  • [X] I searched existing issues before opening this one to avoid duplicates
  • [X] I understand this is not a place for seek help, but to report a bug
  • [X] I understand that the bug must be proven first with a minimal reproduction
  • [X] I will be polite, respectful, and considerate of people's time and effort

Minimal reproduction URL

https://github.com/andrewbranch/tsx-module-resolution

Version

v4.6.2

Node.js version

v21.1.0

Package manager

npm

Operating system

macOS

Problem & Expected behavior

I uploaded a repo instead of using StackBlitz because it looks like StackBlitz doesn’t support Bun yet, and I wanted to use it as a point of comparison.

This is probably not a bug per se but I wanted to raise this as a design issue that I think should be considered.

Module resolution of node_modules packages does not work how I expected, and differs from what both esbuild and Bun do. It looks like tsx is choosing whether to transform the input to CommonJS based on the file extension / package.json "type", and then performing module resolution from the transformed syntax. So, when you run npm run tsx-ts in the provided repro, the import declaration gets invisibly transformed to a require, and therefore loads the "require" conditional export of pkg. But on the same code, esbuild and Bun both load the "import" conditional export.

Also unlike esbuild and Bun, the behavior changes based on the file extension or package.json "type" of the importing file. When we try the same code from a .mts file (npm run tsx-mts), the explicit require call fails because it’s not processed by tsx; it’s passed on to Node.js in an ESM file.

Expected Actual
npm run tsx-ts { import: 'import.mjs', require: 'require.js', dynamicImport: 'import.mjs' } { import: 'require.js', require: 'require.js', dynamicImport: 'import.mjs' }
npm run tsx-mts { import: 'import.mjs', require: 'require.js', dynamicImport: 'import.mjs' } Errors

All this behavior just falls out of Node.js’s behavior, but it’s not clear to me why tsx can’t or won’t override Node.js here and hide more of the implementation details. I think Bun implemented the intuitive behavior here, and I’ve heard many people suggest tsx as a way to bring Bun-style interop to Node.js, but it seems like the two are fairly far apart on how interop should work.

Moreover, there is no tsconfig.json that can represent what tsx is doing with module resolution. I had previously thought --module esnext --moduleResolution bundler (and soon, --module preserve) were accurate settings, but this difference in conditional "exports" resolution proved me wrong. I’ll have to update the TypeScript docs but it seems there’s no great suggestion I can make for how to type check code written for tsx—--module nodenext is stricter than necessary, but --moduleResolution bundler is inaccurate.

Contributions

  • [ ] I plan to open a pull request for this issue
  • [ ] I plan to make a financial contribution to this project

andrewbranch avatar Dec 18 '23 17:12 andrewbranch

Thank you for your detailed issue report, @andrewbranch. Your insight and feedback are highly valued.

Regarding tsx's perceived functionality

Originally, tsx was marketed as an esbuild powered runtime. However, I've shifted away from this description to prevent misunderstandings. Many people filed bugs expecting tsx to mimic esbuild's behavior exactly. In reality, tsx only leverages esbuild for transforming TypeScript (and ESM syntax within CommonJS environments).

I haven't made explicit comparisons to bun, but I acknowledge the need to refine our branding & messaging. Clarifying what users can expect from tsx is a priority I hope to address in early 2024.

Regarding the first issue with npm run tsx-ts

This issue aligns with an existing report: https://github.com/privatenumber/tsx/issues/239

It's not a popular issue and thus not top-priority, but I plan on investigating it.

My plan is to swap the require condition for import in the resolution for ESM-in-CJS files, and transforming further ESM syntax to CommonJS. This approach would be straight forward once Node.js lands complete support for loading CommonJS via Module Hooks.

However, I believe you're suggesting to run ESM syntax in a module context. Initially, tsx operated this way, but it led to compatibility issues with (outdated) packages containing mixed ESM and CJS code. Transforming ESM syntax to CJS was a more practical solution than attempting to fully adapt all CJS functionality in ESM, considering the strict and opt-in nature of modules.

RE: The second issue with npm run tsx-mts

I experimented with compiling via tsc, and running the output yielded the same error. This is not a bug as you noted, but setting "module": "NodeNext" seems to automatically inject createRequire. This was an unexpected feature for me, and I'm not sure how to feel about its implications considering the strictness of modules. But since TypeScript supports this functionality, it seems like a reasonable default to adopt in tsx.

I'm curious—do you know why TypeScript chose to support import a = require('a') in Modules, but not const a = require('a')? Is it for historical, backwards compatibility reasons?

privatenumber avatar Dec 19 '23 09:12 privatenumber

Thank for the pointer to #239, I’ll follow that!

However, I believe you're suggesting to run ESM syntax in a module context.

Honestly, I don’t have a strong mental model around what’s really possible with the loader APIs, so I wasn’t implying an implementation suggestion here. Just changing the require condition to import when triggered from import syntax seems good enough to me.

I'm curious—do you know why TypeScript chose to support import a = require('a') in Modules, but not const a = require('a')? Is it for historical, backwards compatibility reasons?

TypeScript files can import and export not just values, but types too. CommonJS constructs use expression-level syntax that wouldn’t have made sense for types:

interface Foo {}
module.exports = Foo; // AssignmentExpression, Foo must be a value
const SomeType = require("a"); // VariableDeclaration, SomeType declares a value

Using alternative syntax made it more clear that (a) these constructs would be transformed before emit, and (b) they can declare meanings besides values. So const a = require('a') never has any special meaning in TypeScript files, even in files that are going to emit CommonJS. Even if we did recognize it, we wouldn’t transform it with the createRequire trick, since we try to transform existing valid JavaScript as little as possible.

andrewbranch avatar Dec 19 '23 16:12 andrewbranch