TritonDataCenter / node-verror

Rich JavaScript errors
MIT License
1.18k stars 61 forks source link

Typescript problem with WError but not with VError #50

Closed Halt001 closed 6 years ago

Halt001 commented 6 years ago

I'm trying to use VError using Typescript and I have installed @types/verror but I get a compiler error that I can't explain on VError that I don't get with WError.

import { VError, WError } from 'verror';

const createVError = (): VError => { // [ts] Can not find name VError
  return new VError('This does not compile'); // No problem with VError here
};

const createWError = (): WError => {
   return new WError('This compiles fine');
};

Is there something I need to do differently?

davepacheco commented 6 years ago

Sorry, I don't know enough about Typescript or ES6 to know what's wrong here. If there's a specific suggestion for VError or WError that can fix this (and won't break existing users), we'll take it.

TheDocTrier commented 6 years ago

If the types for verror are not included in this repository, should TS-related issues be put here? Anyways, the problem is being caused by the DefinitelyTyped types incorrectly declaring the VError type outside of the exported VError namespace (which makes a second issue of reusing a name in the same scope, VError is declared as both a class and a namespace), see below:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3925a1622a5218568ca23767193b8c69899d0ed1/types/verror/index.d.ts#L18-L38

/* ...
 *   with err.message = 'file not found'
 */
declare class VError extends Error {
    static VError: typeof VError;

    static cause(err: Error): Error | null;
    static info(err: Error): VError.Info;
    static fullStack(err: Error): string;
    static findCauseByName(err: Error, name: string): Error | null;
    static hasCauseWithName(err: Error, name: string): boolean;
    static errorFromList<T extends Error>(errors: T[]): null | T | VError.MultiError;
    static errorForEach(err: Error, func: (err: Error) => void): void;

    cause(): Error | undefined;
    constructor(options: VError.Options | Error, message: string, ...params: any[]);
    constructor(message?: string, ...params: any[]);
}

declare namespace VError {
    interface Info {
        [key: string]: any;

This can be fixed relatively easily and I would do it myself, but I don't have the time to fork the DT repository at the moment. @Halt001, I would recommend that you fork and fix this bug on the DT repository as practice. If not, I'll try to get around to submitting a pull request at some later time. Also remember, if the developers, such as @davepacheco, of a particular repository aren't supplying their own types, it shouldn't be their responsibility to fix types on DT.

@davepacheco, don't wory, it's not an ES6 issue, purely TS. Also, if the types were moved here, it shouldn't be that hard to maintain them (considering that the library is not very complex). Even now, the types on DT are about 30 lines (ignoring comments, which make up most of the file).

Halt001 commented 6 years ago

Thx for your detailed response. I'm trying out what you suggest but I'm struggling with it. One of the problems is that the following line in the verror-tests.ts file is no longer possible: import VError = require("verror");

You would need to use new VError.VError(error, "bar"); in stead of new VError(error, "bar");

This means it would break everybody's code.

TheDocTrier commented 6 years ago

Actually, you can use interfaces to add in that functionality:

declare namespace verror {
  export class VError {
    // ...
  }

  export class WError extends VError {}

  // ...
}

declare const verrorModule: typeof verror & {
  (/* ... */): verror.VError;
  new (/* ... */): verror.VError;
};

export = verrorModule;

All of the following statements compile correctly:

import VError = require("verror");

const err1 = VError();
const err2 = new VError();
const err3 = new VError.WError();
Halt001 commented 6 years ago

I got most things compiling although with duplication of every static method on the VError class. The one thing that I can't get to behave is the following:

import VError = require("verror");
...
const toList1: null|VError|MultiError = VError.errorFromList([verror1]);

It complains that it can not find name VError.

I'm in over my head here and I have not enough knowledge of this to go through with the PR. Thank you for your help but I'll give up here.