Make extensible & externally usable

[OlsonDev]

The way I've written these up for now works in an external project with typings install --save github:OlsonDev/typed-keyboardjs#commitish. I can do something like:

import kbjs = require('keyboardjs/index');
kbjs.default.bind('F12', (e) => win.openDevTools());
const diffLocale = new kbjs.Locale();
diffLocale.localeName = 'foo';
const otherKb = new kbjs.Keyboard(window, document.documentElement, null, null);
otherKb.setLocale('foo', diffLocale);
// ...

However, I can't do something like:

// Error: Cannot find namespace 'keyboardjs'
interface ExtendedKeyboard extends keyboardjs.Keyboard {
    keydownOnce(keys: string | string[], callback: keyboardjs.EventHandler): void;
}

const keyboard = kbjs.default as any as ExtendedKeyboard;

// Error: Cannot find namespace 'keyboardjs'
keyboard.keydownOnce = function(keys: string | string[], callback: keyboardjs.EventHandler) {
    let keyPressed = false;
    return this.bind(keys
        , (e) => {
            if (keyPressed) return;
            keyPressed = true;
            callback(e);
        }
        , () => keyPressed = false
    );
};

I've looked at the examples and played around with a few different syntaxes to see what typings would generate with the yo typings generated command npm run watch and I'm just not experienced enough to figure this out.

@blakeembrey Could you please take a look at what I have and advise specifically what I need to change? One of my biggest issues getting this far was exposing both the types and the Keyboard instance available via default; see how KeyboardJS does its exports.

[blakeembrey]

@OlsonDev Hey, sorry for the delay. Been getting through all the issues on my backlog. These types of interfaces are tricky to type - let me see how we can do this.

[OlsonDev]

@blakeembrey No problem! 😄 tsc compiles the code into something that ultimately does what I want; it just emits warnings/errors I'd rather not see and the IDE/IntelliSense experience is lost.

[blakeembrey]

@OlsonDev Honestly, I just spent 25 minutes looking at it and I can't think of how to make this interface work. I'd go ahead and log an issue with TypeScript or ask a question on StackOverflow. Maybe there's something I'm overlooking.

[OlsonDev]

@blakeembrey Hah, okay. I'll do that in a bit here. Thanks for taking a look!

[blakeembrey]

You can get part of the way with intersections or extending the base class with static methods. For example:

var keyboardjs: ImportedKeyboard & {
    Locale: typeof ImportedLocale;
    KeyCombo: typeof ImportedKeyCombo;
    Keyboard: typeof ImportedKeyboard;
    KeyEvent: typeof ImportedKeyEvent;
};

export = keyboardjs;

However, this only allows you to use the properties in value positions and not type positions so the eventhandler still won't work.

new keyboardjs.KeyCombo('te')

class ExtendedKeyboard extends keyboardjs.Keyboard {
    keydownOnce(keys: string | string[], callback: /* keyboardjs.EventHandler */): void {}
}

[OlsonDev]

Hmm, good to know. That seems really weird to me (extends foo.TypeX working but callback: foo.TypeY not).

EDIT: Nevermind... I see why that obviously doesn't work.

[OlsonDev]

Here's the question on Stack Overflow.

[OlsonDev]

This actually turned out to be relatively simple. Let me know if I'm overlooking something but I landed on this:

import { Locale as ImportedLocale } from './lib/locale';
import { KeyCombo as ImportedKeyCombo } from './lib/key-combo';
import {
    Keyboard as ImportedKeyboard,
    KeyEvent as ImportedKeyEvent,
    EventHandler as ImportedEventHandler
} from './lib/keyboard';

export class Locale extends ImportedLocale {}
export class KeyCombo extends ImportedKeyCombo {}
export class Keyboard extends ImportedKeyboard {}
export interface KeyEvent extends ImportedKeyEvent {}
export interface EventHandler extends ImportedEventHandler {}

declare const out: Keyboard;
export default out;

[blakeembrey]

@OlsonDev You can't use export default, I believe I've mentioned that. The code you are typings is not ES6-compatible, otherwise this would have been simple.

[blakeembrey]

Did you try using the the code wrote? I'm sure you'd see this issue very quickly 😄

[OlsonDev]

I believe I've mentioned that.

Not seeing where... :-\ The docs or in another issue we've discussed?

The code you are typings is not ES6-compatible

Bummer. I don't understand how though -- I'm not familiar with the intricacies of ES6 export.

Did you try using the the code wrote?

Yes, actually. It works. No build errors from typings:

C:\Storage\Projects\typed-keyboardjs>npm run watch

> @ watch C:\Storage\Projects\typed-keyboardjs
> onchange "**/*.ts" -i -e "out/**" -- npm -s run build+test

building...
source-testing...
source-test is not specified
testing...

  Default exists

    √ Fake test passed!


  total:     1
  passing:   1
  duration:  950ms

