Function call changes in TypeScript 4.4 seem to be breaking detection of named exports

[BenSjoberg]

Hi, I'm hoping this is the right place to post this! I just learned about this project when investigating an issue with named imports in NestJS.

TypeScript made a change to how certain function calls are compiled to CommonJS in version 4.4. Long story short, when you write export * from "./foo"; in TypeScript, in 4.3 and earlier it would write something like:

tslib_1.__exportStar(require("./foo"), exports);

But in 4.4 it's writing:

(0, tslib_1.__exportStar)(require("./foo"), exports);

I ran into this while investigating an issue I ran into when upgrading NestJS, and it seems others are experiencing the same problems with other packages.

Like I said, I'm not very familiar with this project, but is it possible that the parsing code could be updated to handle this new syntax? I expect this is going to affect more and more CommonJS projects written in TypeScript as they release new versions compiled with 4.4 or later.

[guybedford]

The problem is if this is fixed, as a user you will then be surprised when your package doesn't work on Node.js 14 / Node.js 12. Everything is fine then suddenly you get an issue from a user running your library in a Lambda.

For this reason we are weary to post feature changes to this package.

Note you can just do

import x from 'nestjs';
const { Export } = x;

And that will always work.

[BenSjoberg]

The problem is if this is fixed, as a user you will then be surprised when your package doesn't work on Node.js 14 / Node.js 12. Everything is fine then suddenly you get an issue from a user running your library in a Lambda.

Since those versions are still under LTS, would it be possible to backport the changes in this library to those versions of Node? (I guess that's a question for the Node.js team.)

I get that it might take some time for everyone to update, but it feels like the lesser of two evils to me. Due to the TypeScript change (assuming they don't implement a fix), every TypeScript-compiled CommonJS library is going to have its named exports done via export * from 'foo' stop working from ESM over time, as maintainers upgrade to TS 4.4+. This is especially frustrating because I assume most package maintainers won't consider upgrading from TS 4.3 to 4.4 to be semver-major change for their package. (That's what brought my attention to this issue in the first place.) This could lead to scenarios where an ESM library stops working when an in-range update to one its CJS dependencies releases a new version compiled with TS 4.4+.

I completely understand your hesitancy though. Especially because there's no guarantee a future version of TS (or some other tool) won't change things again.

Note you can just do [...] And that will always work.

I know. I just wish that auto-imports would work that way when working with CJS modules in ESM code. It's frustrating to write imports that your editor (& the TypeScript compiler) thinks will work, that fail at runtime. Probably easier said than done though! (And obviously out-of-scope for this repo. 🙂)

[guybedford]

It was always clear this would happen to this package, and the policy was to treat this package as frozen because it is better to have things not be detected than be inconsistently detected. This is not a live package that will update to every transpiler change. The line has to be drawn and I know that's hard!

[BenSjoberg]

The line has to be drawn and I know that's hard!

Fair enough. Thanks for your time anyway!

[dnalborczyk]

@guybedford I always thought this module would detect cjs exports at runtime.

what @BenSjoberg is saying is that anyone using star exports and updating to Typescript v4.4 will essentially have their module broken when running in node.js.

if typescript, babel, rollup, webpack, or any other compiler/bundler decide to change the syntax output just so slightly in the future without any runtime changes it'll break a bunch of users.

It was always clear this would happen to this package, and the policy was to treat this package as frozen because it is better to have things not be detected than be inconsistently detected. This is not a live package that will update to every transpiler change. The line has to be drawn and I know that's hard!

with that notion I think this module should have never been incorporated into node.js and the cjs import should have just been left alone with requiring a default import.

[nicolo-ribaudo]

with that notion I think this module should have never been incorporated into node.js and the cjs import should have just been left alone with requiring a default import.

From my perspective, this module is only needed for old dependencies.
Maintained dependency (the ones that can update their typescript version, for example), can easily add an ESM entry point with the correct interface: by doing so they make sure that it can be imported regardless of the compiler they are using.

[guybedford]

This has been resolved thanks to microsoft/TypeScript#47070.