error TS2038 and private type usage

Topics: Language Specification
Jan 17, 2014 at 4:25 AM
Hi,

Inside a module, attempting to reduce redundancy by defining a type interface that is then used in exported code results in an error TS2038: Parameter 'foo' of public method from exported class has or is using private type 'Type'.

I don’t understand why the compiler throws an error in this case. Since it uses structural typing to determine whether or not types are the same, why should it matter that the interface isn’t exposed?

Not OK:
interface ICallback {
  (foo:string, bar:number):void;
}

class Foo {
  observe(callback:ICallback):void {} // error TS2038
}

export = Foo;
OK:
class Foo {
  observe(callback:(foo:string, bar:number) => void):void {}
}

export = Foo;
Also OK, but really dumb:
var callbackType:(foo:string, bar:number) => void;

class Foo {
  observe(callback:typeof callbackType):void {}
}

export = Foo;
Jan 17, 2014 at 4:45 PM
What's wrong with
module foo {
     export interface ICallback {
          (foo:string, bar:number):void;
     }
     export class Foo {
         observe(callback:ICallback):void {}
     }
}
The error is reasonable because you may wish to work with types within the module closure that you don't want accessed by outside code.
Jan 17, 2014 at 5:05 PM
Edited Jan 17, 2014 at 5:06 PM
Most things are wrong with the module keyword. It only makes sense to use it if you are interested in polluting the global scope with legacy JavaScript patterns; if you are writing AMD or CJS modules, your files are already modules, and it is simply wasteful. Also, what you proposed is not the same as the code in the example; I could just put export before the interface and class if I wanted a hash map to be exported as the module value, but I don’t. I want the class to be exported.

There is no “accessing” of the type by outside code in the example I gave, so I do not see or understand how that could be a rationale for this restriction. The type is defined on parameters in order to restrict them, and the restriction is identical regardless of whether the type is named or not. The purpose of naming it is to reduce (keyboard) typing and stop errors caused by the type being changed inconsistently across the file.
Jan 17, 2014 at 5:49 PM
Okay, I see what you're getting at. You'd like to annotate callback just for your own pleasure within the file that you're working.

Call me old fashioned, but if one were to declare a public method on a class then anyone reading that class would expect that method to be used outside of the file.
Jan 17, 2014 at 6:00 PM
Edited Jan 17, 2014 at 6:05 PM
You'd like to annotate callback just for your own pleasure within the file that you're working.
No, I’d like to not have to write (foo:string, bar:number) => void over and over when I should be able to just create a named interface and use that instead. I just want to be able to type less and do more with fewer errors.
Call me old fashioned, but if one were to declare a public method on a class then anyone reading that class would expect that method to be used outside of the file.
TypeScript uses structural subtyping, so one does not have to implement ICallback in order to provide a valid argument from code that has no access to the named ICallback interface. Therefore, in my opinion, there should not be an error about the interface being private, since a consumer is not restricted from implementing a compatible interface, they just will not be able to directly use the original definition (if an object type literal is used instead the same issue exists, except the compiler doesn’t complain about it).
Jan 17, 2014 at 6:08 PM
You should be careful. Revolutions have been started over requiring people to follow secret rules.
Jan 17, 2014 at 6:24 PM
Yes, all understood on the first go. While this might benefit those developing a library or component in isolation, it will lead to a lot of redundancy in large codebases where both the library and it's consumers are part of the same organisation. We would end up with ICallback being defined by every single client.

I would define all interfaces externally and make them available to both client and library.
Coordinator
Jan 17, 2014 at 8:20 PM
Here's one solution, you can create a declaration merge of the class with a module that only contains the interface. This gives you a way to namespace the interfaces to the symbol, so that other people can get to them, while not duplicating effort or changing codegen:
class Foo {
    observe(callback: Foo.ICallback): void { } 
}

module Foo {
    export interface ICallback {
        (foo: string, bar: number): void;
    }
}

export = Foo; 
Marked as answer by csnover on 1/17/2014 at 12:26 PM
Jan 17, 2014 at 8:53 PM
jonturner, it’s interesting and a little scary that identifiers can be duplicated like that and it works. Is it planned that this will continue to work long-term? Are there defined rules about what overrides what in the specification when this is done?

It turns out there is more than one error code related to this condition; TS2027 is basically the same thing. I thought it was odd that this would have been the first time someone ran into this, but for some reason I was unable to find the other one when searching the other day. So, there is a related discussion to this at https://typescript.codeplex.com/discussions/447372, for reference, that may contain additional rationale for not throwing these errors.

The workaround is OK, but it feels hacky, and I still am not entirely clear on what the rationale is for restricting named interfaces like this, and I would like to understand this.

Is the idea to intentionally roadblock people into doing things the Right Way? If so, it’s arguably a fair thing to do, but it makes prototyping components in TS annoying. (Yes, I will split out the interface later to one of our interfaces files, but I don’t want to break my code flow right now by having to go and do that.)

Is it an artefact of the lack of export = until later versions of TS (so exporting the interface wasn’t a big deal before because you always had the ability to export multiple things)? If so, it should probably be re-evaluated.

Is there another reason? If so, I’d like to understand it.

Other than the above case, I have also experienced a need to create separate types when modelling inheritance that TypeScript can’t do correctly by itself (like extending Error, or multiple inheritance). In these cases, the type may be only used once ever (as the export type), and I simply want to split it into a named interface to avoid having really long, really ugly lines of code. (These sorts of classes/interfaces in TypeScript, that use existing JavaScript utility code to generate constructors, are very difficult to manage mentally right now. This might be another topic for another thread.) I’d like to say these are weird edge cases but I seem to keep running into them. It feels like preventing use of private interfaces prevents me from having tidier code.

Thanks everyone for your input,
Coordinator
Jan 21, 2014 at 12:07 AM
This kind of duplication is actually an intended feature called "declaration merging". I've got a draft of the write-up on it here: https://typescript.codeplex.com/wikipage?title=Declaration%20Merging which should help cover the what and the how. Granted this is a draft article, and feedback is always welcome.

The core of the idea is that, just as in JavaScript, you can build up a definition by merging together two ideas. For example, in JavaScript you can add additional members to a function, so now it acts as both a function and as an object. Declaration merging is how you build up these hybrids in TypeScript in a way that maintains type checking. It has the added advantage that it also allows you to model other common patterns, like inner classes.

Declaration merging is a bit different than export=, though they work together so that you can export more complex types as the result of importing a module. The declaration merge gives you a single name for this hybrid that you can use, or pass to export=.

re: the issue extending types like Error - do you have some example code here that isn't working? I'd like to understand that first, to see if there's something we can do there.