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

Gracefully handle global promise pollution #3502

Merged
merged 1 commit into from Oct 29, 2019

Conversation

@lorefnon
Copy link
Collaborator

lorefnon commented Oct 29, 2019

  • Update QueryInterface to proxy to only standard promise methods
  • Update QueryInterface type to be explicit about the methods being
    proxied

This primarily handles pollution of global Promise (either the type or
the global object at runtime) by other libraries.

Fixes: #3422 (comment)

- Update QueryInterface type to be explicit about the methods being
  proxied

This primarily handles pollution of global Promise type by other
libraries which can break Knex typings.

Resolves: #3422 (comment)
@lorefnon lorefnon force-pushed the promise-pollution branch from 707c90b to e9fd66e Oct 29, 2019
@kibertoad kibertoad merged commit 6f5a13d into master Oct 29, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@kibertoad kibertoad deleted the promise-pollution branch Oct 29, 2019
@onehorsetown

This comment has been minimized.

Copy link

onehorsetown commented Oct 31, 2019

e9fd66e

This change broke TSLint for my TypeScript project. Everything lints fine in 0.20.0, but with 0.20.1, I get tons of:

ERROR: (await-promise) ... Invalid 'await' of a non-Promise value.
ERROR: (no-invalid-await)  ...Refactor this redundant 'await' on a non-promise.

Trying to figure out why the tslint no-invalid-await and await-promise don't think this is a promise/thenable/etc.

@lorefnon

This comment has been minimized.

Copy link
Collaborator Author

lorefnon commented Oct 31, 2019

Do you have "await-promise": [true, "Thenable"] in your tslint config ?

types/index.d.ts Show resolved Hide resolved
@onehorsetown

This comment has been minimized.

Copy link

onehorsetown commented Oct 31, 2019

@lorefnon Yes, I've tried every variant of

"await-promise": [true, "Thenable", "PromiseLike", "Bluebird", etc...]

I've stopped in the TSLint code and the name of the type at the point where it is checked is __type. So, this actually makes the problem go away, but seems very wrong:

"await-promise": [true, "__type"]

The other linting error (no-invalid-await) is coming from the tslint-sonarts package which is checking if the type has a then method right here:

https://github.com/SonarSource/SonarTS/blob/4a4080b78001a78d758d1d0fa0190fb9496b6f57/sonarts-core/src/rules/noInvalidAwaitRule.ts#L57

At that point in the linting code, the ChainableInterface does not have a then method, but I have no idea why.

@72636c

This comment has been minimized.

Copy link

72636c commented Oct 31, 2019

You can make an exception with "await-promise": [true, "ChainableInterface"]

@onehorsetown

This comment has been minimized.

Copy link

onehorsetown commented Nov 1, 2019

You can make an exception with "await-promise": [true, "ChainableInterface"]

@72636c - That fixes it. Thanks.

The other issue I then have is that the tslint-sonarts linting plugin does not think the Picked type does not have a then method... which it, of course, does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.