baconjs / bacon.js

Functional reactive programming library for TypeScript and JavaScript
https://baconjs.github.io
MIT License
6.47k stars 330 forks source link

Fix TS-generated invalid ES6 constructor call in firstToPromise() #773

Closed semmel closed 3 years ago

semmel commented 3 years ago

This fixes the ES6 implementation of .firstToPromise() in dist/Bacon.mjs. E.g. in Node.js 12 with the latest (v3.0.15) version of Bacon:

import('./node_modules/baconjs/dist/Bacon.mjs').then(m => {globalThis.B = m;});
B.once(8).firstToPromise().then(console.log, console.warn);

// -> Uncaught TypeError: PromiseCtr is not a constructor
// at firstToPromise (file:///.../baconjs/dist/Bacon.mjs:2989:12)
raimohanska commented 3 years ago

Do you have an idea why this resulted to invalid code?

semmel commented 3 years ago

Do you have an idea why this resulted to invalid code?

Targeting ES5 TS generates for the PromiseCtr a function expression, while targeting ES6 an arrow function expression. Apparently these differ in how the can employed as class constructors using new.

var sCons = function(x) { return new String(x);};
new sCons("d"); // OK
var sConsF = x => new String(x);
new sConsF("d") // Uncaught TypeError: sConsF is not a constructor

Maybe in TS new sConsF() is fine but in ES6 it is not. My take from this is that the TS compiler does a great job compiling to ES5, but with ES6 and later I am skeptical.

raimohanska commented 3 years ago

I see, thanks!

raimohanska commented 3 years ago

I wish we have some automatic test to catch this kinda errors.

raimohanska commented 3 years ago

Oh, now we have!

raimohanska commented 3 years ago

Fix included in 3.0.16. Thanks!

raimohanska commented 3 years ago

Btw the test here allows us to easily add more smoke tests for the ES6 module.

semmel commented 3 years ago

Thanks! Works great. Awesome update speed 🚤

Btw the test here allows us to easily add more smoke tests for the ES6 module.

Yes, should've used that. Next time 🤞🏻