Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug when transpiling Promise class/constructor patch #15294

Closed
ORESoftware opened this issue Apr 20, 2017 · 2 comments
Closed

Bug when transpiling Promise class/constructor patch #15294

ORESoftware opened this issue Apr 20, 2017 · 2 comments
Labels
Duplicate An existing issue was already created

Comments

@ORESoftware
Copy link

ORESoftware commented Apr 20, 2017

This is a bug report. On tsc version 2.2.2

I have this TS code:

const PConstructor  = global.Promise;

const Promise = global.Promise = class PatchedPromise extends PConstructor {
  constructor(executor: Function) {
    if (process.domain) {
      executor = executor && process.domain.bind(executor);
    }
    super(executor); // call native Promise constructor
  }
};

const then = Promise.prototype.then;
Promise.prototype.then = function (fn1: Function, fn2: Function) {

  if (process.domain) {
    fn1 = fn1 && process.domain.bind(fn1);
    fn2 = fn2 && process.domain.bind(fn2);
  }

  return then.call(this, fn1, fn2);
};

const katch = Promise.prototype.catch;
Promise.prototype.catch = function (fn1: Function) {
  if (process.domain) {
    fn1 = fn1 && process.domain.bind(fn1);
  }
  return katch.call(this, fn1);
}; 

This transpiles to:

'use strict';
var __extends = (this && this.__extends) || (function () {
    var extendStatics = Object.setPrototypeOf ||
        ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
        function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
    return function (d, b) {
        extendStatics(d, b);
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
})();

var PConstructor = global.Promise;
var Promise = global.Promise = (function (_super) {
    __extends(PatchedPromise, _super);
    function PatchedPromise(executor) {
        var _this = this;
        if (process.domain) {
            executor = executor && process.domain.bind(executor);
        }
        _this = _super.call(this, executor) || this;
        return _this;
    }
    return PatchedPromise;
}(PConstructor));
var then = Promise.prototype.then;
Promise.prototype.then = function (fn1, fn2) {
    if (process.domain) {
        fn1 = fn1 && process.domain.bind(fn1);
        fn2 = fn2 && process.domain.bind(fn2);
    }
    return then.call(this, fn1, fn2);
};
var katch = Promise.prototype.catch;
Promise.prototype.catch = function (fn1) {
    if (process.domain) {
        fn1 = fn1 && process.domain.bind(fn1);
    }
    return katch.call(this, fn1);
};

however the transpiled code does not work, while the following vanilla JS / ES6 code works:

const PConstructor = global.Promise;

const Promise = global.Promise = class PatchedPromise extends PConstructor {
  constructor(executor) {
    if (process.domain) {
      executor = executor && process.domain.bind(executor);
    }
    super(executor); // call native Promise constructor
  }
};

const then = Promise.prototype.then;
Promise.prototype.then = function (fn1, fn2) {

  if (process.domain) {
    fn1 = fn1 && process.domain.bind(fn1);
    fn2 = fn2 && process.domain.bind(fn2);
  }

  return then.call(this, fn1, fn2);
};

const katch = Promise.prototype.catch;
Promise.prototype.catch = function (fn1) {
  if (process.domain) {
    fn1 = fn1 && process.domain.bind(fn1);
  }
  return katch.call(this, fn1);
};

Here is the test:

const patch = require('../../../lib/patches/all');   // load the patch

process.on('uncaughtException', function(err){
  console.log(' This is uncaught => ', err);
});

const domain = require('domain');
const d = domain.create();

d.on('error', function(err){
  console.error(' => Domain caught => ',err);
});

d.run(function(){
      Promise.resolve().then(function(){
        process.nextTick(function(){
            throw new Error('rah'); // <<<<<<<< the domain will catch this
        });
     });
});

if we load the plain vanilla JS patch, this test script works as expected.

However, if we load the patch generated by TypeScript, we get this error:

TypeError: #<PatchedPromise> is not a promise

Pretty intense, by I am very certain this is some sort of bug.

@RyanCavanaugh
Copy link
Member

I believe this is a manifestation of https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work

@mhegazy
Copy link
Contributor

mhegazy commented Apr 26, 2017

This is not the same as other ES5-builtins. this is a duplicate of #15202.

The main issue here is that ES6 classes (e.g. Promise) are not callable, the way ES5 built in classes (e.g. Array). The emitted code that the compiler produces uses super.call which throws in ES6 engine.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Apr 26, 2017
@mhegazy mhegazy closed this as completed Apr 26, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants