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

transpileModule does not account for package.json type when ran with node16/nodenext module setting #53022

Closed
romkhub opened this issue Feb 28, 2023 · 4 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@romkhub
Copy link

romkhub commented Feb 28, 2023

Bug Report

🔎 Search Terms

transpileModule node16 nodenext

🕗 Version & Regression Information

Checked with

@4.9.4
@beta - 5.0.0-beta
@next - 5.1.0-dev.20230227

It's only about node16/nodenext and therefore doesn't affect TS versions before 4.7

⏯ Playground Link

https://github.com/romkhub/ts-transpile-node16

The repo illustrates how tsc provides ESM, while transpileModule gives CJS upon the same setup.
After debugging I see it related to this code which, as I understand, essentially hides all the files except the one transpiled.

My real-world scenario is an attempt to use node16 TS setup in a repo built with webpack and ts-loader in transpileOnly mode. With transpileOnly: true I get CJS, transpileOnly: false - ESM

🙁 Actual behavior

transpileModule does not recognize "type": "module" in package.json and emit CJS, while tsc emits ESM

🙂 Expected behavior

JS emitted by transpileModule is the same as of tsc

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 28, 2023
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.1.0 milestone Feb 28, 2023
@mrazauskas
Copy link

mrazauskas commented Apr 1, 2023

Hitting the same problem. Seems like the cause of it is the following:

  • a custom compilerHost is created inside of transpileModule()
  • the fileExists() method of the host is only able to check existence of the input file path:
    fileExists: (fileName): boolean => fileName === inputFileName,
    readFile: () => "",
  • the host is passed to getImpliedNodeFormatForFile() function, which is responsible to find nearest package.json and to look at its the "type" field
  • this job is done by getPackageJsonInfo() function, which calls host.fileExists() on each of potential packageJsonPaths:
    if (directoryExists && host.fileExists(packageJsonPath)) {

Unfortunately the host cannot know about package.json files, because it is only able to check the existence of the input file path.


Might be this is expected behaviour. For instance, if the transpileModule() function is intended to work with virtual files only and it shouldn’t know anything about the real file system.

@mrazauskas
Copy link

Here is a patch of the compilerHost which makes "module": "node16" compiler option work as expected:

- fileExists: (fileName): boolean => fileName === inputFileName,
- readFile: () => "",
+ fileExists: (fileName) => fileName.endsWith("package.json") ? sys.fileExists(fileName) : fileName === inputFileName,
+ readFile: (fileName) => fileName.endsWith("package.json") ? sys.readFile(fileName) : "",

@RyanCavanaugh
Copy link
Member

I'll wait for @rbuckton to make a final call, but my impression is that the entire point of transpileModule is to be a text-only transform process. It shouldn't attempt to hit the disk at all; it especially cannot call sys functions.

@rbuckton
Copy link
Member

rbuckton commented May 3, 2023

I concur with @RyanCavanaugh. transpileModule shouldn't interact with the file system at all. If you want it to treat the input file as ESM, you would need to provide a fileName to the transpileOptions argument with a .mts (or .mtsx) extension. If that isn't sufficient, we could consider adding an option to TranspileOptions to override the implied node format.

@rbuckton rbuckton closed this as completed May 3, 2023
dummdidumm added a commit to sveltejs/kit that referenced this issue Nov 21, 2023
transpileModule treats NodeNext as CommonJS because it doesn't read the package.json. Therefore we need to override it. Also see microsoft/TypeScript#53022 (the filename workaround doesn't work).
Fixes #11086
dummdidumm added a commit to sveltejs/svelte-preprocess that referenced this issue Nov 21, 2023
transpileModule treats NodeNext as CommonJS because it doesn't read the package.json. Also see microsoft/TypeScript#53022 (the filename workaround doesn't work). Related to sveltejs/kit#11086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants