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

Extensibility problem #4

Open
niieani opened this issue Sep 27, 2016 · 14 comments
Open

Extensibility problem #4

niieani opened this issue Sep 27, 2016 · 14 comments

Comments

@niieani
Copy link

niieani commented Sep 27, 2016

@blakeembrey My original typings were contained in a named module, which as I see you've changed to a namespace. This is fine, but now it's impossible to extend the typings (unless I'm unaware of a way?) in an external module.

I'd like to recreate the typings for rethinkdbdash without copy-pasting and having to maintain 2 different versions of the same code. rethinkdbdash simply has a wrapper interface and additionally adds the ability to return Promises from any query (without having to .run()).

Any suggestions?

@blakeembrey
Copy link
Contributor

You should be able to use module augmentation with TypeScript for the use-cases where you would previously have used globals to merge modules.

@niieani
Copy link
Author

niieani commented Sep 27, 2016

Yeah, I thought so too. But as soon as I create a module entry to augment the typings, TypeScript starts ignoring the original module. I'll try on a fresh project and see how it goes when I have a bit more time.

@blakeembrey
Copy link
Contributor

It needs a top-level import or export to be considered an "external module" and for augmentation to be valid. Otherwise that same syntax just means "declare module" in a global declaration file. I'll have to test it though.

@ktersius
Copy link

ktersius commented Nov 12, 2016

Has anyone made progress on how to extend an interface?

As mentioned I'm also using rethinkdbdash and the export interface Run needs to be extended to allow running without a connection but I could not figure out a way that was working.

This is what I tried in it's own d.ts file.

import * as rethinkdb from 'rethinkdb';
import Bluebird = require("bluebird");
declare module "rethinkdb" {
    namespace rethinkdb {
        export interface Run <T> {
            run (connection: Connection, cb: Callback<T>): void;
            run (connection: Connection, options: RunOptions, cb: Callback<T>): void;
            run (connection: Connection, options?: RunOptions): Bluebird<T>;
            run (): Bluebird<T>;
        }
    }
}

@blakeembrey
Copy link
Contributor

@ktersius You will probably need to read some of the TypeScript docs/handbook to get a better understanding of how extending a definition works. There's no namespace rethinkdb and using import at the top-level means this is augmentation (not extension). If you wanted to the old-fashioned global extension you'd need something like:

declare module "rethinkdb" {
    import * as rethinkdb from 'rethinkdb';
    import Bluebird = require("bluebird");

    namespace r {
        export interface Run <T> {
            run (connection: Connection, cb: Callback<T>): void;
            run (connection: Connection, options: RunOptions, cb: Callback<T>): void;
            run (connection: Connection, options?: RunOptions): Bluebird<T>;
            run (): Bluebird<T>;
        }
    }
}

If you want to use augmentation, you need to tweak that slightly.

@blakeembrey
Copy link
Contributor

Actually, I think extending an export = may have some issue still with TypeScript, but I can test that later when I get a chance.

@ktersius
Copy link

ktersius commented Nov 12, 2016

@blakeembrey Yes using your example gives typings/modules/rethinkdb/index.d.ts(3647,1): error TS2309: An export assignment cannot be used in a module with other exported elements.

@blakeembrey
Copy link
Contributor

blakeembrey commented Nov 12, 2016

@ktersius Remove export from export interface maybe?

@ktersius
Copy link

ktersius commented Nov 12, 2016

@blakeembrey That has no effect it seems...this is using v 2.0.3 btw.

Still gives:
typings/modules/rethinkdb/index.d.ts(3647,1): error TS2309: An export assignment cannot be used in a module with other exported elements.

declare module "rethinkdb" {
    import * as rethinkdb from 'rethinkdb';
    import Bluebird = require("bluebird");

    namespace r {
        interface Run <T> {
            run (connection: rethinkdb.Connection, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options: rethinkdb.RunOptions, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options?: rethinkdb.RunOptions): Bluebird<T>;
            run (): Bluebird<T>;
        }
    }
}

@ktersius
Copy link

ktersius commented Nov 12, 2016

@blakeembrey

This seems to do the trick and I can now compile. Although webstorm is completely confused :)
So as I understand this is now using module augmentation?

import * as rethinkdb from 'rethinkdb';
import Bluebird = require("bluebird");
declare module "rethinkdb" {
    namespace r {
        interface Run <T> {
            run (connection: rethinkdb.Connection, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options: rethinkdb.RunOptions, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options?: rethinkdb.RunOptions): Bluebird<T>;
            run (): Bluebird<T>;
        }
    }
}

@blakeembrey
Copy link
Contributor

@ktersius Yes, that would be the preferred way to do it. Then re-export the definition using:

export = rethinkdb

@ktersius
Copy link

ktersius commented Nov 12, 2016

@blakeembrey Seems it doesn't want the re-export though:
TS2666: Exports and export assignments are not permitted in module augmentations

Otherwise it seems to work fine as far as the compiler is concerned.

Edit: Correction, following compiles.

import * as rethinkdb from 'rethinkdb';
import * as rethinkdbdash from 'rethinkdbdash';
import Bluebird = require("bluebird");
declare module "rethinkdb" {
    namespace r {
        interface Run <T> {
            run (connection: rethinkdb.Connection, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options: rethinkdb.RunOptions, cb: rethinkdb.Callback<T>): void;
            run (connection: rethinkdb.Connection, options?: rethinkdb.RunOptions): Bluebird<T>;
            run(connection: rethinkdb.Connection, options?: rethinkdbdash.RDashConnectionOptions): Bluebird<T>;
            run(options?: rethinkdbdash.RDashConnectionOptions): Bluebird<T>;
        }
    }
}
export = rethinkdb;

@blakeembrey
Copy link
Contributor

Yeah, the correction is the correct way. It's an augmentation, so everything needs to continue being written in external module format. You can then install that definition with Typings or using typeRoots for TypeScript to discover it.

@ktersius
Copy link

ktersius commented Nov 12, 2016

@blakeembrey Thanks for all your help!

Just posting my tsconfig.json for reference. It doesn't like adding "typeroots" : ["typings", "node_modules/@types"] so I'm using files. Where rethinkdbex/index has the above code.

{
  "compilerOptions": {
    "target": "es6",
    "module": "commonjs",
    "moduleResolution": "node",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": false,
    "inlineSourceMap": true,
    "inlineSources": true,
    "noEmitHelpers": false,
    "strictNullChecks": false,
    "outDir": "./dist",
    "rootDirs": [
      "./src",
      "../shared/src"
    ],
    "baseUrl": "./",
    "paths": {
      "lodash": [
        "node_modules/@types/lodash"
      ],
      "ajv": [
        "node_modules/ajv"
      ]
    }
  },
  "lib": [
    "es6",
    "dom"
  ],
  "files": [
    "typings/modules/rethinkdb/index",
    "typings/modules/rethinkdbdash/index",
    "typings/modules/rethinkdbex/index"
  ],
  "include": [
    "src/**/*"
  ],
  "exclude": [
    "node_modules",
    "dist"
  ],
  "compileOnSave": false,
  "buildOnSave": false
}

@niieani niieani mentioned this issue Mar 5, 2017
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

No branches or pull requests

3 participants