Tsdx: breaks are transpiled to return inside then

[CyriacBr]

Current Behavior

While working on my code I noticed a bug that led to my transpiled files and I saw that tsdx was incorrectly transforming my break inside switch cases to return.
I thoughts that was rather odd so I created a sample project with the following code to pinpoint the issue:

function wait() {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      resolve();
    }, 1000);
  });
}

export const test = async () => {
  await wait();
  switch ('' as any) {
    case 'a':
      break;
    case 'b':
      break;
    default:
      break;
  }
  const foo = { bar: true };
  console.log('foo :', foo);
};

When running tsdx build, this is what I get:

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

function wait() {
  return new Promise(function (resolve, reject) {
    setTimeout(function () {
      resolve();
    }, 1000);
  });
}

var test = function test() {
  try {
    return Promise.resolve(wait()).then(function () {
      switch ('') {
        case 'a':
          return;

        case 'b':
          return;

        default:
          return;
      }

      var foo = {
        bar: true
      };
      console.log('foo :', foo);
    });
  } catch (e) {
    return Promise.reject(e);
  }
};

exports.test = test;
//# sourceMappingURL=switch-test.cjs.development.js.map

As you can see, inside test, breaks inside then are transformed to returns, which is NOT correct. The console.log line would never be reached.

Expected behavior

breaks inside then should stay breaks.

Suggested solution(s)

No idea. The problem may come from babel or rollrup or one of their plugins.
Any idea what may cause the problem?

Your environment

| Software | Version(s) |
| ---------------- | ---------- |
| TSDX | 0.12.3
| TypeScript | ^3.7.5
| Browser |
| npm/Yarn | yarn 1.19.1
| Node | 10.16.3
| Operating System | Linux, but same behavior on windows

[CyriacBr]

So, seems like this is due to https://github.com/rpetrich/babel-plugin-transform-async-to-promises/issues/49.

Can tsdx move away from babel-plugin-transform-async-to-promises?

[agilgur5]

Thanks for tracking down the root cause @CyriacBr ! That certainly seems problematic 🙁 As far as I know, TSDX uses it because it is a syntax transform instead of a runtime polyfill, therefore no need to use regenerator-runtime.

There are some alternatives, like fast-async, but that seems less used and less maintained 🙁

I'm not 100% sure if there's a way to disable it with a custom babelrc (it is merged with TSDX's normally, would have to look into that). You might be able to make some small modifications to your code in the meantime to workaround that bug, but idk

[CyriacBr]

For now I'm going to override the rollup config and only use @rollup/typescript for compilation and transpilation.

[agilgur5]

Ok sure, overriding rollup-plugin-typescript2's target is an option as well. TS does use a generator polyfill however -- see https://github.com/microsoft/TypeScript/issues/31621 about transpiling to promises instead.

Can also override rollup-plugin-babel to stay closer to TSDX. Can see the plugins used internally here.

---

I'm looking to add a Babel preset to help make customizing these types of things easier, #383 has some progress on that.
_Removing_ an option is a the difficult part here though; there's this old issue about Babel not allowing you to disable a plugin within a preset (different from how ESLint works), but maybe that's no longer true with overrides. Haven't faced this issue before (first time reading about overrides etc), sorry I can't be more helpful on that front.

[CyriacBr]

Alright. For the time being I ended up forking TSDX and removed anything babel related. If anyone passing by and encountered the same issue, you can thing my fork here, and it's published. I'm fine with TS's runtime polyfill.

TSDX has been my go-to initializer for many months now, thanks a lot for setting up that.

[agilgur5]

Ok, I don't normally recommend forking and the other maintainers recommend using patch-package for internal changes.

I'm also pretty sure this can be worked around with just a tsdx.config.js like I said above (that's what I thought you were going to do too), either overriding rollup-plugin-babel's config (that way you can remove plugins) or overriding rollup-plugin-typescript2's config to change target to something lower.

[CyriacBr]

I'm also pretty sure this can be worked around with just a tsdx.config.js like I said above (that's what I thought you were going to do too)

