Typescript: definitions should be exported for re-use

rob-pocklington-a139759 commented 5 years ago

It's great to have a Typescript definitions file https://github.com/watson-developer-cloud/botkit-middleware/blob/master/lib/middleware/index.d.ts in the repo, sadly I can't use it in our botkit project because none of the interfaces are exported - can you please amend this? Currently you have one default export which is a deprecated pattern, they should be named so this may be a breaking change.

rob-pocklington-a139759 commented 5 years ago

Also would be good to rename interface Message to interface WatsonMessage (same for Payload and ContextDelta so they follow the same naming convention.

germanattanasio commented 5 years ago

Thanks a lot for filing this issue! Would you like to write a PR for this? I'd be more than happy to walk you through the steps involved.

rob-pocklington-a139759 commented 5 years ago

Yep happy to do a PR if you're good with the suggestions...

germanattanasio commented 5 years ago

Fixed in #165

Naktibalda commented 5 years ago

I can only blame myself for not reacting to this ticket in time.

It was a breaking change.

The old way of importing middleware no longer works:

import createWatsonMiddleware = require('botkit-middleware-watson');

result:

> node ./node_modules/typescript/bin/tsc
src/bot.ts:19:30 - error TS2349: Cannot invoke an expression whose type lacks a call signature. Type 'typeof import("/.../node_modules/botkit-middleware-watson/lib/middlewar...' has no compatible call signatures.

 19     const watsonMiddleware = createWatsonMiddleware({
                                 ~~~~~~~~~~~~~~~~~~~~~~~~
 20         username: process.env.CONVERSATION_USERNAME,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
 25         //minimum_confidence: 0.50, // (Optional) Default is 0.75
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 26     });

But expected way of importing default export doesn't work either:

import createWatsonMiddleware from 'botkit-middleware-watson';

It compiles successfuly but fails at runtime:

> node build/app.js

Initializing Botkit v0.6.21
info: ** No persistent storage method specified! Data may be lost when process shuts down.
/.../build/bot.js:14
    var watsonMiddleware = botkit_middleware_watson_1.default({
                                                             ^

TypeError: botkit_middleware_watson_1.default is not a function

rob-pocklington-a139759 commented 5 years ago

You're right. It's probably best to undo this change and do it properly with a breaking change (which will be cleaner - only using named exports and without a default export)

Naktibalda commented 5 years ago

@rob-pocklington-a139759 I updated middleware to support botkit 4 in https://github.com/watson-developer-cloud/botkit-middleware/pull/174

It will require a bump in major version, so I tried to solve your problem too, but I am a bit rusty on TypeScript and I can't figure out how to export everything correctly. Could you help me to get this export right? https://github.com/watson-developer-cloud/botkit-middleware/pull/174/files#diff-7bfe83a69b36a4228ed276b0923677e8L142 https://github.com/watson-developer-cloud/botkit-middleware/pull/174/files#diff-2b137e40025e00eb45b66941ad8d4ddfR179

rob-pocklington-a139759 commented 5 years ago

Yep, added a comment to the first link above.

germanattanasio commented 5 years ago

Fixed in 2.0.0

watson-developer-cloud / botkit-middleware

Typescript: definitions should be exported for re-use #163