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

Update amqplib definitions to v0.5.0 #12427

Merged
merged 3 commits into from
Nov 7, 2016

Conversation

nfantone
Copy link
Contributor

@nfantone nfantone commented Nov 1, 2016

Please fill in this template.

  • Prefer to make your PR against the types-2.0 branch.
  • The package does not provide its own types, and you can not add them.
  • Test the change in your own code.
  • Follow the advice from the readme.
  • Avoid common mistakes.

If adding a new definition:

  • If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
  • Run tsc without errors.
  • Include the required files and header.

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: amqp-node/amqplib@e5c19d4
  • Increase the version number in the header if appropriate.

@dt-bot
Copy link
Member

dt-bot commented Nov 1, 2016

amqplib/index.d.ts

to authors (@mnahkies @abreits). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

nfantone referenced this pull request in amqp-node/amqplib Nov 1, 2016
@nfantone
Copy link
Contributor Author

nfantone commented Nov 2, 2016

Travis fails with an error unrelated to this PR:

For file:/home/travis/build/DefinitelyTyped/DefinitelyTyped/chocolatechipjs/tsconfig.json
\home\travis\build\DefinitelyTyped\DefinitelyTyped\chocolatechipjs\index.d.ts
     Line 1363: All declarations of 'responseURL' must have identical modifiers.

@vvakame
Copy link
Member

vvakame commented Nov 2, 2016

It is out problem. types-2.0/HEAD is back to green.

import * as Promise from 'bluebird';
import * as events from 'events';
import shared = require('amqplib/properties');
import callback_api = require('amqplib/callback_api');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this callback_api import is unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely right. Removed.

import shared = require('amqplib/properties');
import callback_api = require('amqplib/callback_api');

export import Replies = shared.Replies;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simplier syntax might be to do

export {
    Replies,
    Options,
    Message
} from 'amqplib/properties';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are inlined export imports. Your suggestion wouldn't do the same thing, am I right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, I thought that exporting in the fashion I suggested also imported them into the local scope

@mnahkies
Copy link
Contributor

mnahkies commented Nov 3, 2016

Assuming passes CI after rebase / merge with upstream then 👍

@vvakame vvakame merged commit ba5d738 into DefinitelyTyped:types-2.0 Nov 7, 2016
@nfantone nfantone deleted the update/amqplib branch November 7, 2016 16:45
@nfantone
Copy link
Contributor Author

nfantone commented Nov 7, 2016

@mnahkies Ok, it may be a little too late now - but I just realized that for some awkward reason, Bluebird.Promise is not compatible with Promise<T> interface exported by lib.es6.d.ts - which is very inconvenient when mixing different libraries that expect or use Promises (like RxJS).

When updating to 0.5.0, I keep getting compiler errors like:

[ts] Argument of type 'Bluebird<void>' is not assignable to parameter of type 'Promise<void>'.

Thoughts?

@mnahkies
Copy link
Contributor

mnahkies commented Nov 8, 2016

@nfantone sorry but I haven't had a chance to play with Bluebird so I'm unfamiliar with it's type definitions. Based on your comment on pull request #11036 you have it under control though.

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.

None yet

4 participants