Fix __importDefault when used on typescript libraries

[jakebailey]

Fixes #51473

[jakebailey]

@typescript-bot pack this

[typescript-bot]

Heya @jakebailey, I've started to run the tarball bundle task on this PR at d25da73. You can monitor the build here.

[typescript-bot]

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/138295/artifacts?artifactName=tgz&fileId=10618ECD016CC886706061B6603554907DAB9B79BFC5CC3AF2DCD78B6A8BBAEA02&fileName=/typescript-5.0.0-insiders.20221110.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

[jakebailey]

This does fix the issue, but, I'm not 100% sure this is good or not. default and __esModule really are a mess.

[AlCalzone]

Not sure if this is any help, but esbuild mention this in their docs
https://esbuild.github.io/content-types/#default-interop

[jakebailey]

Yeah, I've seen that, and https://github.com/evanw/bundler-esm-cjs-tests. Unfortunately it seems like in addition to this other stuff, tslib / TS's helpers are another aspect that need to be considered, so I don't know if I can extract a decision out of that. I'll just have to test a bunch of cases, I guess.

[jakebailey]

A table of behaviors, where ts is require("typescript"):

test	4.8.4	main	this PR
ts.version	✅	✅	✅
ts.default.version	❌	❌	✅
__importDefault(ts).version	❌	✅	✅
__importDefault(ts).default.version	✅	❌	✅
__importStar(ts).version	✅	✅	✅
__importStar(ts).default.version	✅	❌	✅

Without hacking things to drop __esModule, I don't know if I can do better than this.

[jakebailey]

Maybe @weswigham or @rbuckton have some insight here? (See also the issue in question, #51473.)

[weswigham]

I'm not a huge fan - we tell people not to do this all the time. Since we only want this for compat, I imagine manipulating the marker may be better. Which, on that note, can't we make the marker not be emitted by using export = ts instead of export *?

[jakebailey]

I haven't found a way yet. esbuild's toCommonJS helper always adds it and I haven't been able to get to do what I want.

Also, export = ts is a d.ts feature only, so that wouldn't be the solution. I have tried module.exports = ts (the equivalent outside of declarations) and that doesn't work because the right side comes from toCommonJS.

I guess I can just hack it...

[jakebailey]

Wait, I'm wrong, module.exports = ts does appear to work. I'll do that instead.

[jakebailey]

Hm, but that only helps the bundler. It breaks everything else :(

[weswigham]

Also, export = ts is a d.ts feature only, so that wouldn't be the solution

Wot? It's definitely a runtime feature we transpile to module.exports = ts, we just only allow it for cjs module targets.

[jakebailey]

With a hack in place to drop this helper:

test	4.8.4	main	this PR
ts.version	✅	✅	✅
ts.default.version	❌	❌	❌
__importDefault(ts).version	❌	✅	❌
__importDefault(ts).default.version	✅	❌	✅
__importStar(ts).version	✅	✅	✅
__importStar(ts).default.version	✅	❌	✅

So, that restores the status quo.

See also evanw/esbuild#1971 but this does seem to be exactly the thing the esbuild docs refer to. I was unable to get the suggestions there to work; neither mts nor "type": "module" changed this behavior. So I think we're stuck with this hack.

[jakebailey]

Wot? It's definitely a runtime feature we transpile to module.exports = ts, we just only allow it for cjs module targets.

Huh, you're right. I must have tested and gotten an error for this a while ago and then assumed it was because export = was d.ts only. I could have sworn I read that...

[jakebailey]

Okay, changed to export =. This leaves us with this state:

test	4.8.4	main	this PR	main --no-bundle	this PR --no-bundle
ts.version	✅	✅	✅	✅	✅
ts.default.version	❌	❌	❌	❌	❌
__importDefault(ts).version	❌	✅	❌	✅	✅
__importDefault(ts).default.version	✅	❌	✅	❌	❌
__importStar(ts).version	✅	✅	✅	✅	✅
__importStar(ts).default.version	✅	❌	✅	❌	❌

So, this makes us match old TS when bundled, which was the goal. --no-bundle doesn't work quite the same, but this PR doesn't seem to change that behavior, so that's fine.

[jakebailey]

Wait, what, it's broken again. Ugh. Bear with me :(

I can't read. It's fine.

[jakebailey]

This should be final now, if you wanted to take another look. I plan to add smoke tests for this to #51464.