Yes, that's what I initially wanted to do, but then I realized I'd have to perform the same process for each new project. That'd defeat the process of using a initializer like TSDX. I wanted something that just works out of the box without me copy-pasting stuff from previous projects.
I'll jump back to tsdx once it is fixed and doesn't need any extra step :).

[agilgur5]

Yes, that's what I initially wanted to do, but then I realized I'd have to perform the same process for each new project. That'd defeat the process of using a initializer like TSDX. I wanted something that just works out of the box without me copy-pasting stuff from previous projects.

Ah gotcha, that makes sense. Yea I can see why some workarounds can be annoying for people with multiple packages, though you might not need this everywhere.

The other option is to create a _shareable_ tsdx.config.js, like:

// tsdx.config.js
module.exports = require('@cyriacbr/tsdx-config-no-babel')

Though if something like this is officially adopted (I'm planning on making an RFC soon), it would probably look more like a plugin than a shareable config:

// tsdx.config.js
const noBabelPlugin = require('@cyriacbr/tsdx-plugin-no-babel')

module.exports = {
   rollup (config) {
      noBabelPlugin(config)
      // ...
      return config
   }
}

You can of course do either (or both) _unofficially_ though.
The RFC will be to establish a spec for compatibility and to ideally eventually bring some common use cases in as "core" plugins (esp. since they can be brittle). But we'll see how that goes.

I'll jump back to tsdx once it is fixed and doesn't need any extra step :).

Yea, unfortunately since it's upstream and hasn't been responded to in ~3 months it might take a while :disappointed: . In the meantime, your fork is likely to become out-of-sync, so it becomes a maintenance burden for you to update :confused:

[agilgur5]

Looks like there was another incorrect transpilation brought up from this plugin: #423 / https://github.com/rpetrich/babel-plugin-transform-async-to-promises/issues/53 .

EDIT: Also seems like #190 is related. But that doesn't seem to error if it's used standalone

Would probably be good to consider replacing this, especially since it hasn't been maintained the past few months (although regenerator maybe the only good alternative) 😕

[agilgur5]

Another comment on that issue revealing more broken code: https://github.com/rpetrich/babel-plugin-transform-async-to-promises/issues/49#issuecomment-656050437. And still no response / maintenance.

At this point, I think it would be optimal to switch to regenerator because at least the code it produces is actually _correct_ and not broken in subtle, hard-to-detect ways. I'm also not sure really how much difference in weight it adds as a polyfill because babel-plugin-transform-async-to-promises also adds helpers, it's not just a polyfill vs. no polyfill comparison.

[schickling]

I've also ran into related gnarly logic bugs stemming from the linked issue above. @agilgur5 @jaredpalmer are there any plans to switch out babel-plugin-transform-async-to-promise in favor of something else?

[agilgur5]

@schickling I did say in the above comment I was looking to switch to regenerator. That's the only other option as far as I know. TSDX also already bundles in regenerator if one uses generators (the async/await part is turned off)

[rivertam]

Not to just add unnecessary noise, but this drove me crazy! I discovered even more broken examples where the emitted code would literally use things like _break4 or _break2 in a different scope than they were declared, code that was not just incorrect but could be statically determined to be incorrect! Compiling to non-regenerator-runtime code is a noble goal, but ultimately I can't justify using tsdx when it's in this state. It does bother me to have to essentially "eject" because of such a glaring issue when otherwise the project is so useful.

One of the main reasons I'm writing this comment is so people who search something like _break4 or _break2 can find this and see the problem!

Thank you @agilgur5 and @hb-seb for trying to resolve this! I hope it can be resolved soon.

[agilgur5]

@allcontributors please add @CyriacBr for bug

[allcontributors[bot]]

@agilgur5

I've put up a pull request to add @CyriacBr! :tada:

[agilgur5]

async-to-promises has been replaced with babel-plugin-polyfill-regenerator in #795 and will be released in v0.14.0 soon. It will only pure polyfill generators for targets that need a polyfill according to your browserslistrc or preset-env targets