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

Fix typings for use inside node.js #40

Closed
wants to merge 1 commit into from
Closed

Conversation

urish
Copy link

@urish urish commented Aug 24, 2017

With the current typings, you must write your import as

import PromisePool from 'es6-promise-pool';

however, this causes a runtime error in Node.js:

TypeError: es6_promise_pool_1.default is not a constructor

This commit changes the supported TypeScript import syntax to

import * as PromisePool from 'es6-promise-pool';

which works correctly in Node.js

@AppVeyorBot
Copy link

Build es6-promise-pool 156 failed (commit 31bce7c1c0 by @urish)

@timdp
Copy link
Owner

timdp commented Aug 25, 2017

Thanks for contributing!

Is this ultimately caused by this TypeScript issue?

In that case, maybe it's also worth mentioning that there's a legacy API with a named export rather than the default one that TypeScript is struggling with. I'm not sure if the compiler will understand it (and you'd need to add typings) but in ES6, this is also possible:

import { PromisePool } from 'es6-promise-pool'

I don't want to keep that API around per se because as I said, it's a legacy thing, but it does seem cleaner than import *, right? What would be the expected behavior here?

I'm no TypeScript expert though. The typings were contributed by @bcherny. Maybe he wants to weigh in as well?

@urish
Copy link
Author

urish commented Aug 25, 2017

Hi Tim, thanks for the super quick and thoughtful response. Yes, it seems to be related.

From examples in the typescript documentation (and also here), seems like they use the following syntax:

import PromisePool = require('es6-promise-pool');

However, it seems like the current typings don't play well with this syntax (typescript complains about Cannot use 'new' with an expression whose type lacks a call or construct signature), whereas the typings from the PR also makes it happy in this case.

image

I created a super-small demo repo for this, just git clone, npm install, npm start to watch it in in action:

https://github.com/urish/es6-promise-pool-typescript

@timdp
Copy link
Owner

timdp commented Aug 25, 2017

Hmm, I'm curious why it worked for @bcherny then.

We use TypeScript at work though. I'll ask someone on my team next week just to make sure we don't break anything. 🙂

@urish
Copy link
Author

urish commented Aug 25, 2017

Awesome, thanks!

Feel free to ping me if anything. And thanks for this awesome library, I recently used it in a project and it was a perfect fit

@jimthedev
Copy link

Hi there, any update to this? Currently seems like for ts/es6/cjs purposes many are switching away from default exports since it just makes things easier.

@Aankhen
Copy link

Aankhen commented May 19, 2018

FWIW, I have the same problem trying to use it with TypeScript and the PR fixed it.

@urish
Copy link
Author

urish commented Jul 29, 2020

Closing due to inactivity.

@urish urish closed this Jul 29, 2020
This pull request was closed.
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

Successfully merging this pull request may close these issues.

5 participants