So I committed it, updated the typings in my project that depends on this, changed a few syntax things there (keyboardjs => kbjs), and tsc had 0 warnings/errors (a first!). My project using this targets es2015 with a system module loader. If I change "module" to es2015, it complains: Import assignment cannot be used when targeting ECMAScript 6 modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead. where I use it. I changed the syntax to how it wanted (import kbjs = require('keyboardjs'); => import * as kbjs from 'keyboardjs') and that error went away.

Which error were you expecting me to see...? Any specific config settings to get it to show up?

[blakeembrey]

You will have a runtime error, not a type error.

[blakeembrey]

Also, your tests in the directory don't actually test anything from what I see?

[blakeembrey]

Basically, export default !== module.exports at all. ES6 has no concept of module.exports, when you concert export default in ES6 to CommonJS, it's equivalent to exports.default (not module.exports). Hence, what's you've got is an implementation that isn't actually correct against the runtime and therefore, will not work in the runtime if you're writing against this type definition.

[OlsonDev]

You will have a runtime error, not a type error.

I tested it at runtime too -- no error.

Also, your tests in the directory don't actually test anything from what I see?

Heh, yup. That's a TODO, as well as putting JSDoc/whatever comments in the .d.ts files. I don't know if you saw the comment in test.ts but I couldn't test in the node context because keyboardjs requires a DOM Element. I don't know which mock library to use. If you have recommendations, I'll take 'em. 😄

[blakeembrey]

@OlsonDev That's not possible, I know that. I suppose you're using SystemJS then, which is the only platform doing that interop for you. Any other system will fail.

[blakeembrey]

Maybe jsdom?

[blakeembrey]

Also, I realised you can do this:

import { Locale } from './lib/locale';
import { KeyCombo } from './lib/key-combo';
import { Keyboard, KeyEvent, EventHandler } from './lib/keyboard';

declare class KeyboardJS extends Keyboard {
    Locale: typeof Locale;
    KeyCombo: typeof KeyCombo;
    Keyboard: typeof Keyboard;
    KeyEvent: typeof KeyEvent;
    EventHandler: EventHandler;
}

declare var keyboardjs: KeyboardJS;

export = keyboardjs;

^ That one is technically correct.

[blakeembrey]

We could also run tests in an actual browser (E.g. PhantomJS) if you're interested in using Browserify + tape-run.

[OlsonDev]

My bad -- I meant I tested the SystemJS at runtime. I didn't try the es2015 module at runtime because pretty much none of that works. 😛

I'll try jsdom then. I don't know that an actual browser matters -- the tests are to test the typings can be applied and no obvious runtime exceptions occur, right? There's no need to duplicate all of the library's tests... right?

I'll try that code you just included. I assume you also meant KeyEvent: KeyEvent?

[blakeembrey]

@OlsonDev Possibly, yes. I did changes all over the place, sorry! Turns out one of them was making KeyEvent a class, but yes, as an interface you shouldn't use typeof.

[blakeembrey]

In the test case, what if you just fake global.document?

Edit: Or whichever the library needs?

[OlsonDev]

Sure, I could make a simple mock, too. I'm also unclear as to how extensive/what the tests should be. I don't think it's the typings repo's job to test the entire library...? Just that the API surface is consistent with the claimed surface, yeah? As in, return values/outputs/whatever don't matter so much as properties/methods existing.

[blakeembrey]

@OlsonDev Yes, doesn't need to be extensive. Let's just make sure the import is tested by using a couple of the methods/classes 😉

[OlsonDev]

Got it. 👍

Your code above actually wouldn't work. Two issues:

1. No default property. Addressed by removing extends Keyboard and adding default: Keyboard; after EventHandler: EventHandler;
2. Can't write a function requiring parameter of type kbjs.EventHandler -- not sure what you intended there?

[blakeembrey]

@OlsonDev THERE IS NO DEFAULT. That's what I'm saying.

[blakeembrey]

And 2 works for me.

[blakeembrey]

I'll just do a PR instead.

[blakeembrey]

@OlsonDev Explained in the PR, please don't introduce default at the definition level - it'll only cause pain for everyone since it doesn't actually exist and is just SystemJS doing magic for you. Instead, use the TypeScript flag to change your behaviour locally (only affects you and not everyone else now).

[OlsonDev]

Okay, I'm pretty sure I get the whole "default isn't real" business now. Thanks for the PR/help.

And 2 works for me.

Because you used typeof where the parameter type was specified -- None of our other examples required it so I did not realize you intended it to be. What I had copied worked for me when I added typeof. 😄 It is kind of a drag to have to use typeof in a type position though. Oh well.

[OlsonDev]

In my other project, I did this:

import keyboard, { Keyboard, EventHandler, KeyEvent } from 'keyboardjs/index';
class ExtendedKeyboard extends Keyboard {
    keydownOnce(keys: string | string[], callback: typeof EventHandler)  {
        // stuff
    }
}
const extended = keyboard as any as ExtendedKeyboard;
extended.keydownOnce = ExtendedKeyboard.prototype.keydownOnce;
// stuff

And tsc generated code that did this: extended = index_1.default; so my runtime doesn't break, even though other runtimes wouldn't have this magical .default, as you say. Now... to remember this lesson. ;-)