fix: Use export = in TypeScript definitions

[jordanbtucker]

Status

Written in TypeScript

• authentication
• authentication-client
• authentication-local
• commons
• configuration
• transport-commons

Has index.d.ts, but needs export =

• express
• feathers
• primus
• primus-client
• rest-client
• socketio
• socketio-client

Has index.d.ts, but doesn't need export =

• errors

Has no index.d.ts

• adapter-commons
• adapter-tests
• client

---

Since Feathers isn't written in TypeScript, and it doesn't use ES modules, its typings should use export = instead of export default.

The main reason for this change is that VSCode's intellisense is completely broken with Feathers when using JavaScript and CommonJS.

const feathers = require('@feathersjs/feathers')
const app = feathers()          // VSCode has no idea what `app` is.
const app2 = feathers.default() // VSCode recognizes `app2`, but it's ugly.

Another reason is that the following imports should all work in TypeScript.

import feathers from '@feathersjs/feathers'
// This works because module.exports.default is set by Feathers.

import * as feathers from '@feathersjs/feathers'
// This is how you would import Feathers if module.exports.default wasn't set.

import feathers = require('@feathersjs/feathers')
// You should also be able to import it this way because Feathers uses CommonJS,
// but it doesn't work because the typings use ES module syntax.

This PR follows the pattern used in Express' typings.

• Moved all of the exported types and interfaces to a new namespace called feathers.

  declare namespace feathers {
    type Id = number | string;
    type NullableId = Id | null;
    ...
  }

• Added a new Feathers interface, and moved version and default to that interface. This is the type of the export.

  interface Feathers {
    <T = any>(): Application<T>;
    version: string;
    default: Feathers;
  }

• Declared a feathers variable that implements Feathers and exported it with export = syntax.

  declare const feathers: Feathers;
  export = feathers;

This should probably be implemented in all feathers packages, but I just did this one as a start to gauge interest. I can do the rest of the typings and add it to this PR if desired.

This issue was raised in feathers-typescript with no response.

These changes were requested in DefinitelyTyped with no response from the package maintainers and eventually merged into DefinitelyTyped today. When I noticed that index.d.ts files were added to this repo, I decided to open a PR here.

Please let me know what you think and if there is anything I'm missing.

[daffl]

I was wondering why VSCode still didn't know about the app. So I think this makes sense, thank you! The tests failed because TSLint is complaining:

ERROR: packages/feathers/index.d.ts:25:1 - 'namespace' and 'module' are disallowed

This might warrant an exception but I'll let you double check. You can run the tests (including linting) with

npm install
npm test

In the cloned repository.

[jordanbtucker]

Sorry, I should know better to lint before submitting a PR.

I disabled the no-namespace rule, but I'm not quite sure if that's the route we want to take since some of the packages are written in TypeScript.

The TSLint docs say:

ES6-style external modules are the standard way to modularize code. Using module {} and namespace {} are outdated ways to organize TypeScript code.

So this applies to *.ts files and ES modules but not *.d.ts files written for CommonJS modules.

Another option might be to use a tool like dtslint to lint the index.d.ts files.

Before merging this, let me know if you want to wait for me to do the rest of the packages and add them to this PR, or if you want me to create a separate PR of each package.

[jordanbtucker]

I should have continued reading the TSLint docs. This is the configuration we should probably use.

"no-namespace": [ true, "allow-declarations" ]

This will prohibit namespace in *.ts files but allow declare namespace in *.d.ts files.

[daffl]

You can also just disable it for that file by adding a

/* tslint:disable:no-namespace */

At the beginning

[jordanbtucker]

You can also just disable it for that file

True, but that will need to be added to the rest of the handwritten index.d.ts files, making it less of an exception. I'll leave it up to you to decide.

[jordanbtucker]

I've got the typings for @feathersjs/express ready. Do you want me to add them to this PR or submit a new one?

[daffl]

Let's get it all into this one PR. And your'e right, if this setting will apply to more than one file it makes sense to keep it global.

[jordanbtucker]

Okay, for all CommonJS packages that already had an index.d.ts file, I updated that package with a CommonJS compatible version of index.d.ts.

A few packages still don't have index.d.ts files, but they are mainly internal use packages, except for client, which still needs typings.

I also removed the headers because they serve no purpose outside of DefinitelyTyped.

A few of the files needed triple-slash directives replaced with imports. See Consuming Dependencies and Global Libraries regarding that.

All tests passed, so the typings referenced by the packages written in TypeScript are working internally. I performed manual tests along the way, but some more human testing before merging wouldn't hurt.

We probably want to squash some, if not all, of these commits. Let me know if you want me to do that.

[daffl]

Argh, sorry I merged some PRs in the wrong order and now I'm confused on how to resolve those conflicts. Would you mind having a look? If the conflicts are fixed it's definitely good to merge.

[jordanbtucker]

Not a problem. I was on vacation last week, so I didn't have much time to work on this PR. I have a few more commits that I haven't pushed, anyway.

I also wanted to see if you'd be okay with me adding test files and implementing dtslint into the test scripts that will help ensure that the typings are correct.

[daffl]

Thanks! And yes, dtslint sounds fine I think but probably better in a new PR after this one.

[daffl]

Once the tests pass this should be good to go right?

[daffl]

Since tests are still passing I'll take this as a yes 😄, thank you for doing this!

[jordanbtucker]

Sorry, I missed your question. Thanks for merging.

I've got a dtslint branch I'm working on that will actually add testing for *.d.ts files.