Please add a new "Convert to bound imports" TS refactoring

[mceachen]

Suggestion

🔍 Search Terms

• unbound named imports
• TypeError: Cannot read properties of undefined
• convert destructured imports

✅ Viability Checklist

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

It'd be wonderful to support a new refactor action that converts destructed imports (that newly fail under TypeScript 4.4) to an import that avoids unbound function calls:

before

import { addListener, pid } from "process";

addListener("SIGINT", (evt) => {
  console.log(pid + " caught SIGINT", evt);
});

after

import process from "process";

process.addListener("SIGINT", (evt) => {
  console.log(process.pid + " caught SIGINT", evt);
});

📃 Motivating Example

This would greatly reduce the impact from this TypeScript 4.4 change.

💻 Use Cases

See #46027

[andrewbranch]

I think the biggest issue here is that we don't understand that

import * as process from "process";

process.addListener

is undefined no matter what module target you’re using, because addListener is non-enumerable. But using the default import instead of the namespace import when you have esModuleInterop enabled and the target module is written with as a CJS export = is probably the right thing to do in that refactor regardless... (Of course, the export = here belies what is actually going on under the hood in Node itself... 😐)

[DanielRosenwasser]

This was a refactoring added in TypeScript 3.0, right?

https://devblogs.microsoft.com/typescript/announcing-typescript-3-0/#named-import-refactoring

[mceachen]

@andrewbranch @DanielRosenwasser thanks for replying!

I think we're talking about different things.

I'm trying to deal with the (new) error caused by unbound function calls that I'm seeing with javascript produced by TypeScript 4.4. @andrewbranch and I discussed it here: #46027

What I'd like is to either

1. have a tsconfig option to disable the new TS4.4's generated (0, function)(...args) method invocations, or

2. add a refactoring action to make converting to bound method import references easier. That's what I'm asking for here.

In the interests of Chesterton's Fence, I'd like to properly understand why y'all made this change, though: Andrew pointed me at meeting notes, but the cure seemed worse than the disease...

[andrewbranch]

@mceachen the error you get with import * as process from "process" is not the same error you get with import { addListener } from "process" and does not share the same root cause. The import * version has nothing to do with the this binding changes in 4.4. Our emit for the import * version is actually correct, because this code does not work in native node ESM:

[alt text]

❯ node --input-type=module -e 'import * as process from "process"; process.addListener("SIGINT", () => {});'
file:///Users/anbranc/Developer/microsoft/eg/[eval1]:1
import * as process from "process"; process.addListener("SIGINT", () => {});
                                            ^

TypeError: process.addListener is not a function
    at file:///Users/anbranc/Developer/microsoft/eg/[eval1]:1:45
    at ModuleJob.run (internal/modules/esm/module_job.js:170:25)
    at async Loader.eval (internal/modules/esm/loader.js:170:24)
    at async internal/process/execution.js:49:24
    at async loadESM (internal/process/esm_loader.js:68:5)

It has always been invalid to write this code in Node, and our CJS emit for this code has exactly the same behavior as Node's ESM behavior—the issue is that we do not prevent you from writing it in any mode. So you’ve turned up two completely separate issues just trying to write this one little piece of code 😄

[andrewbranch]

Syntactically speaking, the refactor we offer you is exactly right, and you end up transforming from one invalid form to another invalid form, neither of which are we capable of noticing is invalid, incredibly. But given that the export we see from process is a CJS export= and you have --esModuleInterop enabled, my hypothesis is that converting to a default import instead of a namespace import is more likely to actually work—we can perhaps have a good chance of taking you from an invalid form to a valid form.

[mceachen]

Thanks for the explanation @andrewbranch!

I just found these discussions as well:

evanw/esbuild#1163
#35420