search

abreits / amqp-ts

AmqpSimple, a RabbitMQ tested Amqp library written in and for Typescript
MIT License
133 stars 37 forks source link

Allow using the noImplicitAny option in TypeScript #6

Closed dchw closed 8 years ago

dchw commented 8 years ago

When using the library via npm, and if you have the noImplicitAny option enabled, you get compiler errors. This should fix that. See attached txt for the errors. noImplicitAnyLog.txt

There were some methods in the .d.ts of the package that lacked return type declarations. I added those to the source, and updated the definitions. Also added the noImplicitAny option to the tsconfig.json file, and added a few missing type declarations to the source to tighten things up.

The transpiled json output remains the same, since the types are only handled by Typescript anyways.

abreits commented 8 years ago

I like how you cleaned up the code, however there are a few points I like to be addressed:

  1. In the Topology interface you remove the ? from the exchanges, queues and bindings properties. They need to be optional, it is allowed to define an incomplete topology.
  2. While I agree with the need to support projects that use noImplicitAny, I do not want to be forced to use it inside this project. So please remove the noImplicitAny from the tsconfig.json. As you have probably seen, the lib/amqp-ts.d.ts is maintained manually, I will make sure that future updates here will be noImplicitAny compatible.
dchw commented 8 years ago

1 - I'll look at that. I don't think I meant to do that! Sorry, ill clean that up ASAP.

2 - I can totally get behind that. It was turned on so I could locate all potential places where the types were missing and fill them in. I'll remove that as well.

Thanks for your feedback!

dchw commented 8 years ago

I went ahead and modified my .d.ts to make it match the organization of yours rather than the auto-generated one from tsc. It makes what changed much more obvious.