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

"#<Object> is not a constructor" when used with TypeScript's allowSyntheticDefaultImports & esModuleInterop #5

Closed
borekb opened this issue Nov 27, 2019 · 17 comments

Comments

@borekb
Copy link

borekb commented Nov 27, 2019

I'm hitting trouble when trying to use this library in a TypeScript & Babel project. I installed both the lib and the types as usual:

npm i promise.allsettled @types/promise.allsettled

The standard require (as described in README) is not TypeScript-friendly, which is normal:

const allSettled = require('promise.allsettled');

// allSettled is `any` at this point, @types are unused

That's why the type tests use this syntax:

import allSettled = require("promise.allsettled");

However, that's not supported by Babel 7 and transitively by all tools that depend on it. For example, our Jest tests fail with this:

SyntaxError: demo.ts: import = is not supported by @babel/plugin-transform-typescript
Please consider using import <moduleName> from '<moduleName>'; alongside Typescript's --allowSyntheticDefaultImports option.

Again, this is quite normal and the common fix is to create this tsconfig.json:

{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
  }
}

and use this import syntax:

import allSettled from 'promise.allsettled';

We use this for several other modules and even with promise.allsettled, TypeScript is happy at this point and Jest test pass (the promise.allsettled code is executed correctly at runtime!).

However, it leads to this error when running the app through plain node (or via ts-node):

TypeError: #<Object> is not a constructor
    at Object.resolve (<anonymous>)
    at Object.PromiseResolve (/example/node_modules/promise.allsettled/node_modules/es-abstract/es2018.js:160:10)
    at /example/node_modules/promise.allsettled/implementation.js:26:24
    at Function.from (<anonymous>)
    at Object.allSettled (/example/node_modules/promise.allsettled/implementation.js:19:22)
    at Object.allSettled [as default] (/example/node_modules/promise.allsettled/index.js:16:9)
    at myFunction (/example/demo.ts:329:53)
    at process._tickCallback (internal/process/next_tick.js:68:7)

My theory is this: in the --esModuleInterop & allowSyntheticDefaultImports mode, TypeScript transpiles the import to this:

"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
const promise_allsettled_1 = __importDefault(require("promise.allsettled"));

Note how it wrapped the module into default property, so the usages look like this in the transpiled code:

await promise_allsettled_1.default(...)

And probably, promise.allsettled doesn't like this and throws the #<Object> is not a constructor error.

I didn't dive deep into how the various "binds" in this library work but if there was a way to make it more friendly to that specific TypeScript emit scenario, that would be great.

@ljharb
Copy link
Member

ljharb commented Nov 27, 2019

The TypeScript module system is broken unless both of those settings are enabled; they should be enabled in every project, full stop (and tsc init enables them in all new projects too)

If typescript’s downleveling is incorrect, you’ll have to file an issue with them - I’d suggest using babel for all transpilation, and only use tsc as a typechecker.

@ljharb
Copy link
Member

ljharb commented Nov 27, 2019

If there’s something that can change in the types (which i don’t maintain) I’d be happy to review and endorse a PR to DefinitelyTyped.

@borekb
Copy link
Author

borekb commented Nov 28, 2019

I agree with the general comment that Babel should be preferred over tsc for transpiling but we cannot make that change right now.

Back to the original issue, the typings are correct but the runtime behavior isn't. This is plain JS demonstrating the issue:

const allSettled = { default: require("promise.allsettled") };

const resolved = Promise.resolve(42);
const rejected = Promise.reject(-1);

allSettled.default([resolved, rejected]).then(results => {
  console.log(results);
});

Output:

TypeError: [object Object] is not a constructor
    at Object.resolve (<anonymous>)
    at Object.PromiseResolve (/home/runner/node_modules/es-abstract/es2018.js:160:10)
    at /home/runner/node_modules/promise.allsettled/implementation.js:26:24
    at Function.from (<anonymous>)
    at Object.allSettled (/home/runner/node_modules/promise.allsettled/implementation.js:19:22)
    at Object.allSettled [as default] (/home/runner/node_modules/promise.allsettled/index.js:16:9)
    at /home/runner/index.js:10:19    at Script.runInContext (vm.js:133:20)
    at Object.<anonymous> (/run_dir/interp.js:156:20)    at Module._compile (internal/modules/cjs/loader.js:778:30)

Playground.

@borekb
Copy link
Author

borekb commented Nov 28, 2019

For anyone needing a quick workaround, here's a TypeScript function emulating the behavior.

Note: It's probably not as generic & correct as this lib's implementation but works fine for the use case I have.

export function allSettled<T>(iterable: Iterable<Promise<T>>): Promise<PromiseResult<T>[]> {
  const arr = Array.from(iterable, item => {
    return item
      .then(value => ({ status: 'fulfilled', value } as PromiseResolution<T>))
      .catch(reason => ({ status: 'rejected', reason } as PromiseRejection<typeof reason>));
  });
  return Promise.all(arr);
}

