GeezerVison

TypeScript Bot (@typescript-bot) pack this

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

Hey Jake Bailey (@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]".;

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

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

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.

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

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

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

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 *?

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...

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

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

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.

With a hack in place to drop this helper:

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.

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...

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

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.

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

I can't read. It's fine.

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