TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Typescript [4.8.2] is adding invalid javascript for *.cjs files

Open dzearing opened this issue 2 years ago • 6 comments

Bug Report

My project is set to emit ESNext modules. I have a variety of TS files, but I also have a single .cjs file, which needs to remain CommonJS to try and require legacy packages.

My input looks like this:

module.exports = {
  tryRequire(modulePath) {
    return require(modulePath);
  },
};

I expect the output I get from TypeScript 4.7.4:

module.exports = {
  tryRequire(modulePath) {
    return require(modulePath);
  },
};

When I upgrade to TypeScript 4.8.2, it now emits an extra export {} at the end of the .cjs file:

module.exports = {
  tryRequire(modulePath) {
    return require(modulePath);
  },
};
export {}

This causes Node to fail parsing the .cjs file because export does not exist in the .cjs context.

🕗 Version & Regression Information

Worked in 4.7.4, broken in 4.8.2, also broken in current nightly (4.9.0-dev.20220905).

Repro here

https://github.com/dzearing/ts-repro-cjs

dzearing avatar Sep 06 '22 00:09 dzearing

--module esnext combined with --moduleResolution nodenext is not super supported... at the very least, it’s playing the game on hard mode. I’m not quite sure why it “worked” in 4.7 or what we should actually be doing in this case, but I’m assuming it works with --module nodenext? That’s the only mode where we actually support emitting some files as ESM and some as CJS.

andrewbranch avatar Sep 06 '22 17:09 andrewbranch

Yes! That fixed it. I changed to module: "nodenext" and added type: "module" to the package.json to achieve the expected output (ESM by default, CJS for cjs files.)

dzearing avatar Sep 06 '22 18:09 dzearing

I am unblocked here. I do still think it's not ideal to be emitting invalid cjs but at least I have a workaround. :)

dzearing avatar Sep 06 '22 18:09 dzearing

Agreed. Also related to https://github.com/microsoft/TypeScript/issues/48854. @weswigham thoughts?

andrewbranch avatar Sep 06 '22 18:09 andrewbranch

I ran into this issue with exactly the same scenario - ESM module with a single .cjs file. But in my case, my tsconfig didn't have any value set for "module" at all, and TS 4.7.4 was totally happy with whatever it was using by default. So it was really unexpected to see the invalid javascript emitted by TS 4.8.2

https://github.com/igordanchenko/sandbox/tree/typescript-50647/main

igordanchenko avatar Sep 13 '22 20:09 igordanchenko

Well, I spoke too soon. Setting "module" to "nodenext" did not resolve my issue. It did fix the .cjs file, but now all .ts files are emitted as CommonJS (event though the package.json "type" is set to "module"). I tried setting "moduleResolution" to "nodenext", but tsc compilation completely fails in this case.

index.ts:3:43 - error TS2349: This expression is not callable.
  Type 'typeof import(".../node_modules/clsx/clsx")' has no call signatures.

Any advise will be appreciated.

Here is a minimal repro - https://github.com/igordanchenko/sandbox/tree/typescript-50647/nodenext

igordanchenko avatar Sep 13 '22 21:09 igordanchenko

Have the same issue. Worked on 4.7.4, doesn't work now on 4.8.4 / 4.9.x-rc. Combination of module: "nodenext" and type: "module" doesn't work - compiler starts to complain regarding to import.meta usages in ESM files.

In my use case I need to have a index.cjs (in commonjs) and everything else in ESM (due to commonjs entrypoint requirements in Electron). But now, when index.cjs adds export {} it doesn't work.

pleerock avatar Nov 10 '22 12:11 pleerock

But now, when index.cjs adds export {} it doesn't work.

Got the same problem, anyone knows how to avoid TypeScript appending the empty export to .csj files? Does it still on version 4.9.3. It's just the last little issue allowing me to easily mix ESM and a single CommonJS file (auto-generated) in same project. I'll fix it with a manual copy or replace, but would be nice to avoid that.

sondreb avatar Nov 24 '22 01:11 sondreb

@sondreb I'm investigating issues with this now in version 4.9.3. Bumping from 4.7.x to 4.9.x indeed broke the compiled output by injecting export {}; into a .cjs file (generated GRPC output).

reyawn avatar Nov 25 '22 10:11 reyawn

@pleerock

In my use case I need to have a index.cjs (in commonjs) and everything else in ESM (due to commonjs entrypoint requirements in Electron). But now, when index.cjs adds export {} it doesn't work.

If it expects cjs, you can't reference ESM code under it. CJS can only import ESM using dynamic import.

You have to compile to CJS. You can't use type: module.

unional avatar Jan 03 '23 20:01 unional

Any updates on this? Experiencing the same issue where an empty export {} gets appended to the outputted js when compile a .cts file. My package.json has type: module set, and my tsconfig.json has module: esnext and moduleResolution: node. Like @igordanchenko, my tsc fails to compile with the exact same error when switching to module: nodenext.

slimshreydy avatar Jan 11 '23 18:01 slimshreydy

.cts file extensions in module: esnext is a contradiction, and there are no plans to make it meaningful at the moment—if anything, it should probably be a program-level error. If you’re running in Node, you need to use --module nodenext and fix the errors. The errors aren’t bugs, they’re real issues that need to be addressed if you expect your code to run safely in Node. For example, this:

index.ts:3:43 - error TS2349: This expression is not callable.
  Type 'typeof import(".../node_modules/clsx/clsx")' has no call signatures.

probably means that you need to add a .default on the end of your dynamic import of a CJS module, because that’s what’s actually required in Node. If you run the JS files produced by tsc in Node, you must use node16 or nodenext for module and moduleResolution. Anything else is 100% incorrect and you will experience problems.

andrewbranch avatar Jan 11 '23 19:01 andrewbranch

Ah thank you! You're right; turns out I needed to import clsx as:

import { clsx } from "clsx";

My ts build now works with module: nodenext, and the empty export is gone.

slimshreydy avatar Jan 11 '23 21:01 slimshreydy

@slimshreydy, nice catch! Importing the named export indeed solves clsx issue.

@andrewbranch, do you have any thoughts on why importing the default clsx export isn't working under "moduleResolution": "NodeNext"?

import clsx from "clsx";
src/index.tsx:5:21 - error TS2349: This expression is not callable.
  Type 'typeof import("[REDACTED]/node_modules/clsx/clsx")' has no call signatures.

5     <div className={clsx("fancy", "div")}>{children}</div>;
                      ~~~~


Found 1 error in src/index.tsx:5

Here is a minimal repro - https://github.com/igordanchenko/sandbox/tree/typescript-50647/nodenext-1

igordanchenko avatar Jan 11 '23 21:01 igordanchenko

Because its typings are wrong. clsx is a CJS library, but its typings say export default. https://unpkg.com/browse/[email protected]/clsx.d.ts

andrewbranch avatar Jan 11 '23 21:01 andrewbranch

@weswigham honestly, I think we should consider making export default in CJS-scoped declaration files an error under node16/nodenext, with a message that says the library is probably wrong, similar to #52173. That’s a super common mistake that can often be overlooked in --moduleResolution node.

andrewbranch avatar Jan 11 '23 21:01 andrewbranch

FYI, the "module": "nodenext" solution does not work if you also need "moduleResolution": "bundler". In my case I'm using bundler because it matches the behavior of my node loader (extensionless and subpath exports).

Option 'bundler' can only be used when 'module' is set to 'es2015' or later.

joshjg avatar Jul 18 '23 21:07 joshjg

The compiler is busted in its handling of the new file extension .mts in combination with --module commonjs. You can see my extended rant in the duplicate issue #51990.

knightedcodemonkey avatar Jul 29 '23 21:07 knightedcodemonkey

This should have been closed by #54567

andrewbranch avatar Nov 20 '23 19:11 andrewbranch

Well, maybe not until --module commonjs and --module esnext report an error on a file in a conflicting format.

andrewbranch avatar Nov 20 '23 19:11 andrewbranch

I just ran into this problem with TS 5.3:

Input:

// eslint-disable-next-line import/no-extraneous-dependencies -- This isn't included in the bundle
import { Configuration } from "webpack";

const { resolve } = require("path");

const config: Configuration = {
  entry: "./src/index.ts",
  module: {
    rules: [
      {
        test: /\.tsx?$/,
        use: "ts-loader",
        exclude: /node_modules|\.d\.ts$/,
      },
    ],
  },
  resolve: {
    extensions: [".tsx", ".ts", ".js"],
  },
  output: {
    filename: "bundle.js",
    path: resolve(__dirname, "dist"),
    library: "bbcode_ast",
  },
  devtool: "source-map",
};

module.exports = config;

Output:

const { resolve } = require("path");
const config = {
    entry: "./src/index.ts",
    module: {
        rules: [
            {
                test: /\.tsx?$/,
                use: "ts-loader",
                exclude: /node_modules|\.d\.ts$/,
            },
        ],
    },
    resolve: {
        extensions: [".tsx", ".ts", ".js"],
    },
    output: {
        filename: "bundle.js",
        path: resolve(__dirname, "dist"),
        library: "bbcode_ast",
    },
    devtool: "source-map",
};
module.exports = config;
export {};
//# sourceMappingURL=webpack.config.cjs.map

PythonCoderAS avatar Jan 09 '24 20:01 PythonCoderAS

This was backed out of 5.5—hoping to reintroduce a fix in 5.6 via #58825.

andrewbranch avatar Jun 17 '24 22:06 andrewbranch