// Types are taken from
// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/1d3c224/types/promise.allsettled/types.d.ts

export interface PromiseResolution<T> {
  status: 'fulfilled';
  value: T;
}

export interface PromiseRejection<E> {
  status: 'rejected';
  reason: E;
}

export type PromiseResult<T, E = unknown> = PromiseResolution<T> | PromiseRejection<E>;

export type PromiseTuple<T extends [unknown, ...unknown[]]> = { [P in keyof T]: Promise<T[P]> };
export type PromiseResultTuple<T extends [unknown, ...unknown[]]> = { [P in keyof T]: PromiseResult<T[P]> };

@ljharb
Copy link
Member

ljharb commented Nov 28, 2019

hmm, I think i see what you mean.

This is due to the behavior here: https://github.com/es-shims/Promise.allSettled/blob/master/index.js#L16 combined with the actual spec behavior for Promise.allSettled, which uses the this.

In other words, the downleveling is indeed incorrect, since it should not be calling the default function with a receiver - it should be something like (0, allSettled.default)([resolved, rejected]), which will ensure its receiver is undefined.

There's no way for me to fix things here without violating the spec; this is indeed a bug in TypeScript's downleveling.

@borekb
Copy link
Author

borekb commented Nov 28, 2019

How about exporting the implementation that doesn't re-bind this? I'd do something like this:

import { allSettled } from 'promise.allsettled/implementation';

@ljharb
Copy link
Member

ljharb commented Nov 28, 2019

That already exists under /implementation, but then it will fail because it's not Promise.

In other words, it seems like what you want is an implementation that always ignores the receiver and sets it to Promise. You could do allSettled.call(null, [resolved, rejected]) i suppose, and that should work even with the broken tsc output, while you wait for TS to fix it? (assuming you file an issue for them)

@borekb
Copy link
Author

borekb commented Nov 28, 2019

Thanks, that's a good-enough workaround.

I'm not sure how to formulate that TypeScript issue, if it's an issue at all as they've been generating the current __importDefault code for a long time and quickly searching their issues doesn't seem like there would be any problems with it. But I'm not sure about the details.

@ljharb
Copy link
Member

ljharb commented Nov 28, 2019

That’s probably just because very few packages have modules that take advantage of this this behavior. Babel does it, and is often much more correct than typescript is, so that’s the example I’d suggest they follow.

@borekb
Copy link
Author

borekb commented Nov 29, 2019

Ok, I'm no expert on this but created this TS issue anyway: microsoft/TypeScript#35420.

Thank you for all the info, @ljharb.

@gauthierm
Copy link

As a workaround I was able to call the shim method and then use the Promise.allSettled API instead of calling allSettled directly.

@joshverd
Copy link

joshverd commented Nov 7, 2020

I fixed this issue by using an import require.

import allSettled = require('promise.allsettled');

@mikevinmike
Copy link

Had the same issue and fixed it with using .call() on allSettled function:
await allSettled.call(Promise, [...]);

@ljharb
Copy link
Member

ljharb commented Mar 3, 2021

This appears to be fixed in the next version of TypeScript.

@alexandrucancescu
Copy link

This still throws the same error when used with the last typescript version 4.2.3

import allSettled from "promise.allsettled";

allSettled([Promise.resolve(1), Promise.reject("")]);

Results in:

TypeError: #<Object> is not a constructor at Object.allSettled (<anonymous>) at Object.allSettled [as default]

tsconfig:

{
  "compilerOptions": {
    "lib": ["ES2019"],
    "module": "commonjs",
    "target": "ES2019",
    "outDir": "dist",
    "sourceMap": true,
    "declaration": true,
    "experimentalDecorators":true,
    "emitDecoratorMetadata": true,
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true
  }
}

@ljharb
Copy link
Member

ljharb commented Mar 31, 2021

@alexandrucancescu yes, because the fix isn't released yet. The next version of TypeScript will fix it.

@Narretz
Copy link

Narretz commented Jun 10, 2021

It was reverted because of a performance regression: microsoft/TypeScript#35877 (comment)

microsoft/TypeScript@cb9cd89

lokmeinmatz added a commit to lokmeinmatz/tsoa that referenced this issue Oct 21, 2022
Without this change, using vitest with "esm": true for routes config and an @Security guarded endpoint led to an error "... is not a constructor" or infinite running tests.

Bug related to es-shims/Promise.allSettled#5 (comment)
lokmeinmatz added a commit to lokmeinmatz/tsoa that referenced this issue Oct 24, 2022
Without this change, using vitest with "esm": true for routes config and an @Security guarded endpoint led to an error "... is not a constructor" or infinite running tests.

Bug related to es-shims/Promise.allSettled#5 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants